1084441 - Fix up test_can_play_type_mpeg.html for Android

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[James Willcox (:snorp) ([e-mail]) (he/him)]

Bug 1014614 adds fragmented MP4 support for Android Jelly Bean and higher. We need to handle that correctly in test_can_play_type_mpeg.html

Comment 1

[James Willcox (:snorp) ([e-mail]) (he/him)]

Attachment: Update test to reflect that MP4 is supported on Android Jelly Bean and higher

Comment 2

[Chris Pearce [:cpearce (Not reading bugmail)]]

Will we be using the MP4Reader on Android on our build/mochitest machines? With this change, will we no longer have test coverage for the old MP4 backend on android?

Comment 3

[James Willcox (:snorp) ([e-mail]) (he/him)]

(In reply to Chris Pearce (:cpearce) from comment #2)
> Will we be using the MP4Reader on Android on our build/mochitest machines?
> With this change, will we no longer have test coverage for the old MP4
> backend on android?

No, we actually have no Jelly Bean devices in our infrastructure, so we can't test the new code currently.

Comment 4

[James Willcox (:snorp) ([e-mail]) (he/him)]

Attachment: Test for MP4 and MP3 support on Android

This version should test both the old omx-plugin backend as well as the new one (whichever is enabled)

Comment 5

[James Willcox (:snorp) ([e-mail]) (he/him)]

This patch fails against current m-c, which is good, I think:

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=f4d979f67638

Here's a run that has the fix from bug 1082550, which should be green (no results yet):

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=f5d2ec0e76e1

Comment 6

[Chris Pearce [:cpearce (Not reading bugmail)]]

(In reply to James Willcox (:snorp) ([e-mail]) from comment #3)
> (In reply to Chris Pearce (:cpearce) from comment #2)
> > Will we be using the MP4Reader on Android on our build/mochitest machines?
> > With this change, will we no longer have test coverage for the old MP4
> > backend on android?
> 
> No, we actually have no Jelly Bean devices in our infrastructure, so we
> can't test the new code currently.

Shipping without regression testing in place seems unwise to me.

Comment 7

[James Willcox (:snorp) ([e-mail]) (he/him)]

(In reply to Chris Pearce (:cpearce) from comment #6)
> (In reply to James Willcox (:snorp) ([e-mail]) from comment #3)
> > (In reply to Chris Pearce (:cpearce) from comment #2)
> > > Will we be using the MP4Reader on Android on our build/mochitest machines?
> > > With this change, will we no longer have test coverage for the old MP4
> > > backend on android?
> > 
> > No, we actually have no Jelly Bean devices in our infrastructure, so we
> > can't test the new code currently.
> 
> Shipping without regression testing in place seems unwise to me.

Indeed. We are working on getting mochitest (or rather, a subset) going on Autophone, though, which should cover this area.

Comment 8

[James Willcox (:snorp) ([e-mail]) (he/him)]

My try run that enabled the MP4 checks for OmxPlugin failed. Some logcat messages indicate that maybe OMX is not present or busted on the pandaboards:

12:39:58     INFO -  01-01 00:00:14.992 E/        ( 1294): android::TIOMXPlugin::TIOMXPlugin(): failed to load libOMX_Core.so
12:39:58     INFO -  01-01 00:00:14.992 E/        ( 1294): mLibHandle is NULL!
12:39:58     INFO -  01-01 00:00:14.992 E/OMXMaster( 1294): OMX plugin failed w/ error 0x80001001 after registering 0 components

So...that's not good. We're going to want to run those media mochitests in Autophone on everything.

Comment 9

[James Willcox (:snorp) ([e-mail]) (he/him)]

I'm going to wait until we have something running our tests than can actually play mp4 before doing something here.

Comment 10

[James Willcox (:snorp) ([e-mail]) (he/him)]

Attachment: Test for ability to play MP4 on Android Jelly Bean and higher

Comment 11

[James Willcox (:snorp) ([e-mail]) (he/him)]

Autophone is running video tests for the new stuff available on Jelly Bean and above. Make sure we assert that MP4 works there.

Comment 12

[James Willcox (:snorp) ([e-mail]) (he/him)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=46c8a578349d

Comment 13

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8571599 [details] [diff] [review]
Test for ability to  play MP4 on Android Jelly Bean and higher

Review of attachment 8571599 [details] [diff] [review]:
-----------------------------------------------------------------

I don't see a androidVersion variable defined anywhere, so I'll assume it's defined in something included...

Comment 14

[James Willcox (:snorp) ([e-mail]) (he/him)]

(In reply to Chris Pearce (:cpearce) from comment #13)
> Comment on attachment 8571599 [details] [diff] [review]
> Test for ability to  play MP4 on Android Jelly Bean and higher
> 
> Review of attachment 8571599 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I don't see a androidVersion variable defined anywhere, so I'll assume it's
> defined in something included...

Yeah, actually I think that's wrong now that I'm looking for where that would be defined...

Comment 15

[James Willcox (:snorp) ([e-mail]) (he/him)]

OK, androidVersion is actually correct. Try run going here:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=427daf3c22ad

Comment 16

[James Willcox (:snorp) ([e-mail]) (he/him)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9d8115c1906a

Comment 17

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/9d8115c1906a