Bug 132247 – [GStreamer] Use GstMetaVideo

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Víctor M. Jáquez L.]

In WebKitVideoSink we announce the usage of GstMetaVideo, but we do not use it when handling the video frames. This might break some decoders and filters that rely on buffer's meta, rather that static caps structures.

This patch will enable the use of GstMetaVideo if it is available in the buffer.

[Víctor M. Jáquez L.]

Created attachment 230288 [details]
Patch

[Nicolas Dufresne]

Well, this is not quite right. I would strongly recommend using the GstVideoFrame API, gst_video_frame_map()/gst_video_frame_unmap(). This way you don't have to worry how to map a buffer base on whether or not it has GstVideoMeta.

[Philippe Normand]

Oh yes that would be much better indeed. Care to update the patch Victor?

[Víctor M. Jáquez L.]

(In reply to comment #3)
> Oh yes that would be much better indeed. Care to update the patch Victor?

Sure.

[Víctor M. Jáquez L.]

Created attachment 230476 [details]
Patch

[Philippe Normand]

Comment on attachment 230476 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=230476&action=review

> Source/WebCore/ChangeLog:19
> +        Also, since either EFL and GTK+ ports use version of glib greater than
> +        2.31, the guards for it were removed.

This is quite unrelated, I'd rather do this in a separate patch, also cleaning the similar ones in MediaPlayerPrivateGStreamerBase.cpp

[Nicolas Dufresne]

> +    GstVideoInfo videoInfo;
> +    if (!gst_video_info_from_caps(&videoInfo, caps))
> +        return;

You should ideally call gst_video_info_init(&videoInfo), since info_from_caps() will only set what it finds in the caps, leaving random value in C, and non-default in C++. Will help if you run some memory checker. (found 2 times)

> +    int stride = GST_VIDEO_FRAME_PLANE_STRIDE(&m_videoFrame, 0);
Not sure if you guys uses these, but an assert that there is only 1 plane would be a nice to have. So if a dev wants to add planar formats support, it won't endup doing weird things silently.

These are details, looks good, nice work !

[Víctor M. Jáquez L.]

Created attachment 230492 [details]
Patch

[Víctor M. Jáquez L.]

Created attachment 230493 [details]
Patch

[Philippe Normand]

Comment on attachment 230493 [details]
Patch

Ok! Thanks to both Nicolas and Victor :)

[WebKit Commit Bot]

Comment on attachment 230493 [details]
Patch

Clearing flags on attachment: 230493

Committed r168060: <http://trac.webkit.org/changeset/168060>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Víctor M. Jáquez L.]

Created attachment 230823 [details]
WebKitGTK+-2.2.x backport