User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/46.0.2490.71 Safari/537.36 Firefox for Android Steps to reproduce: I checked simpl.info/mediarecorder website on Firefox from Android. It prompts me for audio and video input access. I allowed both camera and mic. Actual results: The demo starts but audio only is recorded, not video Expected results: Video had to be recorded and returned as ogv

The URL is 404... Can you provide an updated link or source?

Hi Randell, sorry saw this just now. The url is http://simpl.info/mediarecorder If you click it from the url field of this page it will direct you to https://bugzilla.mozilla.org/simpl.info/mediarecorder which does not exist. So type it or copy and paste the exact url.

Comment on attachment 8732729 [details] [diff] [review] MuxOpusIntoWebm.wip.patch Review of attachment 8732729 [details] [diff] [review]: ----------------------------------------------------------------- I think if we can mux opus into webm, the MediaRecorder api should be work on android. It is a wip patch that I try to mux opus into webm container. (opus+vp8 in webm) Seems works on my android phone. ::: dom/media/encoder/OpusTrackEncoder.cpp @@ +240,5 @@ > mLookahead = 0; > } > > // The ogg time stamping and pre-skip is always timed at 48000. > + SerializeOpusIdHeader(mChannels, 0, mSamplingRate, https://wiki.xiph.org/MatroskaOpus The page says "CodecPrivate consists of the 'OpusHead' packet, identical to the Ogg mapping." So I put the |meta->mIdHeader in OpusTrackEncoder::GetMetadata| into |mEbmlComposer->SetAudioCodecPrivateData|. Then I hit the |codecDelay != pre-skip in OpusDataDecoder::Init()| when play the blob. So set the pre-skip to 0 as a workaround. Ralph: Do you know how to fix this? And any suggestion about opus in webm?

Thanks, Benjamin, for your patch. I think your approach of recording Opus + VP8 in webm is a good one. (I believe there is a lack of fixed point Vorbis support on Android -- so using Vorbis on Android is a non-starter.) I'd actually prefer to record VP8 + Opus (instead of Vorbis) on Desktop as well. With this in mind, let's plan to land this when we have Opus + VP8 playback support (which I believe Ralph is working on). Would you like to own this bug? I've talked with Jesup, and he's fine with giving this bug to you. If you can't take this bug, that's ok. Just let me know what you want to do. Also FYI: I've moved this from a P1 to a P2 (in part because we're gated on Opus + VP8 playback support), but I still very much want this to land as soon as it's ready and playback support is ready. Thanks!

