After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 350449 - [GstBaseTransform] buffer_alloc should alway try downstream
[GstBaseTransform] buffer_alloc should alway try downstream
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other Linux
: Normal blocker
: 0.10.10
Assigned To: Edward Hervey
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2006-08-08 15:47 UTC by Edward Hervey
Modified: 2006-08-21 09:21 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch to implement proper buffer_alloc behaviour. (1.29 KB, patch)
2006-08-08 15:48 UTC, Edward Hervey
committed Details | Review
Fine-tuned the error cases. (1.91 KB, patch)
2006-08-17 08:57 UTC, Edward Hervey
none Details | Review

Description Edward Hervey 2006-08-08 15:47:54 UTC
In gst_base_transform_buffer_alloc, if the element is not initialized or can't figure out the size to request downstream, it returns a NULL buffer and TRUE (which will cause core to allocate the buffer normally, without requesting downstream).

This is wrong for:
1/ Pad blocking. If the stream is blocked downstream, the buffer allocation process needs to block too. Since we don't request a buffer downstream, it never gets to the blocked pad.
2/ Performance. It is always better to request a buffer downstream to have the furthest downstream buffer.

For this to work, GstBaseTransform should call gst_pad_buffer_alloc() on it's srcpad. This will just get a buffer without setting the caps (and therefore modifying the state of the element (initialized or not)).
Comment 1 Edward Hervey 2006-08-08 15:48:33 UTC
Created attachment 70494 [details] [review]
Patch to implement proper buffer_alloc behaviour.
Comment 2 Edward Hervey 2006-08-09 11:04:59 UTC
2006-08-09  Edward Hervey  <edward@fluendo.com>

        * libs/gst/base/gstbasetransform.c:
        (gst_base_transform_buffer_alloc):
        Even if we can't figure out the proper format to request downstream,
        call buffer_alloc() downstream with the input parameters without setting
        the caps on the srcpad. This will force negotiation in the chain
        function.
        Closes #350449

Comment 3 Tim-Philipp Müller 2006-08-16 17:45:58 UTC
This seems to break the following pipeline for me:

$ gst-launch-0.10 videotestsrc ! video/x-raw-yuv,format=\(fourcc\)I420,width=576,height=432,framerate=\(fraction\)25/1 ! ffmpegcolorspace ! ximagesink -v
Setting pipeline to PAUSED ...
/pipeline0/videotestsrc0.src: caps = video/x-raw-yuv, format=(fourcc)I420, width=(int)576, height=(int)432, framerate=(fraction)25/1, pixel-aspect-ratio=(fraction)1/1
Pipeline is PREROLLING ...
/pipeline0/capsfilter0.src: caps = video/x-raw-yuv, format=(fourcc)I420, width=(int)576, height=(int)432, framerate=(fraction)25/1, pixel-aspect-ratio=(fraction)1/1
/pipeline0/capsfilter0.sink: caps = video/x-raw-yuv, format=(fourcc)I420, width=(int)576, height=(int)432, framerate=(fraction)25/1, pixel-aspect-ratio=(fraction)1/1
/pipeline0/ffmpegcsp0.src: caps = video/x-raw-rgb, bpp=(int)16, depth=(int)16, endianness=(int)1234, red_mask=(int)63488, green_mask=(int)2016, blue_mask=(int)31, width=(int)576, height=(int)432, framerate=(fraction)25/1, pixel-aspect-ratio=(fraction)1/1
/pipeline0/ffmpegcsp0.sink: caps = video/x-raw-yuv, format=(fourcc)I420, width=(int)576, height=(int)432, framerate=(fraction)25/1, pixel-aspect-ratio=(fraction)1/1

** (gst-launch-0.10:26761): WARNING **: Size 497664 is not a multiple of unit size 373248
ERROR: from element /pipeline0/ffmpegcsp0: subclass did not specify output size
Additional debug info:
gstbasetransform.c(1448): gst_base_transform_handle_buffer (): /pipeline0/ffmpegcsp0:
subclass did not specify output size
ERROR: pipeline doesn't want to preroll.

Reverting this patch makes that work again.
Comment 4 Edward Hervey 2006-08-17 08:57:07 UTC
Created attachment 71075 [details] [review]
Fine-tuned the error cases.

This patches makes it work, while keeping my other tests working.

What happens it will only allocate downstream if the element is passthrough or always_in_place , in which case we're guaranteed it will not need a buffer of a different size.

I'm not 100% sure about when those variables are set though ... but your testcase now works.
Comment 5 Edward Hervey 2006-08-21 09:21:04 UTC
2006-08-21  Edward Hervey  <edward@fluendo.com>

	* libs/gst/base/gstbasetransform.c:
	(gst_base_transform_buffer_alloc):
	Only call downstream buffer_alloc if transform element is passthrough
	or always_in_place. Closes #350449.