Open Bug 1594141 Opened 5 years ago Updated 2 years ago

Assertion failure: mVideoEncoder->IsInitialized(), at /builds/worker/workspace/build/src/dom/media/encoder/MediaEncoder.cpp:682


(Core :: Audio/Video: Recording, defect, P2)




Tracking Status
firefox72 --- affected


(Reporter: jkratzer, Unassigned)


(Blocks 1 open bug)


(Keywords: assertion, testcase)


(3 files)

Attached file testcase.html

Testcase found while fuzzing mozilla-central rev 4d585c7edc76.

Assertion failure: mVideoEncoder->IsInitialized(), at /builds/worker/workspace/build/src/dom/media/encoder/MediaEncoder.cpp:682

rax = 0x000055df47163340   rdx = 0x0000000000000000
rcx = 0x00007ff50af4f63a   rbx = 0x00007ff4fd2ab660
rsi = 0x00007ff5169a08b0   rdi = 0x00007ff51699f680
rbp = 0x00007ff4fb9a1410   rsp = 0x00007ff4fb9a1390
r8 = 0x00007ff5169a08b0    r9 = 0x00007ff4fb9a2700
r10 = 0x0000000000000002   r11 = 0x0000000000000000
r12 = 0x00007ff4fb9a13c0   r13 = 0x0000000000000000
r14 = 0x0000000000000000   r15 = 0x00007ff4fb9a1470
rip = 0x00007ff506ccd761
OS|Linux|0.0.0 Linux 5.0.0-31-generic #33~18.04.1-Ubuntu SMP Tue Oct 1 10:20:39 UTC 2019 x86_64
CPU|amd64|family 6 model 94 stepping 3|1
35|0||mozilla::MediaEncoder::GetEncodedData(nsTArray<nsTArray<unsigned char> >*)||680|0x32
35|1||mozilla::dom::MediaRecorder::Session::Extract(mozilla::TimeStamp, bool)||946|0x18
35|5||mozilla::detail::RunnableMethodImpl<mozilla::MediaEncoder::EncoderListener*, void (mozilla::MediaEncoder::EncoderListener::*)(), true, (mozilla::RunnableKind)0>::Run()||1176|0x13
35|8||nsThread::ProcessNextEvent(bool, bool*)||1225|0x15
35|9||NS_ProcessNextEvent(nsIThread*, bool)||486|0x11
Flags: in-testsuite?

:bryce, it looks like you were last in this code. Would you care to take a look?

Flags: needinfo?(bvandyk)
Priority: -- → P2
Assignee: nobody → bvandyk

Think I've found the source of the issue.

MediaEncoder expects the track encoders to be initialized by the time we're getting encoded data. This would be a safe assumption if an initialized encoder could never transition to becoming uninitialized, but for the vp8 encoder that is not the case.

There is an edge case that we're not handling:

The above results in us transitioning from an initialized encoder to an uninitialized one and hitting this assert as we get data.

Patch incoming.

Flags: needinfo?(bvandyk)

This is adapted from the original test on the bug. The only changes are:

  • Adding reftest-wait so the test plays nice with our harness.
  • Removing code that doesn't appear required to hit the assert.
    • There are a few lines that don't appear needed.
    • Simplified some arguments to make the test more human readable.

If the VP8 encoder fails to reconfigure in the middle of a stream it can
transition from a state where it was returning true for IsInitialized to
then returning false. This would trigger an assert in GetEncodedData in
MediaEncoder. This patch changes to instead handle this case by logging an
error and moving the encoder into an error state.

Depends on D51955

Try is currently closed. Getting patches up for review, and will push tomorrow/when the tree is open.

Pushed by
Add crashtest for if a Vp8 track encoder becomes uninitialized mid stream. r=pehrsons
Handle failure to reconfigure video encoder in MediaEncoder. r=pehrsons

Backed out for media failures on test_mediarecorder_principals.html




[task 2019-11-08T20:54:27.958Z] 20:53:56 INFO - 286 INFO TEST-PASS | dom/media/test/test_mediarecorder_principals.html | mediaRecorder.start() must throw SecurityError
[task 2019-11-08T20:54:27.958Z] 20:53:56 INFO - 287 INFO TEST-PASS | dom/media/test/test_mediarecorder_principals.html | mediaRecorder.start() should not throw here, and didn't
[task 2019-11-08T20:54:27.958Z] 20:53:56 INFO - Buffered messages finished
[task 2019-11-08T20:54:27.958Z] 20:53:56 WARNING - 288 INFO TEST-UNEXPECTED-FAIL | dom/media/test/test_mediarecorder_principals.html | mediaRecorder.onerror must fire SecurityError - got "UnknownError", expected "SecurityError"
[task 2019-11-08T20:54:27.958Z] 20:53:56 INFO -
[task 2019-11-08T20:54:27.958Z] 20:53:56 INFO - testPrincipals2@dom/media/test/test_mediarecorder_principals.html:113:5
[task 2019-11-08T20:54:27.958Z] 20:53:56 INFO - 289 INFO TEST-PASS | dom/media/test/test_mediarecorder_principals.html | Events fired from onerror should include an error with a stack trace indicating an error in this test
[task 2019-11-08T20:54:27.958Z] 20:53:56 INFO - 290 INFO TEST-PASS | dom/media/test/test_mediarecorder_principals.html | Playback should not have reached end

Flags: needinfo?(bvandyk)

Acking. I don't appear able repro the failure on my local Android emulator, continuing to investigate.

My suspicion is that the failure is caused by the new calls to SetError in the patch are triggering a session end task due to the encode error before the security error the test expects. That indicates that the vp8 encoder is failing to encode data prior to our security error, which is likely a pre-existing issue being shown up by the patch.

There are some r+ patches which didn't land and no activity in this bug for 2 weeks.
:bryce, could you have a look please?
For more information, please visit auto_nag documentation.

Flags: needinfo?(bvandyk)

Rework is pending me being able to debug this on Android, which is proving more fun than I expected. I'm currently soft blocked by bug 1598461.

Depends on: 1598461
Flags: needinfo?(bvandyk)

It seems fixed, maybe it's time to finish it?

Bryce can I help you to finish this one. FWIW, I have a local Android build and I can test stuff in my device.

(In reply to Alex Chronopoulos [:achronop] from comment #14)

Bryce can I help you to finish this one. FWIW, I have a local Android build and I can test stuff in my device.

That would be appreciated.

Flags: needinfo?(bvandyk)
Assignee: bvandyk → nobody
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.