VideoPlayer doesn't work at all, preventing video playback on devices without YouTube application

VERIFIED FIXED in Firefox 32

Status

()

defect
VERIFIED FIXED
6 years ago
3 years ago

People

(Reporter: rnewman, Assigned: rnewman)

Tracking

(Blocks 1 bug, {relnote})

Trunk
Firefox 34
All
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox26 wontfix, firefox28 wontfix, firefox31 wontfix, firefox32 verified, firefox33 verified, firefox34 verified)

Details

(Whiteboard: kindle)

Attachments

(2 attachments, 1 obsolete attachment)

Assignee

Description

6 years ago
Tested in Beta and Nightly, and no YouTube video will play. Doesn't crash, but only get a blank screen.

Need to get some logs for this; needinfo for myself.
Assignee

Updated

6 years ago
Flags: needinfo?(rnewman)
For those following along at home, the Kindle's don't have a YouTube player app, so we ship our own barebones VideoPlayer activity. It tries to parse out the video stream from the YouTube URL.
What happened to VideoPlayer.java (reminisce down bug 847839 lane via patch 1a)?
Assignee

Comment 3

6 years ago
My hypothesis is that it stopped working (or never worked), because we almost exclusively all run devices with YouTube players and Flash. Either that, or I regressed something in the stonkingly simple patch in Bug 886014.

We'll see when I sit down tomorrow and get a log...
Assignee

Comment 4

6 years ago
exception
android.os.NetworkOnMainThreadException
	at android.os.StrictMode$AndroidBlockGuardPolicy.onNetwork(StrictMode.java:1117)
	at java.net.InetAddress.lookupHostByName(InetAddress.java:385)
	at java.net.InetAddress.getAllByNameImpl(InetAddress.java:236)
	at java.net.InetAddress.getAllByName(InetAddress.java:214)
	at libcore.net.http.HttpConnection.<init>(HttpConnection.java:70)
	at libcore.net.http.HttpConnection.<init>(HttpConnection.java:51)
	at libcore.net.http.HttpConnection$Address.connect(HttpConnection.java:359)
	at libcore.net.http.HttpConnectionPool.get(HttpConnectionPool.java:86)
	at libcore.net.http.HttpConnection.connect(HttpConnection.java:128)
	at libcore.net.http.HttpEngine.openSocketConnection(HttpEngine.java:316)
	at libcore.net.http.HttpEngine.connect(HttpEngine.java:311)
	at libcore.net.http.HttpEngine.sendSocketRequest(HttpEngine.java:290)
	at libcore.net.http.HttpEngine.sendRequest(HttpEngine.java:240)
	at libcore.net.http.HttpURLConnectionImpl.getResponse(HttpURLConnectionImpl.java:282)
	at libcore.net.http.HttpURLConnectionImpl.getInputStream(HttpURLConnectionImpl.java:177)
	at org.mozilla.gecko.VideoPlayer.getSpecFromYouTubeVideoID(VideoPlayer.java:69)
	at org.mozilla.gecko.VideoPlayer.onCreate(VideoPlayer.java:50)
Flags: needinfo?(rnewman)
Assignee

Comment 5

6 years ago
Posted patch Part 1: initial fixes. v1 (obsolete) — Splinter Review
It turns out that VideoPlayer is borked for more than just StrictMode violations.

I fixed that, and now it seems like YouTube has changed their URI format. So I fixed that. And now the player UI loads some of the video, doesn't seem to play anything, and doesn't correctly respond to rotations.

Here's a patch, but we're not done yet.

mfinkle gets review, 'cos he's the only person in blame who's still on the team!
Assignee: nobody → rnewman
Status: NEW → ASSIGNED
Attachment #831239 - Flags: review?(mark.finkle)
Assignee

Comment 6

6 years ago
Note that until we finish this bug (i.e., figure out why videos aren't playing), I suggest we don't ship VideoPlayer at all, and instead fire a VIEW intent to let the system browser or other apps try.

People do sideload Firefox onto these devices, so continuing to ship something totally broken seems like a bad choice.
Assignee

Comment 7

6 years ago
Also need to check this on an AOSP device other than the Kindle, see if it's just as borked there.
Comment on attachment 831239 [details] [diff] [review]
Part 1: initial fixes. v1

The only part is was wondering about is the ThreadUtils part. GeckoApp sets some of the ThreadUtils data members, IIRC. Are we OK to be using ThreadUtils from VideoPlayer?

I guess those statics would have been setup since VideoPlayer is always launched from Fennec, which initializes them.
Attachment #831239 - Flags: review?(mark.finkle) → review+
Assignee

Comment 9

6 years ago
(In reply to Mark Finkle (:mfinkle) from comment #8)

> I guess those statics would have been setup since VideoPlayer is always
> launched from Fennec, which initializes them.

The only thing that wouldn't be set is sUiThread -- the background thread will be created by GeckoBackgroundThread if needed.

But yes, so long as GeckoApp runs first, we should be fine here.
Assignee

Comment 10

6 years ago
Proposition: we should throw away VideoPlayer and ship

http://code.google.com/p/android-youtube-player/wiki/OpenYouTubePlayerActiviyInstructions

Alternatively, we should investigate whether playing the content from the mobile version of youtube.com is satisfactory.

Thoughts?
Assignee

Updated

6 years ago
Flags: needinfo?(blassey.bugs)
Assignee

Updated

6 years ago
Flags: needinfo?(mark.finkle)
(In reply to Richard Newman [:rnewman] from comment #10)
> Proposition: we should throw away VideoPlayer and ship
> 
> http://code.google.com/p/android-youtube-player/wiki/
> OpenYouTubePlayerActiviyInstructions
What makes you think this is going to be maintained?
 
> Alternatively, we should investigate whether playing the content from the
> mobile version of youtube.com is satisfactory.
Requesting the HTML5 video version of YouTube would be more of a product call. Previously we've dcome the conclusion that the YouTube App is a better experience (and available to the vase majority of our users).

But... One solution may be to, when there is no handler for vnd-youtube, reload the page in a way to get the HTML5 version (I think you can do something like http://www.youtube.com/embed/<vidoe ID>?html5=1)

That said, not all videos will be available in HTML5, you know DRM and all that.
> 
> Thoughts?
Flags: needinfo?(blassey.bugs)
Assignee

Comment 12

6 years ago
(In reply to Brad Lassey [:blassey] (use needinfo?) from comment #11)

> What makes you think this is going to be maintained?

It's going to be maintained at least as much as VideoPlayer is.


> But... One solution may be to, when there is no handler for vnd-youtube,
> reload the page in a way to get the HTML5 version (I think you can do
> something like http://www.youtube.com/embed/<vidoe ID>?html5=1)

Yeah, that's the situation in which I'm suggesting this. Essentially, if you're on a device without a YouTube player, we should try playing the video inside the browser, rather than launching a separate activity, given that the separate activity doesn't actually seem to work.


> That said, not all videos will be available in HTML5, you know DRM and all
> that.

But that's much the same situation we have with VideoPlayer, right? -- sometimes the info request comes back with "This video isn't available due to...", or just no stream. I had a one-in-three success rate when I was trying to find a video to test against.
I agree with the plan you two are forming:
1. Use vnd-youtube to spawn out to the YouTube player
2. If not available, reload to get the HTML5 version and play it in the browser
3. If no HTML5 version, we apologize

I think we still send the XUL Fennec UA to YouTube so we get vnd-youtube streams. Disabling that UA override should get the HTML5 version. It works for embedded YouTube videos.
Flags: needinfo?(mark.finkle)
(In reply to Mark Finkle (:mfinkle) from comment #13)
> I agree with the plan you two are forming:
> 1. Use vnd-youtube to spawn out to the YouTube player
> 2. If not available, reload to get the HTML5 version and play it in the
> browser
> 3. If no HTML5 version, we apologize
> 
> I think we still send the XUL Fennec UA to YouTube so we get vnd-youtube
> streams. Disabling that UA override should get the HTML5 version. It works
> for embedded YouTube videos.

going a step further, if there is no YouTube player available, set a pref to override the UA override in general so we only have to do this dance once
Assignee

Updated

5 years ago
Blocks: kindle
Assignee

Updated

5 years ago
Assignee: rnewman → nobody
Status: ASSIGNED → NEW
Duplicate of this bug: 906869
To avoid further confusion (see bug 906869 comment 23) would it make sense to adjust the description of this bug?
Assignee

Updated

5 years ago
Summary: Video player doesn't work at all on Kindle Fire HDX → VideoPlayer doesn't work at all, preventing video playback on devices without YouTube application
Assignee

Comment 17

5 years ago
I'm working on this.

The embedding video player is fullscreen, lacks comments, etc; we either need to find a different invocation (such as loading without a special UA), or continue using the action icon in the URL bar to launch the 'player'.

More as I test.
Assignee: nobody → rnewman
Status: NEW → ASSIGNED
Assignee

Comment 18

5 years ago
Tapping a YouTube link:

07-30 12:46:44.397 I/GeckoAppShell(22948): getHandlersForURL: vnd.youtube,
07-30 12:46:44.437 I/GeckoAppShell(22948): getHandlersForURL: vnd.youtube,
07-30 12:46:44.437 I/GeckoAppShell(22948): openUriExternal: vnd.youtube:SCLU3v_AY4Q?vndapp=youtube_mobile&vndclient=mv-google&vndel=watch&vnddnc=1
Assignee

Comment 19

5 years ago
This stubs out the real integration point; see Part 2.
Attachment #8465048 - Flags: review?(mark.finkle)
Assignee

Comment 20

5 years ago
This works for me. Loading a YouTube video page gives you the usual clickable video. Tapping that opens the HTML player rather than opening the YouTube app; you can watch the video then hit Back to return to the video page. Really nice.
Attachment #8465050 - Flags: review?(mark.finkle)
Assignee

Updated

5 years ago
Attachment #831239 - Attachment is obsolete: true
Attachment #8465048 - Flags: review?(mark.finkle) → review+
Comment on attachment 8465050 [details] [diff] [review]
Part 2: delegate to Fennec itself for YouTube HTML5 video instead of VideoPlayer. v1

>diff --git a/mobile/android/base/GeckoAppShell.java b/mobile/android/base/GeckoAppShell.java
>     static String[] getHandlersForURL(String aURL, String aAction) {
>+        Log.v(LOGTAG, "getHandlersForURL: " + aURL + ", " + aAction);

Don't leak URIs. Let's just remove it.

>     public static boolean openUriExternal(String targetURI,
>                                           String mimeType,
>               @OptionalGeneratedParameter String packageName,
>               @OptionalGeneratedParameter String className,
>               @OptionalGeneratedParameter String action,
>               @OptionalGeneratedParameter String title) {
>+        Log.v(LOGTAG, "openUriExternal: " + targetURI);
>+

Don't leak URIs. Let's just remove it.

>     public static Intent getShareIntent(final Context context,
>                                         final String targetURI,
>                                         final String mimeType,
>                                         final String title) {
>+        Log.v(LOGTAG, "getShareIntent: " + targetURI);
>+

Don't leak URIs. Let's just remove it.

>+            if (youtubeURI != null) {
>+                Log.v(LOGTAG, "Opening YouTube URI: " + youtubeURI);

Don't leak URIs. Let's just remove it.

>+    private static Uri getYouTubeHTML5URI(final Uri uri) {
>+        if (uri == null) {
>+            return null;
>+        }
>+
>+        Log.v(LOGTAG, "Incoming URI: " + uri);

Don't leak URIs. Let's just remove it.
Attachment #8465050 - Flags: review?(mark.finkle) → review+
Assignee

Comment 23

5 years ago
Relnote: "Fix YouTube playback on devices without YouTube app."

Assuming this verifies fine, I'd want this for 33. Any contrary opinions? mfinkle?
https://hg.mozilla.org/mozilla-central/rev/2c54d1bdd339
https://hg.mozilla.org/mozilla-central/rev/4cbef502c1dc
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 34
> # pm disable com.google.android.youtube

* Played video directly on YouTube on my Nexus 5 (4.4.4)
* Embedded YouTube works
Status: RESOLVED → VERIFIED
Assignee

Comment 26

5 years ago
Comment on attachment 8465050 [details] [diff] [review]
Part 2: delegate to Fennec itself for YouTube HTML5 video instead of VideoPlayer. v1

Approval Request Comment
[Feature/regressing bug #]:
  Forever.

[User impact if declined]:
  Crashes when attempting to play YouTube videos on AOSP/Kindle Fire.

[Describe test coverage new/current, TBPL]:
  Manually verified.

[Risks and why]: 
  We're loosely targeting 33 as our Kindle-supporting release, and this is actually quite serious for non-Google-branded users, too.

[String/UUID change made/needed]:
  None.
Attachment #8465050 - Flags: approval-mozilla-aurora?
(In reply to Richard Newman [:rnewman] from comment #23)
> Relnote: "Fix YouTube playback on devices without YouTube app."

It's a subpar experience in comparison to using the native YouTube application in entirety, not sure how you would describe that.
Assignee

Comment 28

5 years ago
(In reply to Aaron Train [:aaronmt] from comment #27)

> It's a subpar experience in comparison to using the native YouTube
> application in entirety, not sure how you would describe that.

I think it's implied that we can't send you to the YouTube app… we don't suggest "reimplemented the entire YouTube experience".

Beyond that, I actually feel that the experience is pretty good! It does fullscreen, and even does casting, I think.
(In reply to Richard Newman [:rnewman] from comment #28)
> (In reply to Aaron Train [:aaronmt] from comment #27)
> 
> > It's a subpar experience in comparison to using the native YouTube
> > application in entirety, not sure how you would describe that.
> 
> I think it's implied that we can't send you to the YouTube app… we don't
> suggest "reimplemented the entire YouTube experience".
> 
> Beyond that, I actually feel that the experience is pretty good! It does
> fullscreen, and even does casting, I think.

Agreed. It's pretty good.
Attachment #8465050 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Assignee

Comment 32

5 years ago
Comment on attachment 8465050 [details] [diff] [review]
Part 2: delegate to Fennec itself for YouTube HTML5 video instead of VideoPlayer. v1

Mark and I discussed getting this into Beta.

It's a safe change: we're happy with it in Nightly and on Aurora.

It addresses an obvious and painful crash for not only Kindle but also other AOSP users, which is a pretty big market (in some locales, bigger than Google-licensed Android; cf Xiaomi).

And the change only affects those users: even if this is buggy, it can't be worse than our current "just crash" behavior.
Attachment #8465050 - Flags: approval-mozilla-beta?
Verified as fixed in:
Build: Firefox for Android 33.0a2 (2014-08-11)
Device: Kindle Fire HD 7"
Comment on attachment 8465050 [details] [diff] [review]
Part 2: delegate to Fennec itself for YouTube HTML5 video instead of VideoPlayer. v1

Ouch that we need to have more site specific code in Fennec. I agree with the risk assessment and this it is worth trying to push this out sooner given the limited impact that it should have on the general Fennec population. Let's get this into beta6. beta+
Attachment #8465050 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Verified as fixed in:
Build: Firefox for Android 32 Beta 6
Device: Kindle Fire HD 7"
Assignee

Updated

4 years ago
Blocks: 844414
You need to log in before you can comment on or make changes to this bug.