1372465 - [Fennec][HLS][Test] Make sure GeckoHLSPlayer is used by correct MediaResource/MediaDemuxer pair.

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Audio/Video, defect, P3)

Description

[Kilik Kuo [:kikuo] (inactive)]

Per Bug 1368954 Comment 19, A test should be provided to make sure GeckoHLSPlayer is used between correct MediaResource / MediaDemuxer pair.

Comment 1

[Kilik Kuo [:kikuo] (inactive)]

Attachment: Bug 1372465 - Make sure GeckoHlsPlayer is used by correct media components.

We use an increasing player Id as the token between MediaReousrce and MediaDemuxer to
identify which GeckoHlsPlayer should be used.
In this case, playback of 2 HLS content at the same page to make sure the underlying components won't be selected mistakenly

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

Comment 2

[Kilik Kuo [:kikuo] (inactive)]

Comment on attachment 8888193 [details]
Bug 1372465 - Make sure GeckoHlsPlayer is used by correct media components.

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

Comment 3

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8888193 [details]
Bug 1372465 - Make sure GeckoHlsPlayer is used by correct media components.

https://reviewboard.mozilla.org/r/159128/#review164522

::: dom/media/test/test_hls_player_independency.html:35
(Diff revision 2)
> +  var v4x3_videoHeight = 0;
> +  var v16x9_videoWidth = 0;
> +  var v16x9_videoHeight = 0;
> +
> +  function isDone() {
> +    return v16x9_videoWidth == 416 && v16x9_videoHeight == 234 && v4x3_videoWidth == 400 && v4x3_videoHeight == 300;

The test case will time out if width/heigh goes wrong. I would prefer:
1. check width and hieght in 'ended' handlers respectively.
2. call SimpleTest.finish() when we get 2 'ended' events.

Comment 4

[Kilik Kuo [:kikuo] (inactive)]

Comment on attachment 8888193 [details]
Bug 1372465 - Make sure GeckoHlsPlayer is used by correct media components.

https://reviewboard.mozilla.org/r/159128/#review164522

> The test case will time out if width/heigh goes wrong. I would prefer:
> 1. check width and hieght in 'ended' handlers respectively.
> 2. call SimpleTest.finish() when we get 2 'ended' events.

Thanks !  That's neater

Comment 5

[Kilik Kuo [:kikuo] (inactive)]

Comment on attachment 8888193 [details]
Bug 1372465 - Make sure GeckoHlsPlayer is used by correct media components.

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

Comment 6

[Kilik Kuo [:kikuo] (inactive)]

Comment on attachment 8888193 [details]
Bug 1372465 - Make sure GeckoHlsPlayer is used by correct media components.

https://reviewboard.mozilla.org/r/159128/#review164556

It's more informative to use is() instead of ok()

Comment 7

[Kilik Kuo [:kikuo] (inactive)]

Comment on attachment 8888193 [details]
Bug 1372465 - Make sure GeckoHlsPlayer is used by correct media components.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/159128/diff/3-4/

Comment 8

[Kilik Kuo [:kikuo] (inactive)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5da3d2a15f40&selectedJob=115907803

looks like something weird happening on Android 4.2 API 15+ ... investigating

Comment 9

[Kilik Kuo [:kikuo] (inactive)]

Comment on attachment 8888193 [details]
Bug 1372465 - Make sure GeckoHlsPlayer is used by correct media components.

https://reviewboard.mozilla.org/r/159128/#review166176

Test failed on Fennec (Android 4.2), becase only 1 decoder can be created when API version < 18 [1].
I think it's ok to skip this test on 4.2 since we're not able to play 2 video at the same time no matter it's HLS or not.

[1] http://searchfox.org/mozilla-central/rev/3a3af33f513071ea829debdfbc628caebcdf6996/dom/media/MediaPrefs.h#224-227

Comment 10

[Kilik Kuo [:kikuo] (inactive)]

Comment on attachment 8888193 [details]
Bug 1372465 - Make sure GeckoHlsPlayer is used by correct media components.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/159128/diff/4-5/

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1eaacee91b1b
Make sure GeckoHlsPlayer is used by correct media components. r=jwwang

Comment 12

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

https://hg.mozilla.org/mozilla-central/rev/1eaacee91b1b