1873801 - Crash in [@ mozilla::WebrtcVideoConduit::OnControlConfigChange]

Status: RESOLVED FIXED

(Core :: WebRTC, defect)

Description

[BugBot [:suhaib / :marco/ :calixte]]

Crash report: https://crash-stats.mozilla.org/report/index/efae2096-8371-4fbf-935d-2dc8b0240108

MOZ_CRASH Reason: MOZ_RELEASE_ASSERT(mEncoderConfig.codec_type != webrtc::VideoCodecType::kVideoCodecGeneric)

Top 10 frames of crashing thread:

0  xul.dll  mozilla::WebrtcVideoConduit::OnControlConfigChange  dom/media/webrtc/libwebrtcglue/VideoConduit.cpp:720
1  xul.dll  mozilla::WatchManager<mozilla::dom::HTMLVideoElement>::PerCallbackWatcher::Notify::<lambda_1>::operator const  xpcom/threads/StateWatching.h:251
1  xul.dll  mozilla::detail::RunnableFunction<`lambda at /builds/worker/workspace/obj-build/dist/include/mozilla/StateWatching.h:248:34'>::Run  xpcom/threads/nsThreadUtils.h:548
2  xul.dll  mozilla::SimpleTaskQueue::DrainTasks  xpcom/threads/TaskDispatcher.h:44
2  xul.dll  mozilla::TaskQueue::DrainDirectTasks  xpcom/threads/TaskQueue.cpp:321
3  xul.dll  mozilla::AutoTaskDispatcher::TaskGroupRunnable::MaybeDrainDirectTasks  xpcom/threads/TaskDispatcher.h:243
3  xul.dll  mozilla::AutoTaskDispatcher::TaskGroupRunnable::Run  xpcom/threads/TaskDispatcher.h:226
4  xul.dll  mozilla::TaskQueueWrapper<1>::CreateTaskRunner::<lambda_1>::operator  dom/media/webrtc/libwebrtcglue/TaskQueueWrapper.h:100
5  xul.dll  mozilla::detail::RunnableFunction<`lambda at /builds/worker/checkouts/gecko/dom/media/webrtc/libwebrtcglue/TaskQueueWrapper.h:93:9'>::Run  xpcom/threads/nsThreadUtils.h:548
6  xul.dll  mozilla::TaskQueue::Runner::Run  xpcom/threads/TaskQueue.cpp:257

By querying Nightly crashes reported within the last 2 months, here are some insights about the signature:

• First crash report: 2023-12-23
• Process type: Content
• Is startup crash: No
• Has user comments: No
• Is null crash: Yes - 2 out of 6 crashes happened on null or near null memory address

Comment 1

[Michael Froman [:mjf]]

Looking at the crash reports, I see crashes going back as far as Sep. 2023, so this isn't a new problem. However, the older issues have mostly been Windows. Now we're seeing crashes on macOS and Linux.

Andreas, any thoughts here?

Comment 2

[Andreas Pehrson [:pehrsons]]

It seems to me we are picking a codec we cannot encode in RTCRtpSender::GetNewVideoConfig. It is the first codec from the list of jsep's negotiated codecs. It seems like red and ulpfec are let through there, could they be appearing first in the list?

Comment 3

[Byron Campen [:bwc]]

That seems plausible. Should be fairly easy to write a test for.

Comment 4

[Byron Campen [:bwc]]

I have verified that putting the ulpfec payload type first in a remote offer hits this assertion. There are probably several other variants (answers, red, etc). I will write some more tests, and then a fix.

Comment 5

[Byron Campen [:bwc]]

https://treeherder.mozilla.org/jobs?repo=try&revision=db98f21354e2003423709afc2428332f60314783
https://treeherder.mozilla.org/jobs?repo=try&revision=1fe29c6ca689c12ad5c2a3b734f0728330faf323
https://treeherder.mozilla.org/jobs?repo=try&revision=2cf11bd5c5d0f2926e4cc76d38b2a84525a75f14

Comment 6

[Byron Campen [:bwc]]

Attachment: Bug 1873801: Test cases for bug. r?mjf

Comment 7

[Byron Campen [:bwc]]

Attachment: Bug 1873801: Make sure pseudo codecs are after real codecs in priority order. r?mjf

Depends on D199784

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/33f90f00e8e9
Test cases for bug. r=mjf
https://hg.mozilla.org/integration/autoland/rev/37e780bf8eff
Make sure pseudo codecs are after real codecs in priority order. r=mjf

Comment 9

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/33f90f00e8e9
https://hg.mozilla.org/mozilla-central/rev/37e780bf8eff

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:bwc, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox123 to wontfix.

For more information, please visit BugBot documentation.

Comment 11

[Byron Campen [:bwc]]

There's a small chance that this will upset some middlebox software, since we're putting codecs in a different order on the m-line (this is valid, but poorly coded implementations might want a hard-coded order or something), but I think it would probably be worth it to uplift.

Comment 12

[Byron Campen [:bwc]]

Comment on attachment 9376755 [details]
Bug 1873801: Make sure pseudo codecs are after real codecs in priority order. r?mjf

Beta/Release Uplift Approval Request

• User impact if declined: Some webrtc services will cause content process crashes.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The risk here is that some webrtc service might have invalid hard-coded expectations about the SDP we emit, which has changed a little.
• String changes made/needed: None
• Is Android affected?: Yes

Comment 13

[Pascal Chevrel:pascalc]

Comment on attachment 9376754 [details]
Bug 1873801: Test cases for bug. r?mjf

Approved for 123 beta 8, thanks.

Comment 14

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/ffc398a8382b
https://hg.mozilla.org/releases/mozilla-beta/rev/d2989996f577

Comment 15

[Pascal Chevrel:pascalc]

https://hg.mozilla.org/releases/mozilla-beta/rev/ffc398a8382b
https://hg.mozilla.org/releases/mozilla-beta/rev/d2989996f577