Skip to content

Only advertise internal H264 decode formats if the decoder works - #1313

Merged
MaxHeimbrock merged 4 commits into
mainfrom
max/build-supported-ffmpeg-codec-list-at-runtime
Aug 5, 2026
Merged

Only advertise internal H264 decode formats if the decoder works#1313
MaxHeimbrock merged 4 commits into
mainfrom
max/build-supported-ffmpeg-codec-list-at-runtime

Conversation

@MaxHeimbrock

@MaxHeimbrock MaxHeimbrock commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

GetSupportedFormats() unconditionally appended SupportedH264DecoderCodecs(), so desktop clients claimed 42001f/42e01f decode support in their SDP even though the internal FFmpeg decoder cannot initialize on desktop prebuilts (H.264 codec not compiled in). The SFU then selected Baseline for those subscribers, producing undecodable streams.

H264Decoder::IsSupported() only reflects the WEBRTC_USE_H264 build flag — the missing FFmpeg codec surfaces first when Configure() fails at runtime. Probe Create()+Configure() once at factory construction and advertise the internal formats only when the probe succeeds. Platform-factory formats (VideoToolbox, MediaCodec, NVDEC) are unaffected.

Also gate the internal H264Decoder::Create() fallback in Create() on the same probe, so a format the platform factories reject yields a clear "No VideoDecoder found" error instead of a decoder that fails Configure() at runtime.

Solving issue description client side

GetSupportedFormats() unconditionally appended
SupportedH264DecoderCodecs(), so desktop clients claimed 42001f/42e01f
decode support in their SDP even though the internal FFmpeg decoder
cannot initialize on desktop prebuilts (H.264 codec not compiled in).
The SFU then selected Baseline for those subscribers, producing
undecodable streams.

H264Decoder::IsSupported() only reflects the WEBRTC_USE_H264 build
flag — the missing FFmpeg codec surfaces first when Configure() fails
at runtime. Probe Create()+Configure() once at factory construction
and advertise the internal formats only when the probe succeeds.
Platform-factory formats (VideoToolbox, MediaCodec, NVDEC) are
unaffected.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Changeset ✓

This PR includes a changeset covering all affected packages:

Package Bump
libwebrtc patch
livekit patch
livekit-ffi patch
webrtc-sys patch

MaxHeimbrock and others added 2 commits August 4, 2026 13:58
GetSupportedFormats() no longer advertises the internal H264 formats
when the probe fails, but Create() still handed back the broken FFmpeg
decoder when the platform factories reject a format (e.g. a
profile-level-id VideoToolbox does not match). Gate the fallback on the
same probe so that path returns nullptr with a clear "No VideoDecoder
found" error instead of a decoder that fails Configure() at runtime.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
}
#endif

internal_h264_decoder_works_ = InternalH264DecoderWorks();

@MaxHeimbrock MaxHeimbrock Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

During construction of the factory check if at runtime there is a h264 decoder present from this lib at runtime.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Runtime costs for this new code:

  • On the broken desktop builds (the case that matters): the H264DecoderImpl constructor is trivial member initialization. Configure() allocates one AVCodecContext (a few KB), sets fields, then avcodec_find_decoder() — a lookup in FFmpeg's registered-codec table — returns null, and the failure path immediately calls Release(), freeing the context. Microseconds of latency, a few KB of transient heap.

  • On builds where FFmpeg does have H.264: it additionally runs avcodec_open2(), which initializes the decoder's private context and tables — roughly single-digit milliseconds and some transient allocations. Frame buffers are not allocated here; those come lazily via get_buffer2 during actual decoding. The probe's unique_ptr destructor then runs Release() and everything is freed before InternalH264DecoderWorks() returns.

for (const webrtc::SdpVideoFormat& h264_format :
webrtc::SupportedH264DecoderCodecs())
formats.push_back(h264_format);
if (internal_h264_decoder_works_) {

@MaxHeimbrock MaxHeimbrock Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Filter the supported formats based on if a working h264 decoder is present in this lib at runtime.

if (absl::EqualsIgnoreCase(format.name, webrtc::kVp9CodecName))
return webrtc::VP9Decoder::Create();
if (absl::EqualsIgnoreCase(format.name, webrtc::kH264CodecName))
if (absl::EqualsIgnoreCase(format.name, webrtc::kH264CodecName) &&

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If at runtime for whatever reason we try to create the decoder and it is not supported, error

@MaxHeimbrock
MaxHeimbrock marked this pull request as ready for review August 4, 2026 13:09
@MaxHeimbrock
MaxHeimbrock requested a review from ladvoc as a code owner August 4, 2026 13:09

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 2 additional findings.

Open in Devin Review

@xianshijing-lk xianshijing-lk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, some minor comments.

Comment thread webrtc-sys/include/livekit/video_decoder_factory.h Outdated
Comment thread webrtc-sys/src/video_decoder_factory.cpp
Comment thread webrtc-sys/src/video_decoder_factory.cpp Outdated
Comment thread webrtc-sys/src/video_decoder_factory.cpp
Comment thread webrtc-sys/src/video_decoder_factory.cpp
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@MaxHeimbrock
MaxHeimbrock merged commit 0aea78f into main Aug 5, 2026
23 checks passed
@MaxHeimbrock
MaxHeimbrock deleted the max/build-supported-ffmpeg-codec-list-at-runtime branch August 5, 2026 08:16
@knope-bot knope-bot Bot mentioned this pull request Aug 5, 2026
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.

2 participants