Mediarecorder API does not record video on Firefox Android

RESOLVED FIXED in Firefox 48

Status

()

P2
normal
Rank:
21
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: adifran, Assigned: bechen)

Tracking

44 Branch
mozilla48
Unspecified
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox48 fixed)

Details

(URL)

Attachments

(4 attachments, 1 obsolete attachment)

(Reporter)

Description

3 years ago
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
(Reporter)

Updated

3 years ago
Summary: Mediarecorder API does nor record video on Firefox Android → Mediarecorder API does not record video on Firefox Android
(Reporter)

Updated

3 years ago
(Reporter)

Updated

3 years ago
OS: Unspecified → Android
Component: Untriaged → WebRTC: Audio/Video
Product: Firefox → Core
The URL is 404... Can you provide an updated link or source?
Component: WebRTC: Audio/Video → Audio/Video: Recording
Flags: needinfo?(adifran)
(Reporter)

Comment 2

3 years ago
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.
Flags: needinfo?(adifran)
Status: UNCONFIRMED → NEW
Rank: 15
Ever confirmed: true
Priority: -- → P1
Duplicate of this bug: 955985
Assignee: nobody → rjesup
(Assignee)

Comment 4

3 years ago
Created attachment 8732729 [details] [diff] [review]
MuxOpusIntoWebm.wip.patch
(Assignee)

Comment 5

3 years ago
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?
(Assignee)

Updated

3 years ago
Flags: needinfo?(giles)
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!
Rank: 15 → 21
Flags: needinfo?(bechen)
Priority: P1 → P2
(Assignee)

Updated

3 years ago
Assignee: rjesup → bechen
Flags: needinfo?(bechen)
(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!
Flags: needinfo?(giles)
(Assignee)

Comment 8

3 years ago
Created attachment 8734775 [details]
MozReview Request: Bug 1215115 - part4: Enable MOZ_WEBM_ENCODER by default. r=ted

Review commit: https://reviewboard.mozilla.org/r/42435/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/42435/
Attachment #8734775 - Flags: review?(giles)
(Assignee)

Comment 9

3 years ago
Created attachment 8734776 [details]
MozReview Request: Bug 1215115 - part1: Replace the vorbis by opus in MediaEncoder and also reomve the VorbisTrackEncoder files. r=rillian

Review commit: https://reviewboard.mozilla.org/r/42437/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/42437/
Attachment #8734776 - Flags: review?(giles)
(Assignee)

Comment 10

3 years ago
Created attachment 8734777 [details]
MozReview Request: Bug 1215115 - part2: Mux opus into webm, remove bitdepth. r=rillian

Review commit: https://reviewboard.mozilla.org/r/42439/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/42439/
Attachment #8734777 - Flags: review?(giles)
(Assignee)

Comment 11

3 years ago
Created attachment 8734778 [details]
MozReview Request: Bug 1215115 - part3: Fix gtest. Remove TestVorbisTrackEncoder.cpp. r=rillian

Review commit: https://reviewboard.mozilla.org/r/42441/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/42441/
Attachment #8734778 - Flags: review?(giles)
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.
Attachment #8734775 - Flags: review?(giles) → review+
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.
Attachment #8734776 - Flags: review?(giles) → review+
Attachment #8734777 - Flags: review?(giles)
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.
Attachment #8734778 - Flags: review?(giles) → review+
(Reporter)

Comment 16

3 years ago
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.
(Assignee)

Comment 17

3 years ago
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/
Attachment #8734776 - Attachment description: MozReview Request: Bug 1215115 - part2: Replace the vorbis by opus in MediaEncoder. r? → MozReview Request: Bug 1215115 - part1: Replace the vorbis by opus in MediaEncoder and also reomve the VorbisTrackEncoder files. r=rillian
Attachment #8734777 - Attachment description: MozReview Request: Bug 1215115 - part3: Mux opus into webm. r? → MozReview Request: Bug 1215115 - part2: Mux opus into webm, remove bitdepth. r?
Attachment #8734778 - Attachment description: MozReview Request: Bug 1215115 - part4: Fix gtest testcases. Enable some mochitests on android. r? → MozReview Request: Bug 1215115 - part3: Remove TestVorbisTrackEncoder.cpp. Enable some mochitests on android. r?
Attachment #8734775 - Attachment description: MozReview Request: Bug 1215115 - part1: Enable the MOZ_WEBM_ENCODER for android. r? → MozReview Request: Bug 1215115 - part4: Enable the MOZ_WEBM_ENCODER for android. r?
Attachment #8734777 - Flags: review?(giles)
Attachment #8734775 - Flags: review?(ted)
(Assignee)

Comment 18

3 years ago
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/
(Assignee)

Comment 19

3 years ago
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/
(Assignee)

Comment 20

3 years ago
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.
Attachment #8734777 - Flags: review?(giles) → review+
(Assignee)

Comment 22

3 years ago
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...)
(Assignee)

Updated

3 years ago
Blocks: 1261007
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.
Attachment #8734775 - Flags: review?(ted)
(Assignee)

Comment 24

3 years ago
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/
Attachment #8734777 - Attachment description: MozReview Request: Bug 1215115 - part2: Mux opus into webm, remove bitdepth. r? → MozReview Request: Bug 1215115 - part2: Mux opus into webm, remove bitdepth. r=rillian
Attachment #8734778 - Attachment description: MozReview Request: Bug 1215115 - part3: Remove TestVorbisTrackEncoder.cpp. Enable some mochitests on android. r? → MozReview Request: Bug 1215115 - part3: Fix gtest. Remove TestVorbisTrackEncoder.cpp. r=rillian
Attachment #8734775 - Attachment description: MozReview Request: Bug 1215115 - part4: Enable the MOZ_WEBM_ENCODER for android. r? → MozReview Request: Bug 1215115 - part4: Enable the MOZ_WEBM_ENCODER for android. r=rillian
(Assignee)

Comment 25

3 years ago
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/
(Assignee)

Comment 26

3 years ago
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/
(Assignee)

Updated

3 years ago
Attachment #8732729 - Attachment is obsolete: true
(Assignee)

Comment 27

3 years ago
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/
Attachment #8734775 - Attachment description: MozReview Request: Bug 1215115 - part4: Enable the MOZ_WEBM_ENCODER for android. r=rillian → MozReview Request: Bug 1215115 - part4: Enable MOZ_WEBM_ENCODER by default. r=ted
Attachment #8734775 - Flags: review?(ted)
Attachment #8734775 - Flags: review?(ted) → review+
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
(Assignee)

Updated

3 years ago
Keywords: checkin-needed

Comment 30

3 years ago
bugherder
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
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox48: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
(Assignee)

Updated

2 years ago
Blocks: 1205865

Updated

2 years ago
Depends on: 1272877

Updated

2 years ago
Depends on: 1294753

Updated

2 years ago
Depends on: 1298750
You need to log in before you can comment on or make changes to this bug.