1237224 - Crash in mozilla::WebrtcVideoConduit::SelectSendFrameRate after re-negotiating H.264 stream with max-mbps capability

Status: RESOLVED FIXED

(Core :: WebRTC: Audio/Video, defect, P1)

Description

[solmaks]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_1) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/47.0.2526.106 Safari/537.36

Steps to reproduce:

Firefox crashes after series of subsequent re-negotiations in case H.264 has been negotiated and constrained by max-mbps receiver capability.

1. Create PC1 and PC2
2. Let PC2 simulate remote endpoint supporting H.264 with non-interleaved packetization mode only
3. Let PC2 specify max-mbps H.264 receiver capability
4. Add audio and video tracks to both PC1 and PC2
5. Negotiate bidirectional A/V m-lines between PC1 and PC2
6. Remove video track from PC1, re-negotiate unidirectional video m-line
7. Add video track back to PC1, re-negotiate bidirectional video m-line

Can be reproduced by running the following in Nightly 46 - https://jsfiddle.net/9md05c03/


Actual results:

After re-negotiating bidirectional video Firefox crashes with EXC_ARITHMETIC, full backtrace below:

(lldb) bt
* thread #9: tid = 0x6369, 0x00000001028f07d0 XUL`mozilla::WebrtcVideoConduit::SelectSendResolution(unsigned short, unsigned short, webrtc::I420VideoFrame*) + 71 at VideoConduit.cpp:1249, name = 'Socket Thread', stop reason = EXC_ARITHMETIC (code=EXC_I386_DIV, subcode=0x0)
    frame #0: 0x00000001028f07d0 XUL`mozilla::WebrtcVideoConduit::SelectSendResolution(unsigned short, unsigned short, webrtc::I420VideoFrame*) + 71 at VideoConduit.cpp:1249
    frame #1: 0x00000001028f0789 XUL`mozilla::WebrtcVideoConduit::SelectSendResolution(this=0x000000013e171bb0, width=<unavailable>, height=<unavailable>, frame=0x0000000000000000) + 489 at VideoConduit.cpp:1137
  * frame #2: 0x00000001028f0529 XUL`mozilla::WebrtcVideoConduit::GetVideoEncoderStats(this=0x000000013e171bb0, framerateMean=0x00007000003a5778, framerateStdDev=<unavailable>, bitrateMean=<unavailable>, bitrateStdDev=<unavailable>, droppedFrames=<unavailable>) + 345 at VideoConduit.cpp:185
    frame #3: 0x000000010293598f XUL`mozilla::PeerConnectionImpl::ExecuteStatsQuery_s(query=0x00000001434c3f10) + 3615 at PeerConnectionImpl.cpp:3304
    frame #4: 0x0000000102923d26 XUL`mozilla::EverySecondTelemetryCallback_s(aQueryList=nsAutoPtr<mozilla::Vector<nsAutoPtr<mozilla::RTCStatsQuery>, 0, mozilla::MallocAllocPolicy> > @ 0x00007000003a5af0) + 214 at PeerConnectionCtx.cpp:206
    frame #5: 0x0000000102925b30 XUL`mozilla::runnable_args_func<void (*)(nsAutoPtr<mozilla::Vector<nsAutoPtr<mozilla::RTCStatsQuery>, 0ul, mozilla::MallocAllocPolicy> >), nsAutoPtr<mozilla::Vector<nsAutoPtr<mozilla::RTCStatsQuery>, 0ul, mozilla::MallocAllocPolicy> > >::Run() [inlined] void mozilla::detail::RunnableFunctionCallHelper<void>::apply<void (func=<unavailable>)(nsAutoPtr<mozilla::Vector<nsAutoPtr<mozilla::RTCStatsQuery>, 0ul, mozilla::MallocAllocPolicy> >), nsAutoPtr<mozilla::Vector<nsAutoPtr<mozilla::RTCStatsQuery>, 0ul, mozilla::MallocAllocPolicy> >, 0ul>(void (*)(nsAutoPtr<mozilla::Vector<nsAutoPtr<mozilla::RTCStatsQuery>, 0ul, mozilla::MallocAllocPolicy> >), mozilla::Tuple<nsAutoPtr<mozilla::Vector<nsAutoPtr<mozilla::RTCStatsQuery>, 0ul, mozilla::MallocAllocPolicy> > >&, mozilla::IndexSequence<0ul>) + 22 at runnable_utils.h:79
    frame #6: 0x0000000102925b1a XUL`mozilla::runnable_args_func<void (*)(nsAutoPtr<mozilla::Vector<nsAutoPtr<mozilla::RTCStatsQuery>, 0ul, mozilla::MallocAllocPolicy> >), nsAutoPtr<mozilla::Vector<nsAutoPtr<mozilla::RTCStatsQuery>, 0ul, mozilla::MallocAllocPolicy> > >::Run(this=<unavailable>) + 10 at runnable_utils.h:118
    frame #7: 0x0000000101ea3147 XUL`nsThread::ProcessNextEvent(this=0x000000010072cb00, aMayWait=<unavailable>, aResult=0x00007000003a5c07) + 1495 at nsThread.cpp:989
    frame #8: 0x0000000101ee2653 XUL`NS_ProcessNextEvent(aThread=<unavailable>, aMayWait=true) + 51 at nsThreadUtils.cpp:297
    frame #9: 0x0000000101fb4250 XUL`nsSocketTransportService::Run(this=0x0000000100727640) + 960 at nsSocketTransportService2.cpp:905
    frame #10: 0x0000000101fb4dfd XUL`non-virtual thunk to nsSocketTransportService::Run(this=<unavailable>) + 13 at nsSocketTransportService2.cpp:793
    frame #11: 0x0000000101ea3147 XUL`nsThread::ProcessNextEvent(this=0x000000010072cb00, aMayWait=<unavailable>, aResult=0x00007000003a5dd7) + 1495 at nsThread.cpp:989
    frame #12: 0x0000000101ee2653 XUL`NS_ProcessNextEvent(aThread=<unavailable>, aMayWait=false) + 51 at nsThreadUtils.cpp:297
    frame #13: 0x00000001022c2acc XUL`mozilla::ipc::MessagePumpForNonMainThreads::Run(this=0x0000000111095e80, aDelegate=0x00000001007d44e0) + 284 at MessagePump.cpp:326
    frame #14: 0x000000010228592c XUL`MessageLoop::Run() [inlined] MessageLoop::RunHandler(this=<unavailable>) + 60 at message_loop.cc:227
    frame #15: 0x0000000102285927 XUL`MessageLoop::Run(this=<unavailable>) + 55 at message_loop.cc:201
    frame #16: 0x0000000101ea1033 XUL`nsThread::ThreadFunc(aArg=0x000000010072cb00) + 291 at nsThread.cpp:401
    frame #17: 0x0000000101b3be79 libnss3.dylib`_pt_root(arg=0x00000001007349f0) + 281 at ptthread.c:212
    frame #18: 0x00007fff90f0a9b1 libsystem_pthread.dylib`_pthread_body + 131
    frame #19: 0x00007fff90f0a92e libsystem_pthread.dylib`_pthread_start + 168
    frame #20: 0x00007fff90f08385 libsystem_pthread.dylib`thread_start + 13
