Closed Bug 1953985 Opened 10 months ago Closed 9 months ago

GetCapabilities does not return codecs with identical configuration to what is offered by default

Categories

(Core :: WebRTC, defect, P2)

defect

Tracking

()

RESOLVED FIXED
139 Branch
Tracking Status
firefox139 --- fixed

People

(Reporter: ng, Assigned: ng)

References

Details

Attachments

(1 file)

GetCapabilities returns a minimally configured set of codecs which do not in all cases match what we offer by default. This can cause unwanted behavior with SetParameters.

Here is where we define the opus codec description: https://searchfox.org/mozilla-central/source/dom/media/webrtc/jsep/JsepCodecDescription.h#210 note we don't take any prefs into account here
That in turn through a couple of hops is called from here: https://searchfox.org/mozilla-central/source/dom/media/webrtc/jsapi/RTCRtpTransceiver.cpp#1064 note none of the hops alter the description
It is also called fromGetCapabilities here: https://searchfox.org/mozilla-central/source/dom/media/webrtc/jsapi/PeerConnectionImpl.cpp#2301
It is also called from SetupPreferredCodecs here: https://searchfox.org/mozilla-central/source/dom/media/webrtc/jsapi/PeerConnectionImpl.cpp#2368
FEC is enabled in ConfigureCodec here: https://searchfox.org/mozilla-central/source/dom/media/webrtc/jsapi/PeerConnectionImpl.cpp#721
Which is in turn called by ConfigureJsepSessionCodecs here https://searchfox.org/mozilla-central/source/dom/media/webrtc/jsapi/PeerConnectionImpl.cpp#824
That is called in AddRtpTransceiverToJsepSession, CreateOffer, and SetRemoteDescription: https://searchfox.org/mozilla-central/search?q=symbol:_ZN7mozilla18PeerConnectionImpl26ConfigureJsepSessionCodecsEv&redirect=false
note so it looks like ConfigureCodec is not called in the GetCapabilities case, so the fmtp attributes can be different between our "capabilities" and what we actually offer, which seems wrong.
This test is failing: https://searchfox.org/mozilla-central/source/testing/web-platform/tests/webrtc/RTCRtpParameters-codec.html#445
It calls getCapabilities in a helper function here: https://searchfox.org/mozilla-central/source/testing/web-platform/tests/webrtc/RTCRtpParameters-codec.html#445

The following hack gets it to pass the codec comparison in setParameters (it has other issues preventing the test from passing later):

  static UniquePtr<JsepAudioCodecDescription> CreateDefaultOpus() {
    // Per jmspeex on IRC:
    // For 32KHz sampling, 28 is ok, 32 is good, 40 should be really good
    // quality.  Note that 1-2Kbps will be wasted on a stereo Opus channel
    // with mono input compared to configuring it for mono.
    // If we reduce bitrate enough Opus will low-pass us; 16000 will kill a
    // 9KHz tone.  This should be adaptive when we're at the low-end of video
    // bandwidth (say <100Kbps), and if we're audio-only, down to 8 or
    // 12Kbps.
    auto opus = MakeUnique<JsepAudioCodecDescription>("109", "opus", 48000, 2);
    if (Preferences::GetBool("media.navigator.audio.use_fec", false)) {
      opus->mFECEnabled = true;
    }
    return opus;
  }

Here is the offer without the hack:

