webrtc icon indicating copy to clipboard operation
webrtc copied to clipboard

Match header extensions to all media sections in offer

Open ellenfkh opened this issue 2 years ago • 1 comments

Description

We noticed that once pion has negotiated headers and codecs for a given Kind, when a subsequent offer SDP adds a media section for that same Kind with different header extensions, the generated answer uses the original headers for all media sections.

This doesn't happen in Chrome, which never seems to send mixed headers like this. Firefox does, though.

We ran into this when sending an offer in which the first video media section had simulcast disabled, and thus didn't set this attribute

a=extmap:8/sendonly urn:ietf:params:rtp-hdrext:sdes:rtp-stream-id

but the second video media section had simulcast enabled, so included that header.

The resulting answer didn't include that header in either media section, but the second media section did correctly have these attributes

a=rid:f recv
a=rid:h recv
a=rid:q recv
a=simulcast:recv f;h;q

Resulting in this error

Incoming unhandled RTP ssrc(3870936274), OnTrack will not be fired. stream id RTP Extensions required for Simulcast

This patch fixes our manual test case, but I suspect there are more places in the code where this assumption is being made. I'm also not sure whether Plan B vs Unified expects different behavior in this case.

Reference issue

Fixes #...

ellenfkh avatar Mar 31 '23 20:03 ellenfkh

Codecov Report

Attention: Patch coverage is 77.41935% with 7 lines in your changes missing coverage. Please review.

Project coverage is 79.08%. Comparing base (166d82e) to head (40a80e3).

Files Patch % Lines
mediaengine.go 73.07% 3 Missing and 4 partials :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2444      +/-   ##
==========================================
- Coverage   79.11%   79.08%   -0.04%     
==========================================
  Files          89       89              
  Lines        8249     8270      +21     
==========================================
+ Hits         6526     6540      +14     
- Misses       1256     1260       +4     
- Partials      467      470       +3     
Flag Coverage Δ
go 80.69% <77.41%> (-0.04%) :arrow_down:
wasm 64.98% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Sep 28 '23 20:09 codecov[bot]

cc @boks1971 as you had a related pr #2107 for same issue

cnderrauber avatar Jul 18 '24 04:07 cnderrauber