[chromium] Add textureWidth() and textureHeight() to WebVideoFrame
Created attachment 170005 [details] Patch
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.
Just an interface change at the moment. I'd like to remove stride() eventually, but only after I land the Chromium-side changes as wekk,
Comment on attachment 170005 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=170005&action=review > Source/Platform/ChangeLog:8 > + Additional information of the change such as approach, rationale. Please add per-function descriptions below (OOPS!). You have to fill something out here. For this, when would textureWidth/textureHight be different from the frame's width/height? Why do we need separate fields?
Created attachment 170009 [details] Patch
Please take a look here.
I can't find any equivalent in media/base/video_frame.h. Perhaps there's some context here I'm missing? Is there a crbug or chromium-side code review that goes along with this?
The Chromium bug lives at http://crbug.com/155416, and the Chromium-side change is WIP on my checkout. I can't actually checkin for Chromium until this interface change lands, so this one's going first. (As the interface change just adds two accessors, it's safe to do first.)
(In reply to comment #8) > The Chromium bug lives at http://crbug.com/155416, and the Chromium-side change is WIP on my checkout. I can't actually checkin for Chromium until this interface change lands, so this one's going first. (As the interface change just adds two accessors, it's safe to do first.) @sheu: actually, this is http://crbug.com/140509 :) You taking that over? (that'd be great, since I haven't had time to touch it) (155416 describes a symptom; 140509 describes the root cause and a plan for fixing it)
Comment on attachment 170009 [details] Patch Thanks for the info. Slavomir (cc'd) is working on moving the dependency from media -> cc down to not need the WebVideoFrame intermediate data structure, so hopefully this won't be needed in WebKit for long. But for now it definitely is.
Created attachment 170491 [details] Patch
Updated patch. I have a question on naming in the interface. To wit: is visibleX/visibleY/visibleWidth/visibleHeight the right name to use for the added interface functions, in WebKit style?
(In reply to comment #12) > Updated patch. > > I have a question on naming in the interface. To wit: is visibleX/visibleY/visibleWidth/visibleHeight the right name to use for the added interface functions, in WebKit style? The naming is correct - accessors should match the variable name they are accessing - but it'd be better to have the whole thing be one accessor that returns type WebRect instead of 4 individual methods.
Created attachment 170523 [details] Patch
Comment on attachment 170523 [details] Patch Attachment 170523 [details] did not pass cr-android-ews (chromium-android): Output: http://queues.webkit.org/results/14554451
Created attachment 170530 [details] Patch
Comment on attachment 170530 [details] Patch Great, R=me
Comment on attachment 170530 [details] Patch Clearing flags on attachment: 170530 Committed r132448: <http://trac.webkit.org/changeset/132448>
All reviewed patches have been landed. Closing bug.