Only advertise internal H264 decode formats if the decoder works - #1313
Conversation
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>
Changeset ✓This PR includes a changeset covering all affected packages:
|
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(); |
There was a problem hiding this comment.
During construction of the factory check if at runtime there is a h264 decoder present from this lib at runtime.
There was a problem hiding this comment.
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_) { |
There was a problem hiding this comment.
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) && |
There was a problem hiding this comment.
If at runtime for whatever reason we try to create the decoder and it is not supported, error
xianshijing-lk
left a comment
There was a problem hiding this comment.
lgtm, some minor comments.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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