GNOME Bugzilla – Bug 503231
Change to GST_BUFFER_FLAG_GAP meaning can break with basetransform elements
Last modified: 2007-12-14 16:53:25 UTC
Hi, the change to GST_BUFFER_FLAG_GAP to mean, that this buffer contains neutral data can break basetransform elements as they simply forward this flag. Some elements can now do optimizations on such buffers, i.e. the volume element can simply forward such buffer without processing, etc. Now consider the case where an element does not know that buffers with the GAP flag should contain neutral data and they create some non-neutral data in the buffer. Now a downstream element that does special processing on buffers with the GAP flag will get something that they don't expect and most likely will output something wrong, i.e. the volume element doesn't change anything and keeps the volume as is while it should instead mute the buffer. The attached patch against basetransform is an attemp to prevent such things from happening. Elements that are aware of the change can set the GstBaseTransformClass->gap_aware value to TRUE and will continue to get buffers with the GAP flag set while all other elements will only get output buffers without GAP flag. Comments, suggestions, etc? :) Of course all elements that use the new meaning already need to get a little one-line patch.
Created attachment 100817 [details] [review] gapware.diff
Created attachment 100818 [details] [review] gapware.diff
Created attachment 100819 [details] [review] gapware.diff
Looks good! 2 minor things. 1.) The docs for gst_base_transform_set_gap_aware "If @gap_aware is %TRUE subclasses will get output buffers with the" should say: "If @gap_aware is %TRUE subclasses might get output buffers with the" or even better: "If @gap_aware is %FALSE (as it is by default) subclasses will never get output buffers with the" 2.) Whatfor do we need gst_base_transform_is_gap_aware? Elements only need to tell basetransform. Should anyone else need to know wheter they can optimize or not?
Created attachment 100875 [details] [review] gapware.diff New patch with Stefan's suggestions
2007-12-14 Sebastian Dröge <slomo@circular-chaos.org> * docs/libs/gstreamer-libs-sections.txt: * libs/gst/base/gstbasetransform.c: (gst_base_transform_init), (gst_base_transform_prepare_output_buffer), (gst_base_transform_set_gap_aware): * libs/gst/base/gstbasetransform.h: API: Add gst_base_transform_set_gap_aware() to control whether the element correctly handles GST_BUFFER_FLAG_GAP or shouldn't get buffers with this flag at all. Fixes #503231.