webrtc icon indicating copy to clipboard operation
webrtc copied to clipboard

Turn error into a warning when SSRCs are included when they don't need to be

Open rlpeck opened this issue 3 years ago • 1 comments

Description

I ran into an issue when testing that exposed what I believe is a bug where pion can't consume simulcast streams that it creates. When pion populates the SDP with sender information it includes an SSRC attribute for each encoding. This conflicts with handleUndeclaredSSRC behavior which returns an error if it encounters an a=ssrc... attribute in the media section.

The comment around this functionality implies that this is not prohibited, just discouraged it seems reasonable to log an error when encountering this rather than returning an error

If the remote SDP was only one media section the ssrc doesn't have to be explicitly declared

Reference issue

Fixes #...

rlpeck avatar Sep 12 '22 21:09 rlpeck

Codecov Report

Base: 77.34% // Head: 77.22% // Decreases project coverage by -0.11% :warning:

Coverage data is based on head (5fb9251) compared to base (4e85358). Patch coverage: 0.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2299      +/-   ##
==========================================
- Coverage   77.34%   77.22%   -0.12%     
==========================================
  Files          87       87              
  Lines        9197     9197              
==========================================
- Hits         7113     7102      -11     
- Misses       1651     1659       +8     
- Partials      433      436       +3     
Flag Coverage Δ
go 78.97% <0.00%> (-0.14%) :arrow_down:
wasm 70.16% <ø> (ø)

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

Impacted Files Coverage Δ
peerconnection.go 77.41% <0.00%> (-0.18%) :arrow_down:
internal/mux/endpoint.go 51.72% <0.00%> (-6.90%) :arrow_down:
dtlstransport.go 63.04% <0.00%> (-1.87%) :arrow_down:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

codecov[bot] avatar Sep 12 '22 21:09 codecov[bot]

Sorry I missed this @rlpeck !

Pion would also incorrectly not handle the Simulcast streams. I have addressed/fixed this with d08b3dc40385d9da6cecd092d894480903183ee5

Sean-Der avatar Apr 24 '23 01:04 Sean-Der