1261007 - Enable/fix testcases for MediaRecorder api on Fennec.

Status: RESOLVED FIXED

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

Description

[Benjamin Chen [:bechen]]

See bug 1215115 comment 22.

I want to enable [test_mediarecorder_record_canvas_captureStream.html] and
[test_mediarecorder_record_changing_video_resolution.html] at all platforms.
And fix test_mediarecorder_webm_support.html since we remove the vorbis encoder.

Comment 1

[Benjamin Chen [:bechen]]

Attachment: MozReview Request: Bug 1261007: part1: Force to send video sample into encoder if we got the same video sample more than 1 seconds. Enable testcases. r=jolin

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

Comment 2

[Benjamin Chen [:bechen]]

Attachment: MozReview Request: Bug 1261007 - part2: fix test_mediarecorder_webm_support. r=rillian

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

Comment 3

[Ralph Giles (:rillian)]

Comment on attachment 8740855 [details]
MozReview Request: Bug 1261007 - part2: fix test_mediarecorder_webm_support. r=rillian

https://reviewboard.mozilla.org/r/46047/#review42601

::: dom/media/test/test_mediarecorder_webm_support.html:16
(Diff revision 1)
>  ok(MediaRecorder.isTypeSupported('video/webm'), 'Should support video/webm');
> -ok(MediaRecorder.isTypeSupported('video/webm; codecs="vp8, vorbis"'), 'Should support video/webm + vp8/vorbis');
> +ok(!MediaRecorder.isTypeSupported('video/webm; codecs="vp8, vorbis"'), 'Should not support video/webm + vp8/vorbis');
>  ok(!MediaRecorder.isTypeSupported('video/webm; codecs="vp9, vorbis"'), 'Should not support video/webm + vp9/vorbis');
> +ok(MediaRecorder.isTypeSupported('video/webm; codecs="vp8, opus"'), 'Should support video/webm + vp8/vorbis');
> +ok(!MediaRecorder.isTypeSupported('video/webm; codecs="vp9, opus"'), 'Should not support video/webm + vp9/vorbis');
>  </script>

Message is incorrect for the two new checks. It should be updated to reference opus instead of vorbis. For example:

```
ok(MediaRecorder.isTypeSupported('video/webm; codecs="vp8, opus"'), 'Should support video/webm + vp8/opus');
ok(!MediaRecorder.isTypeSupported('video/webm; codecs="vp9, opus"'), 'Should not support video/webm + vp9/opus');
```

Comment 4

[John Lin [:jhlin][:jolin]]

Comment on attachment 8740854 [details]
MozReview Request: Bug 1261007: part1: Force to send video sample into encoder if we got the same video sample more than 1 seconds. Enable testcases. r=jolin

https://reviewboard.mozilla.org/r/46045/#review42855

Now that TrackEncoder sends only unique images, the logic in OmxVideoTrackEncoder::GetEncodedTrack() seems unnecessary and it would be nice to be removed.

::: dom/media/test/mochitest.ini:705
(Diff revision 1)
>  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
>  tags=msg

This would break B2G but I guess we don't care anymore.

::: dom/media/test/mochitest.ini
(Diff revision 1)
>  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
>  tags=msg
>  [test_mediarecorder_record_changing_video_resolution.html]
> -skip-if = android_version < '17' # Android/Gonk before SDK version 17 does not have the OMX Encoder API.

Ditto.

Comment 5

[John Lin [:jhlin][:jolin]]

Comment on attachment 8740854 [details]
MozReview Request: Bug 1261007: part1: Force to send video sample into encoder if we got the same video sample more than 1 seconds. Enable testcases. r=jolin

Forgot to click 'Ship it!'. :$

Comment 6

[Benjamin Chen [:bechen]]

Attachment: MozReview Request: Bug 1261007 - part3: Remove the same/redundant code of checking the unique image. r=jolin

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

Comment 7

[Benjamin Chen [:bechen]]

Comment on attachment 8740855 [details]
MozReview Request: Bug 1261007 - part2: fix test_mediarecorder_webm_support. r=rillian

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/46047/diff/1-2/

Comment 8

[John Lin [:jhlin][:jolin]]

Comment on attachment 8740854 [details]
MozReview Request: Bug 1261007: part1: Force to send video sample into encoder if we got the same video sample more than 1 seconds. Enable testcases. r=jolin

https://reviewboard.mozilla.org/r/46045/#review43601

Comment 9

[John Lin [:jhlin][:jolin]]

Comment on attachment 8742209 [details]
MozReview Request: Bug 1261007 - part3: Remove the same/redundant code of checking the unique image. r=jolin

https://reviewboard.mozilla.org/r/46975/#review43603

LGTM. Thanks!

Comment 10

[John Lin [:jhlin][:jolin]]

https://reviewboard.mozilla.org/r/46045/#review43605

Comment 11

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/482546446203
https://hg.mozilla.org/integration/mozilla-inbound/rev/5e3441e1a4c1
https://hg.mozilla.org/integration/mozilla-inbound/rev/a1da3524fc25

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/482546446203
https://hg.mozilla.org/mozilla-central/rev/5e3441e1a4c1
https://hg.mozilla.org/mozilla-central/rev/a1da3524fc25