v=0
o=mozilla...THIS_IS_SDPARTA-99.0 4108676482644259273 0 IN IP4 0.0.0.0
s=-
t=0 0
a=sendrecv
a=fingerprint:sha-256 B4:08:4C:E4:B4:45:31:26:F3:EC:3F:1A:E9:D4:BB:B4:D8:A5:44:8D:D9:98:26:25:78:7A:E9:26:1D:C2:4C:D0
a=group:BUNDLE 0
a=ice-options:trickle
a=msid-semantic:WMS *
m=audio 9 UDP/TLS/RTP/SAVPF 109 9 0 8 101
c=IN IP4 0.0.0.0
a=sendrecv
a=extmap:1 urn:ietf:params:rtp-hdrext:ssrc-audio-level
a=extmap:2/recvonly urn:ietf:params:rtp-hdrext:csrc-audio-level
a=extmap:3 urn:ietf:params:rtp-hdrext:sdes:mid
a=fmtp:109 maxplaybackrate=48000;stereo=1;useinbandfec=1
a=fmtp:101 0-15
a=ice-pwd:98b23bf3a40818511b2388200b39380f
a=ice-ufrag:fefdac95
a=mid:0
a=msid:- {7a90a0d0-e24f-4150-82fa-1e1235a85eb2}
a=rtcp-mux
a=rtpmap:109 opus/48000/2
a=rtpmap:9 G722/8000/1
a=rtpmap:0 PCMU/8000
a=rtpmap:8 PCMA/8000
a=rtpmap:101 telephone-event/8000
a=setup:actpass
a=ssrc:3463498437 cname:{cf6f8ca3-7e15-4ede-8b49-28c951ebd13c}

And here is the answer:

v=0
o=mozilla...THIS_IS_SDPARTA-99.0 2311277663883584802 0 IN IP4 0.0.0.0
s=-
t=0 0
a=fingerprint:sha-256 CB:3E:A8:B9:2D:CB:08:0E:B6:96:BB:E6:5A:E9:24:D8:64:7B:19:7F:A4:FA:F0:56:95:42:D0:15:8A:A4:DF:73
a=group:BUNDLE 0
a=ice-options:trickle
a=msid-semantic:WMS *
m=audio 9 UDP/TLS/RTP/SAVPF 9 0 8 101 109
c=IN IP4 0.0.0.0
a=recvonly
a=extmap:1 urn:ietf:params:rtp-hdrext:ssrc-audio-level
a=extmap:3 urn:ietf:params:rtp-hdrext:sdes:mid
a=fmtp:101 0-15
a=fmtp:109 maxplaybackrate=48000;stereo=1;useinbandfec=0
a=ice-pwd:f75195b117fbca9f869a1cf27f5bb6aa
a=ice-ufrag:cda6cbcb
a=mid:0
a=rtcp-mux
a=rtpmap:9 G722/8000/1
a=rtpmap:0 PCMU/8000
a=rtpmap:8 PCMA/8000
a=rtpmap:101 telephone-event/8000/1
a=rtpmap:109 opus/48000/2
a=setup:active
a=ssrc:1094725644 cname:{9372eadb-597a-4a53-9504-1bb92d01576f}

note a=fmtp:109 maxplaybackrate=48000;stereo=1;useinbandfec=1 vs a=fmtp:109 maxplaybackrate=48000;stereo=1;useinbandfec=0

Assignee: nobody → ngrunbaum
Attachment #9472616 - Attachment description: WIP: Bug 1953985 - apply codec prefs to all Jsep codecs;r=bwc!,r!dbaker! → Bug 1953985 - apply codec prefs to all Jsep codecs;r=bwc!,dbaker!
Status: NEW → ASSIGNED
Attachment #9472616 - Attachment description: Bug 1953985 - apply codec prefs to all Jsep codecs;r=bwc!,dbaker! → WIP: Bug 1953985 - apply codec prefs to all Jsep codecs;r=bwc!,dbaker!
Attachment #9472616 - Attachment description: WIP: Bug 1953985 - apply codec prefs to all Jsep codecs;r=bwc!,dbaker! → WIP: Bug 1953985 - apply codec prefs to all Jsep codecs;r=bwc!,dbaker!,pehrsons!
Attachment #9472616 - Attachment description: WIP: Bug 1953985 - apply codec prefs to all Jsep codecs;r=bwc!,dbaker!,pehrsons! → Bug 1953985 - apply codec prefs to all Jsep codecs;r=bwc!,dbaker!,pehrsons!
Pushed by na-g@nostrum.com: https://hg.mozilla.org/integration/autoland/rev/e8b5ce201f6c apply codec prefs to all Jsep codecs;r=bwc,dbaker,pehrsons
Status: ASSIGNED → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → 139 Branch
QA Whiteboard: [qa-triage-done-c140/b139]
Depends on: 1991365
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: