Closed Bug 1286133 Opened 4 years ago Closed 3 years ago

Play HLS video in an Android VideoView

Categories

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

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 50
Tracking Status
relnote-firefox --- 50+
fennec 49+ ---
firefox50 --- fixed

People

(Reporter: blassey, Unassigned)

References

(Depends on 5 open bugs)

Details

Attachments

(3 files, 1 obsolete file)

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.
Attached image 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)
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.
Flags: needinfo?(evan)
Component: General → Audio/Video
Do we need to re-implement video controls in Java if we want to ship this?
From a user-perspective, this approach seems much nicer.
(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. :)
Flags: needinfo?(evan)
tracking-fennec: --- → ?
tracking-fennec: ? → 49+
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 !
Flags: needinfo?(snorp)
Attachment #8775378 - Flags: review?(s.kaspari)
Attachment #8775378 - Flags: feedback?(blassey.bugs)
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
Flags: needinfo?(snorp)
Flags: needinfo?(blassey.bugs)
Flags: needinfo?(bbermes)
I made a little sample page with HLS that you can try here: https://thimbleprojects.org/snorp/89141/
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?
Flags: needinfo?(bbermes)
(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 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?
Attachment #8775378 - Flags: review?(s.kaspari) → review+
Fixed up review comments and added a Telemetry call
Attachment #8776156 - Flags: review+
Attachment #8775378 - Attachment is obsolete: true
Attachment #8775378 - Flags: feedback?(blassey.bugs)
Pushed by jwillcox@mozilla.com:
https://hg.mozilla.org/integration/fx-team/rev/cf23addcfa4d
Use Android VideoView to play HLS r=sebastian,jchen
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'.
https://hg.mozilla.org/mozilla-central/rev/cf23addcfa4d
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 50
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;
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?
> Should I file new bugs for the issues on these two websites?

That seems like good, probably they should block this bug as well.
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.
Flags: needinfo?(blassey.bugs)
Are we saying this is not ready for prime time (49) yet?
Flags: needinfo?(snorp)
Flags: needinfo?(blassey.bugs)
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.
Flags: needinfo?(blassey.bugs)
Should we wrap this in channel specific code so that it is an explicit decision to release it?
(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.
QA Contact: mihai.ninu
Ninu's QA work will be tracked here- https://wiki.mozilla.org/QA/Fennec/Play_HLS
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!
Flags: needinfo?(bbermes)
Depends on: 1301043
Depends on: 1301050
Depends on: 1301053
Depends on: 1301055
Depends on: 1301326
(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.
Flags: needinfo?(snorp)
Flags: needinfo?(bbermes)
(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) 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?
Flags: needinfo?(bbermes)
Added the updated wording in our relnotes word document
Flags: needinfo?(bbermes)
Added to Fennec 50 Beta release notes.
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.
Flags: needinfo?(ioana.chiorean)
No longer blocks: 1295398
Depends on: 1295398
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.
Flags: needinfo?(ioana.chiorean) → needinfo?(mihai.ninu)
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.
Flags: needinfo?(mihai.ninu)
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!
Flags: needinfo?(mihai.ninu)
Flags: needinfo?(bbermes)
Flags: needinfo?(mihai.ninu)
On release-drivers ML, I have requested we disable this feature for Fx50 and re-target for 51 release.
Depends on: 1307074
Depends on: 1307076
Depends on: 1307078
Depends on: 1308205
Depends on: 1310899
Bug 941351 and linked bugs contains a list of websites that we could test.
Depends on: 1313387
Flags: needinfo?(bbermes)
Depends on: 1316876
Blocks: 1316876
No longer depends on: 1316876
Depends on: 1321242
Depends on: 1321243
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.
Flags: needinfo?(snorp)
I did ship...in 51 I believe.
Flags: needinfo?(snorp)
OK, thanks. (I guess I could have figured that out... the flags on this bug are confusing tho)
Depends on: 1354896
You need to log in before you can comment on or make changes to this bug.