[Webkit-unassigned] [Bug 44539] API changes for Video Frame sharing between WebKit and Chromium

[bugzilla-daemon at webkit.org]

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





--- Comment #11 from Victoria <vrk at google.com>  2010-08-25 14:58:51 PST ---
Hi Darin! I fixed my code to match your suggestions. Can you take another look?

> WebCore/platform/graphics/chromium/VideoFrameProvider.h:41
>  +      virtual VideoFrameChromium* getCurrentFrame() = 0;
> please document the ownership model.  who owns the VideoFrameChromium?
> how do I free it?  it looks like i have to call putCurrentFrame to
> free it, but is that the only way?  it might help if these methods
> were more fully documented.
I moved the comment from WebMediaPlayerClientImpl.h to this file. Let me know if you think I should add further clarification.

> WebKit/chromium/public/WebMediaPlayer.h:133
>  +      // FIXME: Will be made pure virtual when API changes are fully landed.
> WebKit API interfaces implemented by the embedder are intended to have
> non-pure virtual methods.  so this FIXME is backwards.  the other methods
> should have empty implementations :)
For now I just deleted the FIXME.

> WebKit/chromium/public/WebVideoFrame.h:71
>  +      virtual int stride(unsigned plane) const = 0;
> why does stride return signed instead of unsigned?
Strides can be negative if a frame is stored bottom-up in memory, and I believe this is true of RGB frames. (Not positive, though.)

> WebKit/chromium/src/VideoFrameChromiumImpl.cpp:56
>  +      m_webVideoFrame(webVideoFrame)
> what is the ownership model for m_webVideoFrame?  it looks like VideoFrameChromiumImpl
> is not responsible for deleting m_webVideoFrame.  should it?
You're correct, VideoFrameChromiumImpl is not responsible for deleting m_webVideoFrame. I added a comment saying such.

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