Skip to content

Minor encoder fixes#224

Open
zlatinski wants to merge 4 commits intomainfrom
minor-encoder-fixes
Open

Minor encoder fixes#224
zlatinski wants to merge 4 commits intomainfrom
minor-encoder-fixes

Conversation

@zlatinski
Copy link
Copy Markdown
Contributor

debug: log VkVideoEncodeUsageInfoKHR at profile creation
fix: guard dlclose/FreeLibrary against null handle in ~VulkanDeviceContext
test: add AV1 encoder quality regression test script
encoder: join worker threads in destructor to prevent use-after-free on exit

…on exit

VkVideoEncoder spawns background worker threads (m_assemblyThreads for
bitstream assembly and m_encoderQueueConsumerThread for draining the
encode submission queue) that reference encoder-owned state. On exit,
DeinitEncoder() signals the queues to flush, but if the destructor
returns before the threads actually exit, they may still be accessing
members that have already been destroyed, causing crashes or UB during
process teardown.

Fix it by explicitly joining all worker threads in ~VkVideoEncoder()
after DeinitEncoder(). Each joinable() check keeps it idempotent and
safe for encoders that never started the threads.

Also remove the unused VkThreadSafeQueue::Drain() helper, which was
superseded by SetFlushAndExit() and had no remaining callers.

Signed-off-by: Tony Zlatinski <tzlatinski@nvidia.com>
@zlatinski zlatinski force-pushed the minor-encoder-fixes branch from 5446163 to 67a5e7c Compare April 16, 2026 22:36
Adds vk_video_encoder/test/av1_encoder_quality_test.py that encodes
test content with H.264, H.265, and AV1 at identical CQP across
configurable GOP sizes, decodes via ffmpeg, measures PSNR, and emits
an HTML + console report. Created to investigate AV1 P-frame quality
collapse (PSNR drop and P-size growth toward I-size within a GOP).

Configurable at the CLI (no hardcoded paths or hosts):
  --samples-root PATH  vulkan-video-samples checkout root; encoder
                       binary derived as <root>/build/vk_video_encoder/
                       test/vulkan-video-enc-test.
  --encoder PATH       Override encoder binary path.
  --remote-host HOST   Run encoder over ssh user@host on a GPU machine
                       (default: local). NFS-shared paths assumed.
  --bpp {8,10}         Input/encode bit depth; drives ffmpeg pix_fmt
                       (yuv420p vs yuv420p10le) and encoder --inputBpp.
  --width/--height/--fps/--num-frames/--qp/--gop-sizes/--codecs/
  --output-dir/--report  Standard knobs.

Report contents: codec comparison table, per-frame PSNR + sizes,
side-by-side AV1 vs H.265 size comparison, color-coded PSNR thresholds,
and AV1 quality-collapse detection (flags PSNR spread > 10 dB).

Prereqs: ffmpeg/ffprobe on PATH; built vulkan-video-enc-test.
Signed-off-by: Tony Zlatinski <tzlatinski@nvidia.com>
…eContext

When the encoder (or decoder) fails during early initialization —
e.g., invalid CLI arguments cause EncoderConfig::CreateCodecConfig()
to return before InitVulkanDevice() is called — the VulkanDeviceContext
destructor calls dlclose(m_libHandle) on a null handle (never loaded),
causing a SEGV in _dl_close.

Reproducer:
  vulkan-video-enc-test --codec h264 --encodeUsage streaming ...
  (--encodeUsage is not a valid flag; correct flag is --usageHints)

The encoder's Initialize() fails at arg parsing, but the
VulkanVideoEncoderImpl was already constructed with m_vkDevCtxt as a
by-value member. When the shared_ptr drops, ~VulkanDeviceContext()
calls dlclose(nullptr) → SEGV.

Fix: null-check m_libHandle before dlclose/FreeLibrary on both
Linux and Windows paths.

Signed-off-by: Tony Zlatinski <tzlatinski@nvidia.com>
Print videoUsageHints, videoContentHints, and tuningMode to stderr
when assembling the encode usage info struct. Guarded by --verbose.
This helps verify that the correct tuning mode are actually reaching
the driver.

Signed-off-by: Tony Zlatinski <tzlatinski@nvidia.com>
@zlatinski zlatinski force-pushed the minor-encoder-fixes branch from 67a5e7c to 878f47f Compare April 16, 2026 22:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant