Closed Bug 1696794 Opened 3 years ago Closed 3 years ago

MediaRecorder audio doesn't record all the duration

Categories

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

Firefox 88
defect

Tracking

()

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

People

(Reporter: sizvix, Assigned: pehrsons)

References

(Regression)

Details

(Keywords: regression)

Attachments

(2 files)

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:88.0) Gecko/20100101 Firefox/88.0

Steps to reproduce:

I record 10 seconds of audio , and read it

Actual results:

At the play , I have only 3 seconds of sound

Expected results:

I should have 10 seconds of audio on playing the audio.
Same code works on Firefox and chromium , but not on Nightly ...

The Bugbug bot thinks this bug should belong to the 'Core::Audio/Video: Playback' component, and is moving the bug to that component. Please revert this change in case you think the bot is wrong.

Component: Untriaged → Audio/Video: Playback
Product: Firefox → Core

I don't have a Linux box to check it locally but this looks like a MediaRecorder issue. Moving it to the recorder component.
I can confirm that it happens in Nightly on macOS too.

Component: Audio/Video: Playback → Audio/Video: Recording
Severity: -- → S2
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(apehrson)
Keywords: regression
Priority: -- → P1
Assignee: nobody → apehrson
Status: NEW → ASSIGNED
Flags: needinfo?(apehrson)

I'd say this is the regressor: https://hg.mozilla.org/mozilla-central/rev/bb3690c52cb92637d097817682902a5165262ddf
In particular the logic that calls Extract() only every second.

We'd only get one page out of OggWriter per GetContainerData() call. Flawed behavior perhaps, but it worked fine prior to bug 1464268, because we'd call it very often. With bug 1464268 we call it once per second. At 128kbps, one page of 4kB is 250ms. Thus with default audio bitrate and no timeslice we'd only get the first 25% of all pages by the end of the recording.

Workarounds:

  • Set the timeslice at <250ms (with default bitrate - 128kbps)
  • Set audio bitrate to <32kbps (with no timeslice - extracting pages every second)
Has Regression Range: --- → yes

With bug 1464268 and friends the ContainerWriters starting being called once per
second to get written data out. OggWriter was only able to handle getting one
page out per call, so if we had written N pages to OggWriter while processing a
second worth of data, it'd still have N-1 pages in a buffer. That led to
MediaRecorder writing much too short files when recording audio/ogg.

This patch fixes this properly in OggWriter by extracting all available pages in
GetContainerData().

Comment on attachment 9210206 [details]
Bug 1696794 - Drain all full pages in OggWriter::GetContainerData. r?bryce!

Beta/Release Uplift Approval Request

  • User impact if declined: Using MediaRecorder for audio-only recordings with otherwise default settings results in only the first ~25% of the recording being saved.
  • 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): Simple change in code with a very narrow use-case. Would only affect this functionality that is already broken (see the impact field on how).
  • String changes made/needed:
Attachment #9210206 - Flags: approval-mozilla-beta?
Attachment #9210205 - Flags: approval-mozilla-beta?

I should say, the simplest workaround would be setting the mime type to audio/webm.

Pushed by pehrsons@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/428a79637282
Add gtest. r=bryce
https://hg.mozilla.org/integration/autoland/rev/c66f0d85eed2
Drain all full pages in OggWriter::GetContainerData. r=bryce

Backed out 2 changesets (bug 1696794) for bustage complaining about gtest.h.

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&fromchange=c66f0d85eed25798de54f7309ac7d6fd9fdb2332&searchStr=build&selectedTaskRun=ezTucJNHRwq3vd5TePv3Mw.0&tochange=2c5cce48931114735d687cf9458482683e3e32f7

Backout link: https://hg.mozilla.org/integration/autoland/rev/2c5cce48931114735d687cf9458482683e3e32f7

Failure log: https://treeherder.mozilla.org/logviewer?job_id=333796669&repo=autoland&lineNumber=10570

[task 2021-03-19T16:41:50.885Z] 16:41:50     INFO -  make[4]: Entering directory '/builds/worker/workspace/obj-build/dom/media/gtest'
[task 2021-03-19T16:41:50.893Z] 16:41:50     INFO -  /builds/worker/fetches/sccache/sccache /builds/worker/fetches/clang/bin/clang++ --sysroot /builds/worker/fetches/sysroot -std=gnu++17 -o Unified_cpp_dom_media_gtest2.o -c  -I/builds/worker/workspace/obj-build/dist/stl_wrappers -I/builds/worker/workspace/obj-build/dist/system_wrappers -include /builds/worker/checkouts/gecko/config/gcc_hidden.h -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -fstack-clash-protection -DNDEBUG=1 -DTRIMMED=1 -DHAVE_UINT64_T -DWEBRTC_POSIX -DWEBRTC_BUILD_LIBEVENT -DWEBRTC_LINUX -DENABLE_SET_CUBEB_BACKEND -DOS_POSIX=1 -DOS_LINUX=1 -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -DSTATIC_EXPORTABLE_JS_API -I/builds/worker/checkouts/gecko/dom/media/gtest -I/builds/worker/workspace/obj-build/dom/media/gtest -I/builds/worker/checkouts/gecko/dom/media/webrtc/common -I/builds/worker/checkouts/gecko/third_party/libwebrtc -I/builds/worker/checkouts/gecko/third_party/libwebrtc/webrtc -I/builds/worker/checkouts/gecko/gfx/2d -I/builds/worker/workspace/obj-build/ipc/ipdl/_ipdlheaders -I/builds/worker/checkouts/gecko/ipc/chromium/src -I/builds/worker/checkouts/gecko/ipc/glue -I/builds/worker/checkouts/gecko/dom/media -I/builds/worker/checkouts/gecko/dom/media/encoder -I/builds/worker/checkouts/gecko/dom/media/gmp -I/builds/worker/checkouts/gecko/dom/media/mp4 -I/builds/worker/checkouts/gecko/dom/media/platforms -I/builds/worker/checkouts/gecko/dom/media/platforms/agnostic -I/builds/worker/checkouts/gecko/dom/media/webrtc -I/builds/worker/checkouts/gecko/security/certverifier -I/builds/worker/workspace/obj-build/dist/include -I/builds/worker/workspace/obj-build/dist/include/nspr -I/builds/worker/workspace/obj-build/dist/include/nss -DMOZILLA_CLIENT -include /builds/worker/workspace/obj-build/mozilla-config.h -Qunused-arguments -Qunused-arguments -Wall -Wbitfield-enum-conversion -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wshadow-field-in-constructor-modified -Wsign-compare -Wtype-limits -Wunreachable-code -Wunreachable-code-return -Wwrite-strings -Wno-invalid-offsetof -Wclass-varargs -Wempty-init-stmt -Wfloat-overflow-conversion -Wfloat-zero-conversion -Wloop-analysis -Wno-range-loop-analysis -Wc++2a-compat -Wcomma -Wimplicit-fallthrough -Wunused-function -Wunused-variable -Werror=non-literal-null-conversion -Wstring-conversion -Wtautological-overlap-compare -Wtautological-unsigned-enum-zero-compare -Wtautological-unsigned-zero-compare -Wno-error=tautological-type-limit-compare -Wno-inline-new-delete -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=backend-plugin -Wno-error=return-std-move -Wno-error=atomic-alignment -Wno-error=deprecated-copy -Wformat -Wformat-security -Wno-gnu-zero-variadic-macro-arguments -Wno-psabi -Wno-unknown-warning-option -D_GLIBCXX_USE_CXX11_ABI=0 -fno-sized-deallocation -fno-aligned-new -fcrash-diagnostics-dir=/builds/worker/artifacts -fno-exceptions -fno-strict-aliasing -fPIC -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -pthread -pipe -g -Xclang -load -Xclang /builds/worker/workspace/obj-build/build/clang-plugin/libclang-plugin.so -Xclang -add-plugin -Xclang moz-check -O2 -fno-omit-frame-pointer -funwind-tables -Werror -Wno-error=shadow -Wno-error=shadow -Wno-inconsistent-missing-override -Wno-unused-private-field -fexperimental-new-pass-manager  -MD -MP -MF .deps/Unified_cpp_dom_media_gtest2.o.pp   Unified_cpp_dom_media_gtest2.cpp
[task 2021-03-19T16:41:50.894Z] 16:41:50     INFO -  In file included from Unified_cpp_dom_media_gtest2.cpp:2:
[task 2021-03-19T16:41:50.894Z] 16:41:50     INFO -  In file included from /builds/worker/checkouts/gecko/dom/media/gtest/TestMediaDataEncoder.cpp:6:
[task 2021-03-19T16:41:50.894Z] 16:41:50    ERROR -  /builds/worker/workspace/obj-build/dist/include/gtest/gtest.h:1445:11: error: comparison of integers of different signs: 'const unsigned long' and 'const int' [-Werror,-Wsign-compare]
[task 2021-03-19T16:41:50.895Z] 16:41:50     INFO -    if (lhs == rhs) {
[task 2021-03-19T16:41:50.895Z] 16:41:50     INFO -        ~~~ ^  ~~~
[task 2021-03-19T16:41:50.895Z] 16:41:50     INFO -  /builds/worker/workspace/obj-build/dist/include/gtest/gtest.h:1473:12: note: in instantiation of function template specialization 'testing::internal::CmpHelperEQ<unsigned long, int>' requested here
[task 2021-03-19T16:41:50.895Z] 16:41:50     INFO -      return CmpHelperEQ(lhs_expression, rhs_expression, lhs, rhs);
[task 2021-03-19T16:41:50.895Z] 16:41:50     INFO -             ^
[task 2021-03-19T16:41:50.895Z] 16:41:50     INFO -  /builds/worker/checkouts/gecko/dom/media/gtest/TestOggWriter.cpp:59:3: note: in instantiation of function template specialization 'testing::internal::EqHelper<false>::Compare<unsigned long, int>' requested here
[task 2021-03-19T16:41:50.895Z] 16:41:50     INFO -    EXPECT_EQ(inputBytes, 16000);
[task 2021-03-19T16:41:50.895Z] 16:41:50     INFO -    ^
[task 2021-03-19T16:41:50.895Z] 16:41:50     INFO -  /builds/worker/workspace/obj-build/dist/include/gtest/gtest.h:1969:63: note: expanded from macro 'EXPECT_EQ'
[task 2021-03-19T16:41:50.896Z] 16:41:50     INFO -                        EqHelper<GTEST_IS_NULL_LITERAL_(val1)>::Compare, \
[task 2021-03-19T16:41:50.896Z] 16:41:50     INFO -                                                                ^
[task 2021-03-19T16:41:50.896Z] 16:41:50     INFO -  1 error generated.
[task 2021-03-19T16:41:50.896Z] 16:41:50    ERROR -  make[4]: *** [/builds/worker/checkouts/gecko/config/rules.mk:674: Unified_cpp_dom_media_gtest2.o] Error 1
[task 2021-03-19T16:41:50.896Z] 16:41:50     INFO -  make[4]: Leaving directory '/builds/worker/workspace/obj-build/dom/media/gtest'
[task 2021-03-19T16:41:50.896Z] 16:41:50    ERROR -  make[3]: *** [/builds/worker/checkouts/gecko/config/recurse.mk:72: dom/media/gtest/target-objects] Error 2
[task 2021-03-19T16:41:50.896Z] 16:41:50     INFO -  make[3]: *** Waiting for unfinished jobs....
Flags: needinfo?(apehrson)
Flags: needinfo?(apehrson)
Pushed by pehrsons@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/4ef08110a07f
Add gtest. r=bryce
https://hg.mozilla.org/integration/autoland/rev/663cc2bafe0a
Drain all full pages in OggWriter::GetContainerData. r=bryce
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 88 Branch

Comment on attachment 9210205 [details]
Bug 1696794 - Add gtest. r?bryce!

It's too late for 87.0. Keeping on the radar for a dot release.

Attachment #9210205 - Flags: approval-mozilla-beta? → approval-mozilla-release?
Attachment #9210206 - Flags: approval-mozilla-beta? → approval-mozilla-release?
Flags: qe-verify+
QA Whiteboard: [qa-triaged]

Reproduced the initial issue on Windows 10 x64 using Firefox 87.0 with the mention that only 4 seconds out of 12 were available.

Confirming that the entire recorded duration is correctly playing back on Firefox 88.0b5 (buildID 20210330185720) on Windows 10 x64 and Ubuntu 20.04.

Status: RESOLVED → VERIFIED
Flags: qe-verify+

Comment on attachment 9210205 [details]
Bug 1696794 - Add gtest. r?bryce!

88 is in RC now and there are no plans for an 87 dot release.

Attachment #9210205 - Flags: approval-mozilla-release? → approval-mozilla-release-
Attachment #9210206 - Flags: approval-mozilla-release? → approval-mozilla-release-
You need to log in before you can comment on or make changes to this bug.