Make MediaRecorder work with canvas captureStream on Android and B2G

RESOLVED FIXED in Firefox 44

Status

()

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: pehrsons, Assigned: pehrsons)

Tracking

33 Branch
mozilla44
Points:
---

Firefox Tracking Flags

(firefox44 fixed)

Details

Attachments

(2 attachments)

(Assignee)

Description

3 years ago
Bug 1182426 makes canvas capturestreams work with MediaRecorder when using the VP8TrackEncoder.

I made a patch also for the OMXCodecWrapper, but for some reason it is not working, as seen in [1]. When testing on a Flame I got the following in the log:

> I/Browser ( 2432): Content JS INFO: PASS: Media recorder stream = canvas stream at the start of recording
> I/Browser ( 2432):     at is (https://rawgit.com/Pehrsons/d24d6d97984144e85705/raw/eb8d0552cc72b8e936b73020943eb357159e03db/capturestream-recorder.html:20:5)
> I/Browser ( 2432): Content JS INFO: PASS: Media recorder should be recording
> I/Browser ( 2432):     at is (https://rawgit.com/Pehrsons/d24d6d97984144e85705/raw/eb8d0552cc72b8e936b73020943eb357159e03db/capturestream-recorder.html:20:5)
> I/Gecko   ( 2432): [GFX1-]: Invalid draw target type specified: 0
> I/Gecko   ( 2432): [GFX1-]: Failed GetAsDrawTarget true, 0xaf622000 + 16, Size(100,100), 400, 0
> E/OMXMaster(  201): A component of name 'OMX.qcom.audio.decoder.aac' already exists, ignoring this one.
> I/OMXClient( 2432): Using client-side OMX mux.
> E/OMX-VENC-720p(  201): set_parameter: metamode is valid for input port only
> E/OMXNodeInstance(  201): OMX_SetParameter() failed for StoreMetaDataInBuffers: 0x8000101a
> E/ACodec  ( 2432): [OMX.qcom.video.encoder.avc] storeMetaDataInBuffers (output) failed w/ err -2147483648
> I/ACodec  ( 2432): setupVideoEncoder succeeded
> E/OMX-VENC-720p(  201): VIDIOC_REQBUFS CAPTURE_MPLANE Failed

The MediaRecorder in this case didn't even raise the "error" event, which it probably should have. Instead it looks like it silently failed as the tests in [1] timed out.

[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=cdc9c3513112
(Assignee)

Comment 1

3 years ago
Created attachment 8668263 [details]
MozReview Request: Bug 1210286 - Fall back to converting SourceSurfaces (RGB) to NV12 in OMXCodecWrapper. r?jolin

Bug 1210286 - Fall back to converting SourceSurfaces (RGB) to NV12 in OMXCodecWrapper. r?jolin
(Assignee)

Comment 2

3 years ago
Created attachment 8668264 [details]
MozReview Request: Bug 1210286 - Enable MediaRecorder with Canvas CaptureStream on Android and B2G. r?jolin

Bug 1210286 - Enable MediaRecorder with Canvas CaptureStream on Android and B2G. r?jolin
(Assignee)

Comment 3

3 years ago
John, here's the patch for converting SourceSurface to NV12 for the OMX encoder. If you want to test, they need to go on top of bug 1182426 (for the mochitests). Any hints you have to why it doesn't work (see comment 0) are much appreciated!
Flags: needinfo?(jolin)
https://reviewboard.mozilla.org/r/20941/#review18925

I got some compile error in this patch. Could you please help check again?

::: dom/media/omx/OMXCodecWrapper.cpp:20
(Diff revision 1)
> +#include "libyuv.h"

"file not found" error for this. Perhaps adding its path in moz.build is needed?

::: dom/media/omx/OMXCodecWrapper.cpp:397
(Diff revision 1)
> +  RefPtr<SourceSurface> surf = img->GetAsSourceSurface();

compile error: |img| not found.

::: dom/media/omx/OMXCodecWrapper.cpp:399
(Diff revision 1)
> +    CODEC_ERROR("Getting surface %s from image failed", Stringify(format).c_str());

|format| is not found either.
(Assignee)

Comment 5

3 years ago
I recognise those. Obviously I have the fixes only on my b2g-building VM. Sorry about that, I'll have a new version up shortly.

The builds passed on try for Android and B2G emulator though, are we not using OMXCodecWrapper there? I thought we at least used some software backed OMX codec on B2G emulator?
(Assignee)

Comment 6

3 years ago
Comment on attachment 8668263 [details]
MozReview Request: Bug 1210286 - Fall back to converting SourceSurfaces (RGB) to NV12 in OMXCodecWrapper. r?jolin

Bug 1210286 - Fall back to converting SourceSurfaces (RGB) to NV12 in OMXCodecWrapper. r?jolin
(Assignee)

Comment 7

3 years ago
Comment on attachment 8668264 [details]
MozReview Request: Bug 1210286 - Enable MediaRecorder with Canvas CaptureStream on Android and B2G. r?jolin

Bug 1210286 - Enable MediaRecorder with Canvas CaptureStream on Android and B2G. r?jolin
(Assignee)

Comment 8

3 years ago
Patches should compile now, but before you run it I'll have to figure out what's wrong with bug 1182426.
(Assignee)

Comment 9

3 years ago
Ok, feel free to try again. You only need to update the patches from bug 1182426 if you're going to run mochitests on the device.

Otherwise you can use my self contained test page at https://rawgit.com/Pehrsons/d24d6d97984144e85705/raw/eb8d0552cc72b8e936b73020943eb357159e03db/capturestream-recorder.html
(Assignee)

Updated

3 years ago
Attachment #8668263 - Flags: review?(jolin)
(Assignee)

Comment 11

3 years ago
Comment on attachment 8668263 [details]
MozReview Request: Bug 1210286 - Fall back to converting SourceSurfaces (RGB) to NV12 in OMXCodecWrapper. r?jolin

Bug 1210286 - Fall back to converting SourceSurfaces (RGB) to NV12 in OMXCodecWrapper. r?jolin
(Assignee)

Comment 12

3 years ago
Comment on attachment 8668264 [details]
MozReview Request: Bug 1210286 - Enable MediaRecorder with Canvas CaptureStream on Android and B2G. r?jolin

Bug 1210286 - Enable MediaRecorder with Canvas CaptureStream on Android and B2G. r?jolin
Attachment #8668264 - Flags: review?(jolin)
https://reviewboard.mozilla.org/r/20941/#review19049

I test it on Flame and it works flawlessly. Good job Andreas.

::: dom/media/omx/OMXCodecWrapper.cpp:434
(Diff revision 2)
> +      NS_ASSERTION(false, "Unsupported SourceSurface format");

Nit: use MOZ_ASSERT() instead?
Comment on attachment 8668264 [details]
MozReview Request: Bug 1210286 - Enable MediaRecorder with Canvas CaptureStream on Android and B2G. r?jolin

https://reviewboard.mozilla.org/r/20945/#review19051
Attachment #8668264 - Flags: review?(jolin) → review+
Comment on attachment 8668263 [details]
MozReview Request: Bug 1210286 - Fall back to converting SourceSurfaces (RGB) to NV12 in OMXCodecWrapper. r?jolin

https://reviewboard.mozilla.org/r/20943/#review19053
Attachment #8668263 - Flags: review?(jolin) → review+
Flags: needinfo?(jolin)
(Assignee)

Comment 16

3 years ago
Thanks John!

I also managed to figure out why the tests were failing on b2g emulator (and presumably Android, but I didn't get any useful logs from there).

See the B2G M-6 emulator-log here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1a6872ba3a4b

> 10-05 06:03:37.800 I/PRLog   (  852): 1074558060[40448000]: MediaRecorder.Start 4637bd60
> 10-05 06:03:37.800 I/PRLog   (  852): 1074558060[40448000]: Session.Start 4630da00
> 10-05 06:03:37.809 I/GeckoDump(  852): ⰲ겿{"action":"test_status","time":1444025017806,"thread":null,"pid":null,"source":"mochitest","test":"/tests/dom/media/test/test_mediarecorder_record_canvas_captureStream.html","subtest":"Media recorder should be recording","status":"PASS","js_source":"TestRunner.js"}ⰲ겿
> 10-05 06:03:37.930 I/PRLog   (  852): 1074558060[40448000]: Session.NotifyTracksAvailable track type = (2)
> 10-05 06:03:37.930 I/PRLog   (  852): 1074558060[40448000]: Session.InitEncoder 4630da00
> 10-05 06:03:37.940 I/PRLog   (  852): 1074558060[40448000]: Can not find any encoder to record this media stream

Relevant code is here: https://dxr.mozilla.org/mozilla-central/rev/3d7532ce81ac571962abc3b99582fe7f5d685192/dom/media/encoder/MediaEncoder.cpp#145

Shouldn't there be either the WEBM or OMX encoder on Android/B2G-emulator? John, do you perhaps know more context here?

It seems like we're running most other mediarecorder tests on B2G, but how can they be passing without an encoder...
Flags: needinfo?(jolin)
(Assignee)

Comment 17

3 years ago
For the record we concluded offline that the OMX encoder is only available for Android SDK versions 17 and up (see [1]) and WebM encoding on mobile is more of a political potato that we won't deal with here.

[1] https://dxr.mozilla.org/mozilla-central/rev/d56c816b35c34133a9249663b2c19830a29b133a/configure.in#247
Flags: needinfo?(jolin)
(Assignee)

Comment 18

3 years ago
Comment on attachment 8668263 [details]
MozReview Request: Bug 1210286 - Fall back to converting SourceSurfaces (RGB) to NV12 in OMXCodecWrapper. r?jolin

Bug 1210286 - Fall back to converting SourceSurfaces (RGB) to NV12 in OMXCodecWrapper. r?jolin
(Assignee)

Comment 19

3 years ago
Comment on attachment 8668264 [details]
MozReview Request: Bug 1210286 - Enable MediaRecorder with Canvas CaptureStream on Android and B2G. r?jolin

Bug 1210286 - Enable MediaRecorder with Canvas CaptureStream on Android and B2G. r?jolin
(Assignee)

Updated

3 years ago
Attachment #8668264 - Flags: review+ → review?(jolin)
(Assignee)

Updated

3 years ago
Assignee: nobody → pehrsons
Status: NEW → ASSIGNED
Attachment #8668264 - Flags: review?(jolin)
Comment on attachment 8668264 [details]
MozReview Request: Bug 1210286 - Enable MediaRecorder with Canvas CaptureStream on Android and B2G. r?jolin

https://reviewboard.mozilla.org/r/20945/#review19683

::: dom/media/test/mochitest.ini:688
(Diff revision 3)
> -skip-if = (toolkit == 'android' || toolkit == 'gonk') # Bug 1210286 to fix canvas capturestream support in the OMX backend
> +skip-if = android_version < '17' # Android/Gonk before SDK version 17 does not have the OMX Encoder API.

Dump question: what android_version would be on platforms other than Android and B2G?
(Assignee)

Comment 22

3 years ago
(In reply to John Lin [:jolin][:jhlin] from comment #21)
> ::: dom/media/test/mochitest.ini:688
> (Diff revision 3)
> > -skip-if = (toolkit == 'android' || toolkit == 'gonk') # Bug 1210286 to fix canvas capturestream support in the OMX backend
> > +skip-if = android_version < '17' # Android/Gonk before SDK version 17 does not have the OMX Encoder API.
> 
> Dump question: what android_version would be on platforms other than Android
> and B2G?

It'll be undefined, so basically the less than operator returns false.

See https://dxr.mozilla.org/mozilla-central/search?q=android_version+path%3Atesting%2F for where it gets defined.

The tests are not skipped on my Mac so seems to be true even :-)
(Assignee)

Updated

3 years ago
Attachment #8668264 - Flags: review?(jolin)
Comment on attachment 8668264 [details]
MozReview Request: Bug 1210286 - Enable MediaRecorder with Canvas CaptureStream on Android and B2G. r?jolin

https://reviewboard.mozilla.org/r/20945/#review19687

Great! Thanks a lot for the info.
Attachment #8668264 - Flags: review?(jolin) → review+
https://hg.mozilla.org/mozilla-central/rev/da0ad0926084
https://hg.mozilla.org/mozilla-central/rev/c95bfb27dec9
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox44: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
You need to log in before you can comment on or make changes to this bug.