1286133 - Play HLS video in an Android VideoView

Status: RESOLVED FIXED

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

Description

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: android_video_player.patch

Rather than try to open a potentially non-existant external player, we could create an android VideoView and layer it over the content at the same size as the video element. This has all of the crappy scrolling problems of plugin content, but it seems strictly better than kicking our users out to another app and certainly better than telling them to use another app which isn't on their phone.

I've attached a proof of concept patch that creates and plays the video in an android VideoView, but doesn't attempt to handle scrolling or tab switching.

Comment 1

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: 2016-07-12 03.26.37.png

screen shot taken from http://www.newshub.co.nz/business/powerball-winners-cash-in-tickets-2016071211#axzz4EA1uumyJ

Screenshotting a video is a little silly, but just in case I didn't describe it well, this is what it looks like. The news anchors are the HLS video, playing in the android VideoView widget positioned over the HTML video element. (the red tint is my twightlight app)

Comment 2

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Evan, you were saying maybe you could try to pick up Android skill again. Would you be able to work on this? Don't worry if this is beyond your technical knowledge.

Comment 3

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Do we need to re-implement video controls in Java if we want to ship this?

Comment 4

[Mike Taylor [:miketaylr]]

From a user-perspective, this approach seems much nicer.

Comment 5

[Evan Tseng [:evanxd]]

(In reply to Tim Guan-tin Chien [:timdream] (please needinfo) from comment #2)
> Evan, you were saying maybe you could try to pick up Android skill again.
> Would you be able to work on this? Don't worry if this is beyond your
> technical knowledge.

Hi Tim,

I will focus on Firefox Desktop first, thanks for telling me the interesting features we would like to implement.

Anyone, please feel free to take this bug. :)

Comment 6

[Rachelle Yang [:ryang][[e-mail]]]

Hi James, 

May you please kindly help us find the right person to review the patch of attachment 8769966 [details] [diff] [review] ?
Thank you very much !

Comment 7

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

Attachment: Use Android VideoView to play HLS

Comment 8

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

I revised the patch a bit. Brad, Barbara, can you give this a try? It's still a bit rough, but seems to mostly work. APK here: https://people.mozilla.org/~jwillcox/hls.apk

Comment 9

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

I made a little sample page with HLS that you can try here: https://thimbleprojects.org/snorp/89141/

Comment 10

[Barbara Bermes [:barbara] - NI please!]

This is great, I guess this only works for embedded videos though, if I click on a direct HLS video link, it still asks me to pick my preferred app to open it with.

Another thing I noticed there is no stop/exit button in that default Android Video View, so people need to use their phone back button. I assume that's a limitation with the Android video view.

Either way, I think this is a huge improvement. 

I think we've also talked about probes here, can we confirm we are keeping track of how many people actually use this as opposed to seeing an error or trying to open it with a different app?

Comment 11

[Blake Wu [:bwu][:blakewu]]

(In reply to Barbara Bermes [:barbara] from comment #10)
> This is great, I guess this only works for embedded videos though, if I
> click on a direct HLS video link, it still asks me to pick my preferred app
> to open it with.
I think that is the original design since from Bug 780379. That's why bug 1282410 tried to follow the same behavior but it turns out it may confuse users..  
> 
> Another thing I noticed there is no stop/exit button in that default Android
> Video View, so people need to use their phone back button. I assume that's a
> limitation with the Android video view.
Should we ask UX guys to check it?
> 
> Either way, I think this is a huge improvement. 
> 
> I think we've also talked about probes here, can we confirm we are keeping
> track of how many people actually use this as opposed to seeing an error or
> trying to open it with a different app?
We have telemetry (Bug 1262659) to know how many people see this errors, but no data for that case of opening it with a different app.

Comment 12

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8775378 [details] [diff] [review]
Use Android VideoView to play HLS

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

::: mobile/android/base/java/org/mozilla/gecko/BrowserApp.java
@@ +1989,5 @@
> +                        VideoView view = new VideoView(app);
> +                        android.widget.MediaController mediaController = new android.widget.MediaController(app);
> +                        view.setMediaController(mediaController);
> +                        view.setVideoURI(Uri.parse(uri));
> +                        app.addFullScreenPluginView(view);

You can access the outer class here using BrowserApp.this.addFullScreenPluginView(view);

::: mobile/android/chrome/content/browser.js
@@ +4717,5 @@
>        case 'MozMouseHittest':
>          this._handleRetargetedTouchStart(aEvent);
>          break;
> +      case 'OpenMediaWithExternalApp': {
> +        dump("SNORP: OpenMediaWithExternalApp! " + aEvent.target.currentSrc || aEvent.target.src);

nit: debug log?

@@ +4724,5 @@
> +          return;
> +        }
> +        let mediaSrc = aEvent.target.currentSrc || aEvent.target.src;
> +        let rect = aEvent.target.getBoundingClientRect();
> +        Cu.reportError("rect: " + JSON.stringify(rect));

nit: More debugging?

Comment 13

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

Attachment: Use Android VideoView to play HLS

Fixed up review comments and added a Telemetry call

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/fx-team/rev/cf23addcfa4d
Use Android VideoView to play HLS r=sebastian,jchen

Comment 15

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

Barbara, I've added a telemetry UI event for this which you should be able to see with your normal tools. The event is SHOW, method is CONTENT, and the extra string is 'playhls'.

Comment 16

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/cf23addcfa4d

Comment 17

[Marco Castelluccio [:marco]]

There are some issues with videos on m.ilmattino.it (bug 1247433):
- the video starts playing after a while with no indication that it's loading;
- only upper half of the video is shown initially;
- weird video controls are shown initially, they are then replaced by the Video View controls;

Comment 18

[Marco Castelluccio [:marco]]

Something strange happens on ilfattoquotidiano.it as well (bug 1274864).

When you load a page containing a video, it's automatically shown in full screen. In full screen, you can't use the slider to seek. If you exit full screen mode with the back button, an error is shown in place of the video element ("This video is either unavailable or not supported in this browser").

Should I file new bugs for the issues on these two websites?

Comment 19

[Mike Taylor [:miketaylr]]

> Should I file new bugs for the issues on these two websites?

That seems like good, probably they should block this bug as well.

Comment 20

[Brad Lassey [:blassey] (use needinfo?)]

Spoke to snorp earlier, but for everyone else's benefit, I'm concerned that the transition from the web page to a full screen video is jarring, especially when that fullscreen video is black for several seconds.

Comment 21

[Barbara Bermes [:barbara] - NI please!]

Are we saying this is not ready for prime time (49) yet?

Comment 22

[Brad Lassey [:blassey] (use needinfo?)]

I think we can land improvements on nightly and evaluate them for uplift. Sometime in the aurora cycle (before 49 goes to beta) we should evaluate whether it should ride the trains.

Comment 23

[Kevin Brosnan [Ex-Mozilla]]

Should we wrap this in channel specific code so that it is an explicit decision to release it?

Comment 24

[Marco Castelluccio [:marco]]

(In reply to Marco Castelluccio [:marco] (PTO until August 9) from comment #18)
> Something strange happens on ilfattoquotidiano.it as well (bug 1274864).

The same on www.independent.co.uk (bug 1243304), I guess it happens on every website which uses the Brightcove player.

Comment 25

[Ioana Chiorean]

Ninu's QA work will be tracked here- https://wiki.mozilla.org/QA/Fennec/Play_HLS

Comment 26

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hello Snorp, Barbara, is this a pref controlled feature? It landed in 50 and wondering if this will release with Fx50 or not and how much testing is needed to make that happen. Thanks!

Comment 27

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

(In reply to Ritu Kothari (:ritu) from comment #26)
> Hello Snorp, Barbara, is this a pref controlled feature? It landed in 50 and
> wondering if this will release with Fx50 or not and how much testing is
> needed to make that happen. Thanks!

It's not pref controled, and we expect it to launch in 50. Not too much testing needed, but SV folks have found some issues.

Comment 28

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

(In reply to James Willcox (:snorp) ([e-mail]) from comment #27)
> 
> It's not pref controled, and we expect it to launch in 50. Not too much
> testing needed, but SV folks have found some issues.

Got it. This is worth relnoting. Barbara, can you please help with suggested wording?

Comment 29

[Barbara Bermes [:barbara] - NI please!]

Added the updated wording in our relnotes word document

Comment 30

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Added to Fennec 50 Beta release notes.

Comment 31

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hi Ioana, how does the pre-aurora sign off for HLS video look? Are there any specific bugs you'd call blocking? If so, we can work with the dev team to get those fixed.

Comment 32

[Ioana Chiorean]

Hi Ritu, Ninu gathered the bugs related here https://wiki.mozilla.org/QA/Fennec/Play_HLS#Bug_Work and I know he had some concerns related to the signoff.

Comment 33

[u549602]

Hi everyone, my suggestion is that this feature should be kept under Aurora flag until all major issues are fixed (Bug 1301043, Bug 1295398, Bug 1301053) and decided upon (Bug 1301055,Bug 1302729). The before mentioned issues have a huge impact on the App and other features (Bug 1268368).
So, the overall status of this feature is [AT RISK], it works but it might have a major impact overall.

Comment 34

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hi Barbara, SoftVision is recommending we disable this for Beta50? What do you think? Do you believe we might be able to fix the blocking issues in the next week or two so as to consider shipping this in Fennec 50?

Mihai, could you please send a sign-off email with your recommendation (like other pre-Beta sign offs) to r-d ML? Thanks!

Comment 35

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

On release-drivers ML, I have requested we disable this feature for Fx50 and re-target for 51 release.

Comment 36

[Marco Castelluccio [:marco]]

Bug 941351 and linked bugs contains a list of websites that we could test.

Comment 37

[Mike Taylor [:miketaylr]]

Snorp, do you know what the status of this feature is? I'm guessing it didn't ship in 51...but maybe I'm looking at the wrong bugs.

Comment 38

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

I did ship...in 51 I believe.

Comment 39

[Mike Taylor [:miketaylr]]

OK, thanks. (I guess I could have figured that out... the flags on this bug are confusing tho)