Support wider range of H264 profile-level-id in webrtc#1867
Support wider range of H264 profile-level-id in webrtc#1867wkazmierczak wants to merge 3 commits intomasterfrom
profile-level-id in webrtc#1867Conversation
There was a problem hiding this comment.
Pull request overview
Extends WebRTC H.264 SDP handling to accept a broader set of offered profile-level-id values, and introduces Vulkan-specific filtering so codec entries can be constrained to GPU capabilities when using Vulkan H.264 encode/decode.
Changes:
- Expand advertised H.264 codec parameter variants (
profile-level-idlist) and generate them programmatically. - Rework H.264 offer filtering to “mirror” offer payload types/fmtp into local codec registrations.
- Add a Vulkan capability-based H.264 profile/level filter and wire it into WHIP input and WHEP output paths.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| smelter-core/src/pipeline/webrtc/whip_input/video_preferences.rs | Passes PipelineCtx to allow Vulkan H.264 capability filtering for decoder preferences. |
| smelter-core/src/pipeline/webrtc/whip_input/create_new_session.rs | Updates call site for new video_params_compliant_with_offer(ctx, ...) signature. |
| smelter-core/src/pipeline/webrtc/whep_output/peer_connection.rs | Adds Vulkan H.264 capability filtering for encoder codec registration. |
| smelter-core/src/pipeline/webrtc/supported_codec_parameters.rs | Broadens H.264 profile-level-id coverage and simplifies param generation. |
| smelter-core/src/pipeline/webrtc/mod.rs | Registers the new Vulkan capability filter module. |
| smelter-core/src/pipeline/webrtc/h264_vulkan_capability_filter.rs | New module: filters H.264 codec params based on Vulkan max supported level per profile. |
| smelter-core/src/pipeline/webrtc/h264_offer_filter.rs | Reworks offer filtering to mirror offered H.264 payload types + fmtp constraints. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
smelter-core/src/pipeline/webrtc/whip_input/video_preferences.rs
Outdated
Show resolved
Hide resolved
| video_preferences | ||
| .iter() | ||
| .any(|pref| matches!(pref, VideoDecoderOptions::VulkanH264)) |
There was a problem hiding this comment.
uses_vulkan_h264 enables Vulkan capability filtering if any Vulkan H264 decoder is present in video_preferences. If the preferences include both FfmpegH264 and VulkanH264 (and FFmpeg is ordered first), this will still restrict the advertised/negotiated H264 variants to Vulkan-supported ones, potentially preventing negotiation of profiles that FFmpeg could decode. Consider basing this on the preferred H264 decoder (first occurrence of either H264 option) rather than any().
| video_preferences | |
| .iter() | |
| .any(|pref| matches!(pref, VideoDecoderOptions::VulkanH264)) | |
| // Determine whether the *preferred* H264 decoder is Vulkan by looking at the | |
| // first occurrence of any H264-related preference. | |
| video_preferences | |
| .iter() | |
| .find(|pref| { | |
| matches!( | |
| pref, | |
| VideoDecoderOptions::FfmpegH264 | VideoDecoderOptions::VulkanH264 | |
| ) | |
| }) | |
| .map_or(false, |pref| matches!(pref, VideoDecoderOptions::VulkanH264)) |
smelter-core/src/pipeline/webrtc/whep_output/peer_connection.rs
Outdated
Show resolved
Hide resolved
5b7837f to
5f5ef07
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let mut codec = template.clone(); | ||
|
|
||
| offer_h264_params | ||
| .iter() | ||
| .any(|(offer_plid, offer_pmode)| *offer_plid == plid && *offer_pmode == pmode) | ||
| }) | ||
| .collect() | ||
| codec.payload_type = offer.payload_type; | ||
| codec.capability.sdp_fmtp_line = build_h264_fmtp( | ||
| offer.profile_level_id.as_deref(), | ||
| offer.packetization_mode.as_str(), | ||
| ); | ||
| passthrough.push(codec); |
There was a problem hiding this comment.
This overwrites the registered H264 payload_type with the value from the remote SDP offer. Downstream RTP packetization in this repo sets RTP header payload types explicitly (e.g., Payloader uses PayloaderOptions.payload_type), and parts of the WHEP output path currently initialize payload type to a fixed constant (H264=102). With this change, the negotiated/answered payload type can legitimately differ from 102, so the sender may emit RTP with a payload type that doesn’t match the negotiated SDP. Consider plumbing the negotiated payload type into the payloader (similar to whip_output/setup_track.rs which uses codec_params.payload_type) or otherwise ensuring the RTP payload type matches the negotiated codec.
| let session_description = offer.unmarshal().ok()?; | ||
| let mut h264_payload_types = HashSet::new(); | ||
| let mut params = Vec::new(); | ||
|
|
||
| for md in &session_description.media_descriptions { | ||
| if !md.media_name.media.eq_ignore_ascii_case("video") { | ||
| continue; | ||
| } | ||
|
|
||
| for attr in &md.attributes { | ||
| if !attr.key.eq_ignore_ascii_case("rtpmap") { | ||
| continue; | ||
| } | ||
| let value = attr.value.as_deref().unwrap_or(""); | ||
| let Some((pt, codec_desc)) = value.split_once(' ') else { | ||
| continue; | ||
| }; | ||
| let Some(payload_type) = pt.trim().parse::<u8>().ok() else { | ||
| continue; | ||
| }; | ||
|
|
||
| let codec_name = codec_desc.split('/').next().unwrap_or(""); | ||
| if codec_name.eq_ignore_ascii_case("H264") { | ||
| h264_payload_types.insert(payload_type); | ||
| } | ||
| } |
There was a problem hiding this comment.
h264_payload_types is accumulated across all video media descriptions, but RTP payload types are scoped per media section (m-line). If an SDP offer contains multiple video m-lines that reuse the same payload numbers for different codecs, this cross-media set can cause fmtp lines from one m-line to be incorrectly treated as H264 based on rtpmap from another. Consider computing the H264 payload-type set per-media-description (move the set inside the for md loop) so rtpmap/fmtp matching stays within the same m-line.
a716f09 to
144eb30
Compare
No description provided.