Closed Bug 1210286 Opened 4 years ago Closed 4 years ago

Make MediaRecorder work with canvas captureStream on Android and B2G

Categories

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

33 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla44
Tracking Status
firefox44 --- fixed

People

(Reporter: pehrsons, Assigned: pehrsons)

References

Details

Attachments

(2 files)

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
Bug 1210286 - Fall back to converting SourceSurfaces (RGB) to NV12 in OMXCodecWrapper. r?jolin
Bug 1210286 - Enable MediaRecorder with Canvas CaptureStream on Android and B2G. r?jolin
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.
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?
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
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
Patches should compile now, but before you run it I'll have to figure out what's wrong with bug 1182426.
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
Attachment #8668263 - Flags: review?(jolin)
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
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)
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)
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)
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
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+ → review?(jolin)
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?
(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 :-)
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
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
You need to log in before you can comment on or make changes to this bug.