825106 - Media negotiation fails when ISAC is present and ahead of Opus

Status: RESOLVED FIXED

(Core :: WebRTC: Signaling, defect, P1)

Description

[Eric Rescorla (:ekr)]

The following offer/answer pair is produced by the attached patch.

Note that we pick PCMU even though Opus is present. 

To make matters worse, when we go to configure the Conduit, we give it what appears to be a
bogus rate parameter:


(gdb) bt
#0  mozilla::WebrtcAudioConduit::ConfigureRecvMediaCodecs (this=0x10c62df20, codecConfigList=@0x10d4a9a58) at AudioConduit.cpp:315
#1  0x000000010006b7c7 in vcmRxStartICE_m (mcap_id=0, group_id=0, stream_id=3, level=1, pc_stream_id=0, pc_track_id=0, call_handle=131074, peerconnection=0x10d25e3cc "f6e3354055ad9689", num_payloads=1, payloads=0x10d255240, fingerprint_alg=0x1010956d0 "sha-256", fingerprint=0x1010956da "A8:76:8C:4C:FA:2E:67:D7:F8:1D:28:4E:90:24:04:12:EB:B4:A6:69:3D:05:92:E4:91:C3:EA:F9:B7:54:D3:09", attrs=0x10d1bc858) at VcmSIPCCBinding.cpp:1329
#2  0x0000000100070415 in mozilla::runnable_args_nm_13_ret<int (*)(unsigned short, unsigned short, unsigned short, int, int, int, unsigned int, char const*, int, vcm_payload_info_t const*, char const*, char const*, vcm_attrs_t_*), unsigned short, unsigned short, unsigned short, int, int, int, unsigned int, char const*, int, vcm_payload_info_t const*, char const*, char const*, vcm_attrs_t_*, int>::Run (this=0x10c62deb0) at runnable_utils_generated.h:1291
#3  0x00000001042c63c2 in nsThreadSyncDispatch::Run (this=0x10d29f280) at /Users/ekr/dev/mozilla-inbound/xpcom/threads/nsThread.cpp:774
#4  0x00000001042c5a56 in nsThread::ProcessNextEvent (this=0x10c62b4a0, mayWait=true, result=0x10d4a9dde) at /Users/ekr/dev/mozilla-inbound/xpcom/threads/nsThread.cpp:627
#5  0x000000010422a0b9 in NS_ProcessNextEvent_P (thread=0x10c62b4a0, mayWait=true) at nsThreadUtils.cpp:237
#6  0x00000001042c4487 in nsThread::ThreadFunc (arg=0x10c62b4a0) at /Users/ekr/dev/mozilla-inbound/xpcom/threads/nsThread.cpp:265
#7  0x000000010bd6d465 in _pt_root (arg=0x10c6108c0) at /Users/ekr/dev/mozilla-inbound/nsprpub/pr/src/pthreads/ptthread.c:156
#8  0x00007fff85485fd6 in _pthread_start ()
#9  0x00007fff85485e89 in thread_start ()
(gdb) p codecConfigList[0]
/Users/ekr/dev/mozilla-inbound/media/webrtc/signaling/test/signaling_unittests.cpp:665: Failure
Value of: res
  Actual: false
Expected: true
$2 = (const_reference) @0x10c6034a8: 0x10e01dc80
(gdb) p *codecConfigList[0]
$3 = {
  mType = 104, 
  mName = {
    _M_dataplus = {
      <std::allocator<char>> = {
        <__gnu_cxx::new_allocator<char>> = {<No data fields>}, <No data fields>}, 
      members of std::basic_string<char>::_Alloc_hider: 
      _M_p = 0x10e01dcb8 "PCMU"
    }, 
    static npos = 18446744073709551615
  }, 
  mFreq = 32000, 
  mPacSize = 640, 
  mChannels = 1, 
  mRate = 256000
}
(gdb) 

