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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Aug 5 18:34:46 PDT 2010


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





--- Comment #4 from Victoria <vrk at google.com>  2010-08-05 18:34:45 PST ---
This is a patch that contains the proposed plumbing between WebKit and Chromium to give VideoLayerChromium direct access to the video frames in the Chromium side. As a proof of concept, the Y layer is output to a texture, so running this patch (and the corresponding Chromium patch) with --enable-accelerated-compositing should output a black-and-white video to the screen.

Some parts are a bit rough around the edges still, but I wanted to put the patch up for code review/discussion before completely polishing things.

The patch requires a bit of explanation. To remind everyone what we're doing: In the VideoLayerChromium, we wanted to have direct access to the uncompressed YUV frame data such that we could copy this data directly to textures and use the GPU to do color conversion. The uncompressed YUV frame data lies in the VideoRendererImpl, which is in chromium.

I drew a diagram here to help with the explanation:
* https://docs.google.com/a/google.com/drawings/edit?id=15THug8YVpvTgp0KPkW4mIJG0ZTG-qRL862Ovlf-ciW0&hl=en
(Let me know if you have trouble accessing it.)

VideoLayerChromium needed a way to ask for frames. I wrote an interface called WebCore::VideoFrameProvider that consists of getCurrentFrame and putCurrentFrame methods, analogous to media::VideoFrame's get/put methods.

WebMediaPlayerClientImpl implements the WebCore::VideoFrameProvider interface, as it has access to VideoRendererImpl indirectly through m_webMediaPlayer, and it creates and stores a reference to VideoLayerChromium. When a VideoLayerChromium is created, it is given a reference to its owning WebMediaPlayerClientImpl and stores it as a VideoFrameProvider* m_provider. It then uses m_provider to get/put frames when needed.

One of the trickiest parts of implementing this plumbing was due to the data type for the VideoFrame and the strict divisions in code between WebKit/WebKit/chromium/public, WebKit/WebCore/, and webkit/glue. VideoLayerChromium needs to be able to access the video frame, so there had to be a WebCore::VideoFrameChromium type. However, WebMediaPlayerClientImpl can only communicate to VideoRendererImpl through m_webMediaPlayer, which implements the WebKit::WebMediaPlayer interface. This WebKit interface cannot use WebCore types, and we also try to avoid WebCore types in chromium. Thus to make the communication possible, I decided to create both a WebCore::VideoFrameChromium interface and a WebKit::WebVideoFrame interface. These interfaces are virtually identical other than their namespaces.

Thus to make all the types happy: I wrote a WebKit::WebVideoFrame interface that is implemented by webkit_glue::WebVideoFrameImpl and is simply a wrapper for media::VideoFrame (takes a VideoFrame* in the constructor). I then wrote a WebCore::VideoFrameChromium interface implemented by WebKit::VideoFrameChromiumImpl that is a wrapper for WebKit::WebVideoFrame (takes a WebVideoFrame* in the constructor).

What this means is there are 3 parts of the code between WebKit, WebCore, and chromium that are unfortunately duplicated: WebKit::WebVideoFrame, WebCore::VideoFrameChromium, and media::VideoFrame. This actually wouldn't be *that* bad since the method headers copied are not likely going to change, *but* I also have to duplicate the enums and constants. That is really going to suck to try to keep the enums synchronized in the 3 places. Any tips on how to clean this part up would be greatly appreciated!

You can refer to the diagram to see how these pieces all work together. The entire patch is available for review here:
http://codereview.chromium.org/3058055

Since I'm not going to commit this patch as-is anyway, I am not really looking for line-by-line feedback as I am looking for feedback on the overall design (line-by-line nits are welcome if you're up to it, though!). Please let me know if you have any questions or suggestions. Thanks!

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



More information about the webkit-unassigned mailing list