(In reply to Benjamin Chen [:bechen] from comment #5) > Then I hit the |codecDelay != pre-skip in OpusDataDecoder::Init()| when play > the blob. You need to convert the pre-skip value from the private header from "samples at 48000 Hz" to "nanoseconds" and emit a CodecDelay element ([56][AA]) with that value inside the TrackEntry element in the file header. https://matroska.org/technical/specs/index.html#CodecDelay You also need to emit a SeekPreRoll element in the same header (standard value of 80 ms also converted to nanoseconds). Let me know if you have any other issues!

Review commit: https://reviewboard.mozilla.org/r/42435/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/42435/

Review commit: https://reviewboard.mozilla.org/r/42437/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/42437/

Review commit: https://reviewboard.mozilla.org/r/42439/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/42439/

Review commit: https://reviewboard.mozilla.org/r/42441/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/42441/

Comment on attachment 8734775 [details] MozReview Request: Bug 1215115 - part4: Enable MOZ_WEBM_ENCODER by default. r=ted https://reviewboard.mozilla.org/r/42435/#review39245 This should probably be last, not first, given the build breakage. Looks fine to me, but should have build peer review as well.

Comment on attachment 8734776 [details] MozReview Request: Bug 1215115 - part1: Replace the vorbis by opus in MediaEncoder and also reomve the VorbisTrackEncoder files. r=rillian https://reviewboard.mozilla.org/r/42437/#review39247 r=me with comment and build failures addressed. ::: dom/media/encoder/moz.build:36 (Diff revision 1) > - EXPORTS += ['VorbisTrackEncoder.h', > + EXPORTS += ['VP8TrackEncoder.h', > - 'VP8TrackEncoder.h', > ] > - UNIFIED_SOURCES += ['VorbisTrackEncoder.cpp', > + UNIFIED_SOURCES += ['VP8TrackEncoder.cpp', > - 'VP8TrackEncoder.cpp', > ] You should also remove the `VorbisTrackEncoder.*` source files if you're not building them any more. They'll still be available in the repository history if anyone wants the code later.

Comment on attachment 8734777 [details] MozReview Request: Bug 1215115 - part2: Mux opus into webm, remove bitdepth. r=rillian https://reviewboard.mozilla.org/r/42439/#review39271 Looks like a reasonable start, but I'd like to see it again. Also, TestVorbisTrackEncoder should be rewritten to be TestOpusTrackEncoder. ::: dom/media/encoder/OpusTrackEncoder.cpp:235 (Diff revision 1) > > RefPtr<OpusMetadata> meta = new OpusMetadata(); > + meta->mChannels = mChannels; > + meta->mSamplingFrequency = mSamplingRate; > + // TODO: fix me > + meta->mBitDepth = 32; mBitDepth doesn't seem to actually be used anywhere. Try removing it entirely; Take it out of `VorbisMetadata` and `OpusMetadata`, remove the argument from `EbmlComposer::SetAudioConfig()` and see if anything complains. ::: dom/media/webm/EbmlComposer.cpp:59 (Diff revision 1) > + // Details in OpusTrackEncoder.cpp. > + uint64_t codecDelay = > + (uint64_t)LittleEndian::readUint16(mCodecPrivateData.Elements() + 10) > + * PR_NSEC_PER_SEC / 48000; > + // Fixed 80ms, convert into nanoseconds. > + uint64_t seekPreRoll = 80 * PR_USEC_PER_SEC; `80 * PR_NSEC_PER_MSEC` ::: dom/media/webm/EbmlComposer.cpp:149 (Diff revision 1) > Ebml_SerializeUnsigned(&ebml, Timecode, mClusterTimecode); > mFlushState |= FLUSH_CLUSTER; > } > > - bool isVorbis = (frameType == EncodedFrame::FrameType::VORBIS_AUDIO_FRAME); > + bool isOpus = (frameType == EncodedFrame::FrameType::OPUS_AUDIO_FRAME); > short timeCode = aFrame->GetTimeStamp() / PR_USEC_PER_MSEC - mClusterTimecode; You may need to add CodecDelay here, depending on how the timestamps get set on the incoming frame.

Comment on attachment 8734778 [details] MozReview Request: Bug 1215115 - part3: Fix gtest. Remove TestVorbisTrackEncoder.cpp. r=rillian https://reviewboard.mozilla.org/r/42441/#review39331 r=me with comment addressed. ::: dom/media/gtest/moz.build:35 (Diff revision 1) > ] > > if CONFIG['MOZ_WEBM_ENCODER']: > UNIFIED_SOURCES += [ > 'TestVideoTrackEncoder.cpp', > - 'TestVorbisTrackEncoder.cpp', > + #'TestVorbisTrackEncoder.cpp', Don't comment-out lines like this, just delete them. Or replace this with TestOpusTrackEncoder.cpp, really.

Hi there. If I understand well, you are setting things up so that MediaRecorder API output has now Opus encoding for audio. Please note that this will create a problem with Web Audio API, which does not read Opus. The problem is already known on Chrome. They are working on it as per this bug https://bugs.chromium.org/p/chromium/issues/detail?id=482934 It would be bad to have MediaRecorder API and Web Audio API not able to talk to each other, I mean, I think it is obvious to everybody. So please, just take this into account. If I did not understand well your last comments and you are fixing something else, please disregard this comment.

Comment on attachment 8734776 [details] MozReview Request: Bug 1215115 - part1: Replace the vorbis by opus in MediaEncoder and also reomve the VorbisTrackEncoder files. r=rillian Review request updated; see interdiff: https://reviewboard.mozilla.org/r/42437/diff/1-2/

Comment on attachment 8734777 [details] MozReview Request: Bug 1215115 - part2: Mux opus into webm, remove bitdepth. r=rillian Review request updated; see interdiff: https://reviewboard.mozilla.org/r/42439/diff/1-2/

Comment on attachment 8734778 [details] MozReview Request: Bug 1215115 - part3: Fix gtest. Remove TestVorbisTrackEncoder.cpp. r=rillian Review request updated; see interdiff: https://reviewboard.mozilla.org/r/42441/diff/1-2/

Comment on attachment 8734775 [details] MozReview Request: Bug 1215115 - part4: Enable MOZ_WEBM_ENCODER by default. r=ted Review request updated; see interdiff: https://reviewboard.mozilla.org/r/42435/diff/1-2/

Comment on attachment 8734777 [details] MozReview Request: Bug 1215115 - part2: Mux opus into webm, remove bitdepth. r=rillian https://reviewboard.mozilla.org/r/42439/#review39877 ::: dom/media/webm/EbmlComposer.h:71 (Diff revision 2) > // The cluster length position. > uint64_t mClusterLengthLoc; > // Audio codec specific header data. > nsTArray<uint8_t> mCodecPrivateData; > + // Codec delay in nanoseconds. > + uint64_t mCodecDelay; Since the inialization in conditional in GenerateHeader() you need to set this to zero in the EmblComposer constructor at the end of EbmlComposer.cpp.

https://reviewboard.mozilla.org/r/42441/#review40011 ::: dom/media/test/mochitest.ini (Diff revision 2) > [test_mediarecorder_record_audiocontext_mlk.html] > tags=msg > [test_mediarecorder_record_audionode.html] > tags=msg > [test_mediarecorder_record_canvas_captureStream.html] > -skip-if = (android_version < '17' || toolkit == 'android') # Android/Gonk before SDK version 17 does not have the OMX Encoder API and Fennec does not support video recording I will not enable these 3 tests because I found they always fail at all platforms. Seems that remove the |skip-if = (android_version < '17' || toolkit == 'android')| also enable the testcase at other platforms. (I guess other platforms have a android_version which is smaller than 17...)

Comment on attachment 8734775 [details] MozReview Request: Bug 1215115 - part4: Enable MOZ_WEBM_ENCODER by default. r=ted https://reviewboard.mozilla.org/r/42435/#review40133 If this is going to be enabled by default on Firefox desktop and Firefox for Android I submit that it should just be on-by-default in configure.

Comment on attachment 8734777 [details] MozReview Request: Bug 1215115 - part2: Mux opus into webm, remove bitdepth. r=rillian Review request updated; see interdiff: https://reviewboard.mozilla.org/r/42439/diff/2-3/

Comment on attachment 8734778 [details] MozReview Request: Bug 1215115 - part3: Fix gtest. Remove TestVorbisTrackEncoder.cpp. r=rillian Review request updated; see interdiff: https://reviewboard.mozilla.org/r/42441/diff/2-3/

Comment on attachment 8734775 [details] MozReview Request: Bug 1215115 - part4: Enable MOZ_WEBM_ENCODER by default. r=ted Review request updated; see interdiff: https://reviewboard.mozilla.org/r/42435/diff/2-3/

Comment on attachment 8734775 [details] MozReview Request: Bug 1215115 - part4: Enable MOZ_WEBM_ENCODER by default. r=ted Review request updated; see interdiff: https://reviewboard.mozilla.org/r/42435/diff/3-4/

Comment on attachment 8734775 [details] MozReview Request: Bug 1215115 - part4: Enable MOZ_WEBM_ENCODER by default. r=ted https://reviewboard.mozilla.org/r/42435/#review40293

https://hg.mozilla.org/integration/mozilla-inbound/rev/0d41311af46d https://hg.mozilla.org/integration/mozilla-inbound/rev/bf791204fc36 https://hg.mozilla.org/integration/mozilla-inbound/rev/908acebd5af4 https://hg.mozilla.org/integration/mozilla-inbound/rev/11cfc06439f9

https://hg.mozilla.org/mozilla-central/rev/0d41311af46d https://hg.mozilla.org/mozilla-central/rev/bf791204fc36 https://hg.mozilla.org/mozilla-central/rev/908acebd5af4 https://hg.mozilla.org/mozilla-central/rev/11cfc06439f9