v=0
o=- 3835809413 2 IN IP4 127.0.0.1
s=-
t=0 0
a=group:BUNDLE audio video
a=msid-semantic: WMS ahheYQXHFU52slYMrWNtKUyHCtWZsOJgjlOH
m=audio 1 RTP/SAVPF 103 104 111 0 8 107 106 105 13 126
c=IN IP4 1.1.1.1
a=rtcp:1 IN IP4 1.1.1.1
a=ice-ufrag:jz9UBk9RT8eCQXiL
a=ice-pwd:iscXxsdU+0gracg0g5D45orx
a=ice-options:google-ice
a=fingerprint:sha-256 A8:76:8C:4C:FA:2E:67:D7:F8:1D:28:4E:90:24:04:12:EB:B4:A6:69:3D:05:92:E4:91:C3:EA:F9:B7:54:D3:09
a=sendrecv
a=mid:audio
a=rtcp-mux
a=crypto:1 AES_CM_128_HMAC_SHA1_80 inline:/he/v44FKu/QvEhex86zV0pdn2V4Y7wB2xaZ8eUy
a=rtpmap:103 ISAC/16000
a=rtpmap:104 ISAC/32000
a=rtpmap:111 opus/48000/2
a=rtpmap:0 PCMU/8000
a=rtpmap:8 PCMA/8000
a=rtpmap:107 CN/48000
a=rtpmap:106 CN/32000
a=rtpmap:105 CN/16000
a=rtpmap:13 CN/8000
a=rtpmap:126 telephone-event/8000
a=ssrc:3389377748 cname:G5I+Jxz4rcaq8IIK
a=ssrc:3389377748 msid:ahheYQXHFU52slYMrWNtKUyHCtWZsOJgjlOH a0
a=ssrc:3389377748 mslabel:ahheYQXHFU52slYMrWNtKUyHCtWZsOJgjlOH
a=ssrc:3389377748 label:ahheYQXHFU52slYMrWNtKUyHCtWZsOJgjlOHa0
m=video 1 RTP/SAVPF 100 116 117
c=IN IP4 1.1.1.1
a=rtcp:1 IN IP4 1.1.1.1
a=ice-ufrag:jz9UBk9RT8eCQXiL
a=ice-pwd:iscXxsdU+0gracg0g5D45orx
a=ice-options:google-ice
a=fingerprint:sha-256 A8:76:8C:4C:FA:2E:67:D7:F8:1D:28:4E:90:24:04:12:EB:B4:A6:69:3D:05:92:E4:91:C3:EA:F9:B7:54:D3:09
a=sendrecv
a=mid:video
a=rtcp-mux
a=crypto:1 AES_CM_128_HMAC_SHA1_80 inline:/he/v44FKu/QvEhex86zV0pdn2V4Y7wB2xaZ8eUy
a=rtpmap:100 VP8/90000
a=rtpmap:116 red/90000
a=rtpmap:117 ulpfec/90000
a=ssrc:3613537198 cname:G5I+Jxz4rcaq8IIK
a=ssrc:3613537198 msid:ahheYQXHFU52slYMrWNtKUyHCtWZsOJgjlOH v0
a=ssrc:3613537198 mslabel:ahheYQXHFU52slYMrWNtKUyHCtWZsOJgjlOH
a=ssrc:3613537198 label:ahheYQXHFU52slYMrWNtKUyHCtWZsOJgjlOHv0

OnAddStream called hints=1 type=audio thread=0x10c6108c0
OnAddStream called hints=2 type=video thread=0x10c6108c0
onSetRemoteDescriptionSuccess = 0
Creating answer:
onCreateAnswerSuccess = v=0
o=Mozilla-SIPUA 21605 0 IN IP4 0.0.0.0
s=SIP Call
t=0 0
a=ice-ufrag:ff3abf20
a=ice-pwd:b28e8dfb697ac805d6d014975d990e68
a=fingerprint:sha-256 2B:C4:D1:24:9A:64:2B:22:64:E3:E7:5B:4A:B5:F6:9D:38:A2:E0:36:CE:75:83:08:0E:DE:25:FF:65:0F:C6:97
m=audio 61089 RTP/SAVPF 104 126
c=IN IP4 74.95.2.169
a=rtpmap:104 PCMU/8000
a=rtpmap:126 telephone-event/8000
a=fmtp:126 0-15
a=sendrecv
a=candidate:0 1 UDP 2113601791 192.168.1.100 61089 typ host
a=candidate:1 1 UDP 1694236671 74.95.2.169 61089 typ srflx raddr 192.168.1.100 rport 61089
a=candidate:0 2 UDP 2113601790 192.168.1.100 61180 typ host
a=candidate:1 2 UDP 1694236670 74.95.2.169 61180 typ srflx raddr 192.168.1.100 rport 61180
m=video 64958 RTP/SAVPF 100
c=IN IP4 74.95.2.169
a=rtpmap:100 VP8/90000
a=recvonly
a=candidate:0 1 UDP 2113601791 192.168.1.100 64958 typ host
a=candidate:1 1 UDP 1694236671 74.95.2.169 64958 typ srflx raddr 192.168.1.100 rport 64958
a=candidate:0 2 UDP 2113601790 192.168.1.100 54402 typ host
a=candidate:1 2 UDP 1694236670 74.95.2.169 54402 typ srflx raddr 192.168.1.100 rport 54402

Comment 1

[Eric Rescorla (:ekr)]

Attachment: Test chrome audio

Comment 2

[Adam Roach [:abr]]

The bogus rate parameter (256000) is because we're using a bogus frequency parameter (32000). For G.711, rate = 8 * frequency (all samples occupy 8 bits).

The 32000, in turn, appears to be coming from the second ISAC line ("a=rtpmap:104 ISAC/32000") -- as does the payload type (104). I'm about to dig into the code, but my first pass guess is that we are somehow incorrectly considering the second ISAC line to represent PCMU somehow. I'm digging into this now.

Comment 3

[Adam Roach [:abr]]

Attachment: Use correct pointer comparison when determining master

Comment 4

[Adam Roach [:abr]]

When I split the remote information into two arrays (rather than one array with two 7-bit values masked into an integer), I botched the logic to determine which index to use here.

Comment 5

[Randell Jesup [:jesup] (needinfo me)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/255428fcf589

Comment 6

[Graeme McCutcheon [:graememcc]]

https://hg.mozilla.org/mozilla-central/rev/255428fcf589