Bug 88826 - [chromium] Create a CCStreamVideoDrawQuad used for StreamTexture video output
Summary: [chromium] Create a CCStreamVideoDrawQuad used for StreamTexture video output
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dana Jansens
URL:
Keywords:
Depends on: 88814
Blocks: 88828
  Show dependency treegraph
 
Reported: 2012-06-11 17:52 PDT by Dana Jansens
Modified: 2012-06-13 17:38 PDT (History)
11 users (show)

See Also:


Attachments
Patch (18.74 KB, patch)
2012-06-11 18:00 PDT, Dana Jansens
no flags Details | Formatted Diff | Diff
Patch (18.65 KB, patch)
2012-06-13 08:22 PDT, Dana Jansens
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dana Jansens 2012-06-11 17:52:57 PDT
[chromium] Create a CCStreamVideoDrawQuad used for StreamTexture video output
Comment 1 Dana Jansens 2012-06-11 18:00:34 PDT
Created attachment 146982 [details]
Patch
Comment 2 James Robinson 2012-06-11 18:03:50 PDT
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)?
Comment 3 Dana Jansens 2012-06-11 18:05:26 PDT
(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?
Comment 4 James Robinson 2012-06-11 18:06:58 PDT
(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.
Comment 5 Adrienne Walker 2012-06-12 12:24:28 PDT
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.
Comment 6 Min Qin 2012-06-12 17:03:17 PDT
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
Comment 7 James Robinson 2012-06-12 17:34:48 PDT
OK, well when you do catch up please let us know if this explodes.  I think we'll have to just move on optimistically.
Comment 8 Min Qin 2012-06-12 18:42:49 PDT
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.
Comment 9 Dana Jansens 2012-06-13 08:18:42 PDT
Thanks Min
Comment 10 Dana Jansens 2012-06-13 08:22:02 PDT
Created attachment 147320 [details]
Patch

rebase
Comment 11 WebKit Review Bot 2012-06-13 17:38:16 PDT
Comment on attachment 147320 [details]
Patch

Clearing flags on attachment: 147320

Committed r120263: <http://trac.webkit.org/changeset/120263>
Comment 12 WebKit Review Bot 2012-06-13 17:38:21 PDT
All reviewed patches have been landed.  Closing bug.