928804 - Youtube Flash videos are displayed upside down

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Plugins, defect)

Description

[u421692]

Attachment: upside_down.png

Environment:
Device: LG Nexus 4 (Android 4.2.2) / Asus Transformer TF101 (Android 4.0.3)
Build: Firefox Nightly 27.0a1 (2013-10-20)

Steps to reproduce:
1. Open Youtube Desktop Site or a site that contains Youtube embedded video (e.g. http://www.intel.com/museumofme/mobile/index.htm)
2. Play video.

Expected result:
Video is played without any issues.

Actual result:
The video is upside down, though the Youtube player controls are oriented correctly.

Comment 1

[u421692]

Nightly 27.0a1 (2013-10-11)                                - BAD
Nightly 27.0a1 (2013-10-04) to Nightly 27.0a1 (2013-10-10) - CRASH
Nightly 27.0a1 (2013-10-03)                                - GOOD

I can only think that the patches on Bug 923917 created this regression.

Comment 2

[bhavana bajaj [:bajaj]]

Passing this onto :nical to help with investigation here as this is a fallout from 923917.

Comment 3

[Aaron Train [:aaronmt]]

Reproduced on my LG Nexus 4 (Android 4.3) running the latest available version of the Flash APK.

Comment 4

[Nicolas Silva [:nical]]

Interestingly, zooming in puts the video back in the correct orientation at same time the tiles (in the rest of the page) are repainted (so typically when some layout as been triggered). This looks like something outside of layers is not giving the correct information to its layer. I'll put some breakpoints and see.

Comment 5

[Kevin Brosnan [Ex-Mozilla]]

Worth an Aurora relnote.

Comment 6

[Nicolas Silva [:nical]]

I am having issues debugging on my 4.3 devices. I also have a 2.3 device (on which jimdb works well) but it doesn't reproduce there (it wouldn't be fun).
Still looking.

Comment 7

[Nicolas Silva [:nical]]

I have been probing every bit of TextureClient/Host code in search for a place where we switch from y-inverted and non-y-inverted textures, or any mismatch between what is passed to the client and what the host does, and in fact it appears that this stuff is working perfectly fine. The created texture clients and host are all Y-inverted, and the compositor is always drawing Y-inverted quads for the plugin.

It is the content GL texture that the plugin gives us that is, at first not inverted (but nsPluginInstanceOwner still asks the layer to invert it), and as soon as we cause a reflow, becomes inverted (which then corresponds to what the layer is asked to do). Weird.

Comment 8

[Nicolas Silva [:nical]]

snorp, your name appears in the mercurial history of the code around mInverted in nsPluginInstanceOwner.cpp, do you know what's the logic around y-inverting plugin frames on fennec? I am a bit lost right now.

Comment 9

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

(In reply to Nicolas Silva [:nical] from comment #8)
> snorp, your name appears in the mercurial history of the code around
> mInverted in nsPluginInstanceOwner.cpp, do you know what's the logic around
> y-inverting plugin frames on fennec? I am a bit lost right now.

It's only a problem for video, right? So most likely something is (unsurprisingly) going wrong here: http://mxr.mozilla.org/mozilla-central/source/dom/plugins/base/nsPluginInstanceOwner.cpp#1696

If all the layers machinery is working, I can just take this bug from you if you'd like.

Comment 10

[Nicolas Silva [:nical]]

Sweet, I am assigned to a lot of bugs so I hereby gladly accept your offer :)

Comment 11

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

Attachment: Always use an up-to-date texture transform when compositing SharedTextureSourceOGL

Comment 12

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

I would really like to kill the SharedTexture stuff and just add some sort of native support for SurfaceTexture/ANativeWindow in TextureHost. This stuff sucks.

Comment 13

[Nicolas Silva [:nical]]

(In reply to James Willcox (:snorp) ([e-mail]) from comment #12)
> I would really like to kill the SharedTexture stuff and just add some sort
> of native support for SurfaceTexture/ANativeWindow in TextureHost. This
> stuff sucks.

SharedTexture is also used on mac iirc, and SurfaceTexture is a higher level concept than TextureHost. We should add support for ANativeWindow at the compositable level directly and not use TextureHost in the cases where a plugin talks to us through ANativeWindow.

Comment 14

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/ad311f0dd591

Comment 15

[Ryan VanderMeulen [:RyanVM]]

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

Comment 16

[bhavana bajaj [:bajaj]]

(In reply to James Willcox (:snorp) ([e-mail]) from comment #14)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/ad311f0dd591

Ready for aurora uplift here or are we waiting on QA verification ?

Comment 17

[Teodora Vermesan (:TeoVermesan)]

Verified fixed on:
Build: Firefox for Android 28.0a1 (2013-11-24)
Device: LG Nexus 4
OS: Android 4.2.2

Comment 18

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

Comment on attachment 8333932 [details] [diff] [review]
Always use an up-to-date texture transform when compositing SharedTextureSourceOGL

[Approval Request Comment]
Bug caused by (feature/regressing bug #): new textures on android (bug ?)
User impact if declined: Upside down content/video with Flash
Testing completed (on m-c, etc.): a week or so on m-c, verified fixed
Risk to taking this patch (and alternatives if risky): Low
String or IDL/UUID changes made by this patch: None

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/090261583441

Comment 20

[Teodora Vermesan (:TeoVermesan)]

Verified fixed on:
Build: Firefox for Android 27.0a2 (2013-12-01)
Device: LG Nexus 4
OS: Android 4.2.2