Closed Bug 1693043 Opened 3 years ago Closed 3 years ago

Assertion failure: mInitialized || mCanceled, at /builds/worker/checkouts/gecko/dom/media/encoder/OpusTrackEncoder.cpp:254

Categories

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

defect

Tracking

()

VERIFIED FIXED
88 Branch
Tracking Status
firefox-esr78 --- wontfix
firefox86 --- wontfix
firefox87 --- wontfix
firefox88 --- verified

People

(Reporter: jkratzer, Assigned: pehrsons)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, testcase, Whiteboard: [bugmon:confirm])

Attachments

(3 files)

Attached file testcase.zip

Testcase found while fuzzing mozilla-central rev fc74eb2c7b84 (built with --enable-debug).

Assertion failure: mInitialized || mCanceled, at /builds/worker/checkouts/gecko/dom/media/encoder/OpusTrackEncoder.cpp:254

    #0 0x7f95d12774d8 in mozilla::OpusTrackEncoder::Encode(mozilla::AudioSegment*) /builds/worker/checkouts/gecko/dom/media/encoder/OpusTrackEncoder.cpp:254:3
    #1 0x7f95d1278dad in mozilla::AudioTrackEncoder::NotifyEndOfStream() /builds/worker/checkouts/gecko/dom/media/encoder/TrackEncoder.cpp:260:7
    #2 0x7f95d1282c2b in operator() /builds/worker/checkouts/gecko/dom/media/encoder/MediaEncoder.cpp:158:58
    #3 0x7f95d1282c2b in mozilla::detail::RunnableFunction<mozilla::MediaEncoder::AudioTrackListener::NotifyRemoved(mozilla::MediaTrackGraph*)::'lambda'()>::Run() /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:534:5
    #4 0x7f95cd9ccee2 in mozilla::TaskQueue::Runner::Run() /builds/worker/checkouts/gecko/xpcom/threads/TaskQueue.cpp:158:20
    #5 0x7f95cd9e5287 in nsThreadPool::Run() /builds/worker/checkouts/gecko/xpcom/threads/nsThreadPool.cpp:303:14
    #6 0x7f95cd9dc6f3 in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/checkouts/gecko/xpcom/threads/nsThread.cpp:1152:16
    #7 0x7f95cd9e2a6a in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/checkouts/gecko/xpcom/threads/nsThreadUtils.cpp:548:10
    #8 0x7f95ce2fa11d in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /builds/worker/checkouts/gecko/ipc/glue/MessagePump.cpp:302:20
    #9 0x7f95ce264563 in MessageLoop::RunInternal() /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:335:10
    #10 0x7f95ce26447d in RunHandler /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:328:3
    #11 0x7f95ce26447d in MessageLoop::Run() /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:310:3
    #12 0x7f95cd9d8e16 in nsThread::ThreadFunc(void*) /builds/worker/checkouts/gecko/xpcom/threads/nsThread.cpp:391:10
    #13 0x7f95e32ebcdb in _pt_root /builds/worker/checkouts/gecko/nsprpub/pr/src/pthreads/ptthread.c:201:5
    #14 0x7f95e398c608 in start_thread /build/glibc-eX1tMB/glibc-2.31/nptl/pthread_create.c:477:8
    #15 0x7f95e3555292 in clone /build/glibc-eX1tMB/glibc-2.31/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:95
Flags: in-testsuite?

This assertion dates back to 2017, but there were significant changes around it recently.

Andreas, can you triage this, please?

Flags: needinfo?(apehrson)

This bug happens because we plumb through an audio bitrate that is a uint32_t, that in our opus encoder gets cast to an int. So if the uint32_t is large enough, the cast int becomes negative, which is rejected by the underlying opus encoder.

We are supposed to cap the audio bitrate at 512kbps. That's not working, and that must be the bug.

Assignee: nobody → apehrson
Status: NEW → ASSIGNED
Flags: needinfo?(apehrson)

Ah, no, the cap is only part of the algorithm for when the [[ConstrainedBitsPerSecond]] slot (by spec) is not undefined.

We should fix the cast.

I haven't verified a regression range yet. It looks like the bad cast causing encoder-init rejection is old. Perhaps the assertion failure is new, since this wasn't found earlier.

I wrote a crashtest for this so the info below is based on running that test.

The assertion failure seems to come from bug 1464268 where AudioTrackEncoder::NotifyEndOfStream calls Encode() which has the assert. Prior to that bug we'd just set a flag. Pulling data out of the encoder would be an async operation -- which would never be triggered since we weren't initialized. This caused a shutdown hang instead, so in that sense bug 1464268 made things better by surfacing this issue.

In non-debug we now pass gracefully, so bug 1464268 made that better too. That also makes this much less severe.

Because the root cause here is not a recent regression, I will not mark it as one.

Severity: -- → S4
Priority: -- → P2
Pushed by pehrsons@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/7fcb529d1fa0
Add crashtest. r=bryce
https://hg.mozilla.org/integration/autoland/rev/43a138a41dd8
Avoid overflowing the opus encoder bitrate. r=bryce
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 88 Branch

Comment on attachment 9207494 [details]
Bug 1693043 - Avoid overflowing the opus encoder bitrate. r?bryce!

Beta/Release Uplift Approval Request

  • User impact if declined: We won't be able to record audio in some cases, though not a recent bug. What is recent is that this causes an assertion failure in debug builds.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: No
  • 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): Trivial change to avoid an overflowing cast.
  • String changes made/needed:
Attachment #9207494 - Flags: approval-mozilla-beta?
Attachment #9207493 - Flags: approval-mozilla-beta?

Given this is so old and we build the last 87 beta today I'd rather let this ride the trains.

Attachment #9207493 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
Attachment #9207494 - Flags: approval-mozilla-beta? → approval-mozilla-beta-

Bugmon Analysis:
Verified bug as fixed on rev mozilla-central 20210311093001-bd8121e747b5.
Removing bugmon keyword as no further action possible.
Please review the bug and re-add the keyword for further analysis.

Status: RESOLVED → VERIFIED
Keywords: bugmon
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: