Bug 88826 – [chromium] Create a CCStreamVideoDrawQuad used for StreamTexture video output

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Dana Jansens]

[chromium] Create a CCStreamVideoDrawQuad used for StreamTexture video output

[Dana Jansens]

Created attachment 146982 [details]
Patch

[James Robinson]

Comment on attachment 146982 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=146982&action=review

> Source/WebCore/ChangeLog:16
> +        No new tests, no change in behaviour.

I'm a teensy bit nervous about this - have you had a chance to smoke test this on a device that uses the stream video texture path and made sure everything works as expected (we have no automated tests covering this path)?

[Dana Jansens]

(In reply to comment #2)
> (From update of attachment 146982 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=146982&action=review
> 
> > Source/WebCore/ChangeLog:16
> > +        No new tests, no change in behaviour.
> 
> I'm a teensy bit nervous about this - have you had a chance to smoke test this on a device that uses the stream video texture path and made sure everything works as expected (we have no automated tests covering this path)?

Right, there is no upstream code that calls this path right now, is there? That makes it difficult (or impossible) for me to test this right now.. Is it worth going to the effort to make that possible?

[James Robinson]

(In reply to comment #3)
> (In reply to comment #2)
> > (From update of attachment 146982 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=146982&action=review
> > 
> > > Source/WebCore/ChangeLog:16
> > > +        No new tests, no change in behaviour.
> > 
> > I'm a teensy bit nervous about this - have you had a chance to smoke test this on a device that uses the stream video texture path and made sure everything works as expected (we have no automated tests covering this path)?
> 
> Right, there is no upstream code that calls this path right now, is there? That makes it difficult (or impossible) for me to test this right now.. Is it worth going to the effort to make that possible?

I was hoping you could use non-upstream code or ask someone else to do so.  They aren't upstream so in one sense they get what they deserve, but I think it's worth making an attempt at keeping this path working if possible.

[Adrienne Walker]

Comment on attachment 146982 [details]
Patch

R=me.  These changes all look good.  It's sad that Android gives us a full 4x4 matrix when they're probably not using anything but scale and translation and we have to treat it opaquely.

Before landing, I'd like qinmin to chime in here.  If they have time to test this downstream, that'd be great.  Or, if they think it looks reasonable as-is and are willing to just fix any issues when merging, that's fine too.

[Min Qin]

There are quite some dependencies, and the webkit tree for chrome on android is several weeks behind. Not sure whether we are able to test this easily

[James Robinson]

OK, well when you do catch up please let us know if this explodes.  I think we'll have to just move on optimistically.

[Min Qin]

Though I have no easy way to test this, I checked the current diff between our downstream code and this patch and I think the patch looks good. 
If there are any problems when we merge the patch, it should not be difficult to fix.

[Dana Jansens]

Thanks Min

[Dana Jansens]

Created attachment 147320 [details]
Patch

rebase

[WebKit Review Bot]

Comment on attachment 147320 [details]
Patch

Clearing flags on attachment: 147320

Committed r120263: <http://trac.webkit.org/changeset/120263>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.