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> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | agomez, calvaris, cgarcia, commit-queue, eric.carlson, glenn, gustavo, jer.noble, menard, mrobinson, nicolas, philipj, pnormand, sergio | ||||||||||||
Priority: | P2 | ||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||
Hardware: | Unspecified | ||||||||||||||
OS: | Unspecified | ||||||||||||||
Attachments: |
|
Description
Víctor M. Jáquez L.
2014-04-28 03:19:14 PDT
Created attachment 230288 [details]
Patch
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. Oh yes that would be much better indeed. Care to update the patch Victor? (In reply to comment #3) > Oh yes that would be much better indeed. Care to update the patch Victor? Sure. Created attachment 230476 [details]
Patch
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 > + 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 ! Created attachment 230492 [details]
Patch
Created attachment 230493 [details]
Patch
Comment on attachment 230493 [details]
Patch
Ok! Thanks to both Nicolas and Victor :)
Comment on attachment 230493 [details] Patch Clearing flags on attachment: 230493 Committed r168060: <http://trac.webkit.org/changeset/168060> All reviewed patches have been landed. Closing bug. Created attachment 230823 [details]
WebKitGTK+-2.2.x backport
|