(lldb) frame select 0
frame #0: 0x00000001028f07d0 XUL`mozilla::WebrtcVideoConduit::SelectSendResolution(unsigned short, unsigned short, webrtc::I420VideoFrame*) + 71 at VideoConduit.cpp:1249
   1246	    mb_height = (mSendingHeight + 15) >> 4;
   1247
   1248	    cur_fs = mb_width * mb_height;
-> 1249	    max_fps = mCurSendCodecConfig->mEncodingConstraints.maxMbps/cur_fs;
   1250	    if (max_fps < mSendingFramerate) {
   1251	      new_framerate = max_fps;
   1252	    }

Issue seems to caused by telemetry polling performed every second by PeerConnectionCtx catching individual PC video conduit in a state where transmission has been already restarted due to processing incoming re-invite (which shouldn't even be changing anything as media direction and previously negotiated PT and its params remain the same), but encoder hasn't yet managed to produce any frames. This situation leads to both VideoConduit::mSendingWidth and VideoConduit::mSendingHeight still set to 0, causing the VideoConduit to perform division by zero.  Issue does not reproduce if remote endpoint does not specify max-mbps receiver capability in a=fmtp attribute for H.264 codec.


Expected results:

Firefox shouldn't crash

Comment 2

[Loic]

It's strange, builds loaded from mozregression don't crash, I have to copy the /firefox folder and use a custom profile to make them crash.

CR FF46:
https://crash-stats.mozilla.com/report/index/8919faec-c94b-4f71-98fa-2df202160106

Regression range:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=cc82d9b8f949a60ba3ad9b88c8fca77513e4fa68&tochange=d02f5e8ca0dc36845881530734049a064904c002

Probably:
Randell Jesup — Bug 1198458: Webrtc updated to branch 43; pull made 2015-09-29 09:00AM PDT rs=jesup

Comment 3

[Loic]

Attachment: test.html

Comment 4

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

Attachment: Check sending framesize is set before calculating max fps when max-mbps is negotiated

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2f2eb3c40ab0

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/2f2eb3c40ab0

Comment 7

[Liz Henry (:lizzard) (privacy/fingerprinting team)]

Tracking for 45+ since this is a recent regression.
Noting there are no crashes with this signature in the last week for any version. 
jesup do you want to request uplift to aurora since it was also affected?  
Would it be useful or possible to have test coverage for this issue?

Comment 8

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

Comment on attachment 8706638 [details] [diff] [review]
Check sending framesize is set before calculating max fps when max-mbps is negotiated

Approval Request Comment
[Feature/regressing bug #]: bug 1198458 

[User impact if declined]: Crash when talking to services that use max-mbps for H.264 (really it means such services will have to avoid it).

[Describe test coverage new/current, TreeHerder]: Not tested currently.  We don't usually ask for max-mbps.

[Risks and why]: exceedingly low risk - basically just an "if (value > 0)" test before doing a divide.

[String/UUID change made/needed]: none

Comment 9

[Liz Henry (:lizzard) (privacy/fingerprinting team)]

Comment on attachment 8706638 [details] [diff] [review]
Check sending framesize is set before calculating max fps when max-mbps is negotiated

Prevents a crash, recent regression. Please uplift to aurora.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/c1213d2b203e