GNOME Bugzilla – Bug 657319
videorate should use basetransform
Last modified: 2011-10-29 15:16:11 UTC
Created attachment 194693 [details] [review] proposed patch Currently videorate doesn';t support interesting features like forwarding pad-allocs and upstream negotiation, easiest way to fix this is to port to basetransform...
Created attachment 194706 [details] [review] Additional patch to fix some other issues
Created attachment 194839 [details] [review] Updated patch for new basetransform api
Does basetransform work correctly when input and output buffers are not 1-1? I'm pretty sure the answer is (was?) no.
Review of attachment 194839 [details] [review]: Looks good in general (and you can use basetransform for non-1-to-1 elements, see audiofirfilter for example, you just have to handle additional buffers yourself and drop buffers with GST_BASE_TRANSFORM_FLOW_DROPPED) ::: gst/videorate/gstvideorate.c @@ +285,3 @@ { + GstStructure *s; + gint nom, denom; It's numerator, not nomerator/nominator ;) @@ +302,3 @@ GstStructure *structure; gboolean ret = TRUE; + //GstPad *otherpad, *opeer; C++/C99 comment @@ -407,3 @@ - if (gst_structure_has_field (structure, "pixel-aspect-ratio")) - gst_structure_fixate_field_nearest_fraction (structure, - "pixel-aspect-ratio", 1, 1); These fixations might still be necessary in the fixate vfunc
Review of attachment 194706 [details] [review]: Looks good
(In reply to comment #4) > Review of attachment 194839 [details] [review]: > > Looks good in general (and you can use basetransform for non-1-to-1 elements, > see audiofirfilter for example, you just have to handle additional buffers > yourself and drop buffers with GST_BASE_TRANSFORM_FLOW_DROPPED) > > ::: gst/videorate/gstvideorate.c > @@ +285,3 @@ > { > + GstStructure *s; > + gint nom, denom; > > It's numerator, not nomerator/nominator ;) > > @@ +302,3 @@ > GstStructure *structure; > gboolean ret = TRUE; > + //GstPad *otherpad, *opeer; > > C++/C99 comment Fixed up/merged this two (will attach the updated patch) > @@ -407,3 @@ > - if (gst_structure_has_field (structure, "pixel-aspect-ratio")) > - gst_structure_fixate_field_nearest_fraction (structure, > - "pixel-aspect-ratio", 1, 1); > > These fixations might still be necessary in the fixate vfunc As discussed on IRC, basetransform falls back to gst_pad_fixate_caps if after calling the fixate vfunc some fields aren't fixated yet, so this shouldn't be needed anymore
Created attachment 195291 [details] [review] updated patch
his problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report. commit ea46b3c706ea99257d2b31cb5c7fb281f43b3178 Author: Sjoerd Simons <sjoerd.simons@collabora.co.uk> Date: Tue Aug 23 10:11:52 2011 +0200 videorate: Port to basetransform