Bug 88826

Summary: [chromium] Create a CCStreamVideoDrawQuad used for StreamTexture video output
Product: WebKit Reporter: Dana Jansens <danakj>
Component: New BugsAssignee: Dana Jansens <danakj>
Status: RESOLVED FIXED    
Severity: Normal CC: cc-bugs, enne, eric.carlson, feature-media-reviews, fischman, jamesr, piman, qinmin, sievers, webkit.review.bot, wjmaclean
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 88814    
Bug Blocks: 88828    
Attachments:
Description Flags
Patch
none
Patch none

Dana Jansens
Reported 2012-06-11 17:52:57 PDT
[chromium] Create a CCStreamVideoDrawQuad used for StreamTexture video output
Attachments
Patch (18.74 KB, patch)
2012-06-11 18:00 PDT, Dana Jansens
no flags
Patch (18.65 KB, patch)
2012-06-13 08:22 PDT, Dana Jansens
no flags
Dana Jansens
Comment 1 2012-06-11 18:00:34 PDT
James Robinson
Comment 2 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)?
Dana Jansens
Comment 3 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?
James Robinson
Comment 4 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.
Adrienne Walker
Comment 5 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.
Min Qin
Comment 6 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
James Robinson
Comment 7 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.
Min Qin
Comment 8 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.
Dana Jansens
Comment 9 2012-06-13 08:18:42 PDT
Thanks Min
Dana Jansens
Comment 10 2012-06-13 08:22:02 PDT
Created attachment 147320 [details] Patch rebase
WebKit Review Bot
Comment 11 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>
WebKit Review Bot
Comment 12 2012-06-13 17:38:21 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.