GNOME Bugzilla – Bug 703271
[gstreamer-1.2-port] Add support for GstCaps features
Last modified: 2013-12-19 12:11:53 UTC
This bug is to track the patches required to support caps features (available in GStreamer 1.2) http://cgit.freedesktop.org/gstreamer/gstreamer/tree/docs/design/part-caps.txt
Created attachment 248032 [details] [review] Add capsfeature support. https://bugzilla.gnome.org/show_bug.cgi?id=698054
Review of attachment 248032 [details] [review]: ::: gst/vaapi/gstvaapidecode.c @@ +190,3 @@ gst_video_codec_state_unref(state); +#if !GST_CHECK_VERSION(1,1,0) I'd prefer the newest APIs first, i.e. #if GST_CHECK_VERSION(1,1,0) state->caps = gst_video_info_to_caps(&vis); #else ... #endif Doesn't really matter but it would look more consistent and easier to strip down to include only the git master bits. ::: gst/vaapi/gstvaapisink.c @@ +743,3 @@ + templ_caps = gst_static_pad_template_get_caps (&gst_vaapisink_sink_factory); + out_caps = gst_caps_copy (templ_caps); + gst_caps_unref (templ_caps); Can't we just make out_caps = gst_static_pad_template_get_caps() since that already increases the reference of the underlying GstCaps? Otherwise, if we want to write into out_caps, it's probably clearer to call gst_caps_make_writable() explicitly, even though this reduces to the same thing.
Created attachment 248364 [details] [review] Add capsfeature support Minor modifications as gb proposed.
Review of attachment 248364 [details] [review]: ::: gst/vaapi/gstvaapisink.c @@ +94,3 @@ + GST_VIDEO_CAPS_MAKE(GST_VIDEO_FORMATS_ALL); + +#elif !GST_CHECK_VERSION(1,0,0) sree_, the patch doesn't apply correctly anymore, because of 769f33ca Could you update the patch?
hm, i didn't see your comment.The stupid bugzilla didn't send any email notification! It seems that the gstreamer-vaapi master branch is changing rapidly with many patches.So IMHO it is better to wait for Gwenole to merge the 1.2 stuffs . Otherwise we will end up with rebasing all the patches again and again.
> IMHO it is better to wait for Gwenole to merge the 1.2 stuffs . > Otherwise we will end up with rebasing all the patches again and again. Agree
Review of attachment 248364 [details] [review]: ::: gst/vaapi/gstvaapisink.c @@ +752,3 @@ + if (yuv_caps) { + if (!gst_caps_is_writable(out_caps)) + out_caps = gst_caps_make_writable(out_caps); out_caps = gst_caps_make_writable(out_caps) should be enough. More specifically, gst_caps_is_writable() does not exist in GStreamer 0.10 APIs. :)
Patch slightly arranged and integrated into git master branch. Thanks.
commit 8fe3bb0b14120738834000d3de3dfa842e692e24 Author: Sreerenj Balachandran <sreerenj.balachandran@intel.com> Date: Thu Jul 4 11:03:52 2013 +0300 plugins: add support for GstCaps features. Move VA video buffer memory from "video/x-surface,type=vaapi" format, as expressed in caps, to the more standard use of caps features. i.e. add "memory:VASurface" feature attribute to the associated caps. https://bugzilla.gnome.org/show_bug.cgi?id=703271 Signed-off-by: Gwenole Beauchesne <gwenole.beauchesne@intel.com>
It seems that you forgot to expose the raw video formats in sink caps template which was there in my patch. Was that intentional? Anyway i will attach another patch to add that.
Created attachment 256794 [details] [review] vaapisink: expose the raw video formats in static caps template. Expose all raw video formats in the static caps template since the vaapisink is supporting raw data. We will get the exact set of formats supported by the driver dynamically through the _get_caps() routine.
Re-opening..
Hi, in this case, gst_vaapisink_get_caps_impl() needs some arrangements to avoid duplicates from the template pad caps. e.g. remove formats from what we get from the template pad caps, or don't actually use gst_static_pad_template_get_caps() but rather build clean caps that only mention the first series for caps with memory:VASurface feature.
The initial bug was fixed. The vaapisink template caps update is followed up on bug #720737