|Summary:||[GStreamer] Use GstMetaVideo|
|Product:||WebKit||Reporter:||Víctor M. Jáquez L. <vjaquez>|
|Component:||WebKit Misc.||Assignee:||Víctor M. Jáquez L. <vjaquez>|
|Severity:||Normal||CC:||agomez, calvaris, cgarcia, commit-queue, eric.carlson, glenn, gns, jer.noble, menard, mrobinson, nicolas, philipj, pnormand, sergio|
|Version:||528+ (Nightly build)|
Description Víctor M. Jáquez L. 2014-04-28 03:19:14 PDT
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.
Comment 2 Nicolas Dufresne 2014-04-28 06:52:25 PDT
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.
Comment 3 Philippe Normand 2014-04-28 06:57:25 PDT
Oh yes that would be much better indeed. Care to update the patch Victor?
Comment 4 Víctor M. Jáquez L. 2014-04-28 07:04:35 PDT
(In reply to comment #3) > Oh yes that would be much better indeed. Care to update the patch Victor? Sure.
Comment 6 Philippe Normand 2014-04-30 08:11:13 PDT
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
Comment 7 Nicolas Dufresne 2014-04-30 08:22:48 PDT
> + 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 !
Comment 10 Philippe Normand 2014-04-30 11:41:41 PDT
Comment on attachment 230493 [details] Patch Ok! Thanks to both Nicolas and Victor :)
Comment 11 WebKit Commit Bot 2014-04-30 16:18:08 PDT
Comment on attachment 230493 [details] Patch Clearing flags on attachment: 230493 Committed r168060: <http://trac.webkit.org/changeset/168060>
Comment 12 WebKit Commit Bot 2014-04-30 16:18:13 PDT
All reviewed patches have been landed. Closing bug.