[Webkit-unassigned] [Bug 43590] Implementing video frame sharing between WebKit and Chromium

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=43590





--- Comment #11 from Victoria <vrk at google.com>  2010-08-17 11:11:01 PST ---
(In reply to comment #10)
> (From update of attachment 63661 [details])
> WebKit/chromium/src/VideoFrameChromiumImpl.cpp:13
>  +  WebVideoFrame* VideoFrameChromiumImpl::toWebVideoFrame(WebCore::VideoFrameChromium* frame)
> the choice of variable names "frame" and "m_webFrame" is probably
> not so great since those are used when we have WebCore::Frame
> and WebKit::WebFrame pointers.
> 
Good call, thanks!

> WebCore/platform/graphics/chromium/VideoFrameChromium.h:74
>  +      virtual SurfaceType type() const = 0;
> i don't see any usage of type and format methods.  is that an oversight?
> or just something that will happen later?
> 
In the (near) future, I think it'd be nice to have access to these Format/SurfaceType methods. Right now the code just assumes that the format is YV12 and the surface type is system memory, but these are not good assumptions to be making.

> WebKit/chromium/public/WebVideoFrame.h:75
>  +      virtual SurfaceType type() const = 0;
> instead of exposing all of this information, have you considered
> just having a method on this interface that you can call to load
> the frame into a GLES2 texture?  how much information about the
> frame do you really need to expose to WebKit?
> 
This is a good question. I did think about having a method like that, but the shader requires knowledge of the video frame such as dimensions and stride. So to completely push VideoFrame out of WebKit, the shader logic would have to be pushed out of WebKit as well, which doesn't seem very consistent with the rest of the design. But I will experiment; it is probably true that I don't need all this frame information exposed to WebKit and I should be able to strip it down to the bare minimum. The design is also going to morph a bit as I try to eliminate our extra memcopies.


> WebCore/platform/graphics/chromium/VideoFrameChromium.h:36
>  +  class VideoFrameChromium {
> we typically avoid this double thunking by making the WebCore "interface"
> use non virtual methods.  then, we just provide the concrete implementation
> in WebKit/chromium/src/, and that implementation then calls out to the
> WebKit API equivalent which provides the virtual functions.
Perhaps I am misunderstanding (I am still a bit of a C++ novice), but I am not sure if I can do this because VideoFrameChromiumImpl is just a wrapper for WebKit::WebVideoFrame and the constructor needs to take a WebVideoFrame as a parameter. If I got rid of VideoFrameChromiumImpl.h, the VideoFrameChromium.h would need to have that parameter instead, which I believe is no good since it is mixing WebKit types in WebCore. 

Thanks so much for taking a look! I will update with a new patch pretty soon as I play with reducing memcopies.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.