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 551509 - gst_base_transform_prepare_output_buffer: assertion failed: (*out_buf != NULL)
gst_base_transform_prepare_output_buffer: assertion failed: (*out_buf != NULL)
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other Linux
: Normal blocker
: 0.10.21
Assigned To: Wim Taymans
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2008-09-09 13:39 UTC by Stefan Sauer (gstreamer, gtkdoc dev)
Modified: 2008-09-28 21:19 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
unit test which reproduces the issue (4.52 KB, patch)
2008-09-12 17:15 UTC, Tim-Philipp Müller
none Details | Review
alternative patch (6.75 KB, patch)
2008-09-24 22:20 UTC, Jan Schmidt
none Details | Review
updated patch (7.13 KB, patch)
2008-09-27 01:04 UTC, Jan Schmidt
accepted-commit_now Details | Review

Description Stefan Sauer (gstreamer, gtkdoc dev) 2008-09-09 13:39:23 UTC
GST_DEBUG="*:4" G_DEBUG="fatal_warnings" gdb --args gst-launch-0.10 -v filesrc location=test.jpg ! queue ! image/jpeg, framerate=\(fraction\)15/1 ! jpegdec ! freeze ! ffmpegcolorspace ! xvimagesink

0:00:10.161792872 26686  0x81d4d10 DEBUG             GST_CAPS gstpad.c:2601:gst_pad_get_allowed_caps:<capsfilter0:src> allowed caps image/jpeg, framerate=(fraction)15/1, width=(int)[ 16, 4096 ], height=(int)[ 8, 4096 ]
0:00:10.161833313 26686  0x81d4d10 DEBUG           capsfilter gstcapsfilter.c:336:gst_capsfilter_prepare_buf:<capsfilter0> Have unfixed output caps image/jpeg, framerate=(fraction)15/1, width=(int)[ 16, 4096 ], height=(int)[ 8, 4096 ]
**
** ERROR:(gstbasetransform.c:1144):gst_base_transform_prepare_output_buffer: assertion failed: (*out_buf != NULL)
Comment 1 Stefan Sauer (gstreamer, gtkdoc dev) 2008-09-09 13:49:49 UTC
This is because of jpegdec having width and heigh on its sink-caps. If I determine sizes and use full caps, I don't get the gstbasetransform-assertion, but 
0:00:00.121062159 27204  0x81cbfd0 WARN               jpegdec gstjpegdec.c:1042:gst_jpeg_dec_chain:<jpegdec0> error: Failed to decode JPEG image
0:00:00.121425424 27204  0x81cbfd0 WARN               jpegdec gstjpegdec.c:1042:gst_jpeg_dec_chain:<jpegdec0> error: Error #69: Unsupported marker type 0x%02x
0:00:00.121537736 27204  0x81cbfd0 WARN               basesrc gstbasesrc.c:2234:gst_base_src_loop:<filesrc0> error: Internal data flow error.
0:00:00.121604089 27204  0x81cbfd0 WARN               basesrc gstbasesrc.c:2234:gst_base_src_loop:<filesrc0> error: streaming task paused, reason error (-5)
ERROR: from element /GstPipeline:pipeline0/GstJpegDec:jpegdec0: Failed to decode JPEG image

setting the framerate via capsfilter is the method we document for pngdec/jpegdec to not eos after first picture. This is needed to produce a stream with freeze to e.g. overlay an image on a video with videomixer.
Comment 2 Tim-Philipp Müller 2008-09-11 19:53:10 UTC
> setting the framerate via capsfilter is the method we document for
> pngdec/jpegdec to not eos after first picture. This is needed to
> produce a stream with freeze to e.g. overlay an image on a video
> with videomixer.

Where is this documented?

If you put a caps filter *before* jpegdec and set a framerate there, jpegdec will assume the input is a stream of multiple jpeg pictures and packetised, ie. one-buffer-per-image. The above 'failed to decode' error is due to you not specifying the right block-size on the filesrc, so jpegdec expects a full picture as input but only gets a chunk of it.

Basetransform shouldn't abort of course, this definitly needs fixing.
Comment 3 Stefan Sauer (gstreamer, gtkdoc dev) 2008-09-12 06:52:41 UTC
Okay, I then I was misunderstanding. I've tried to overlay a video with an image and what I understood is that putting the capsfilter with a framerate infront of jpegdec/pngdec will tell them to not eos, so that e.g. freeze and make a stream of  the image. I think we have some conceptual problmes with that, but that a different issue. Lets have a look on the basetransform in the scope of this report only.
Comment 4 Tim-Philipp Müller 2008-09-12 17:15:51 UTC
Created attachment 118606 [details] [review]
unit test which reproduces the issue
Comment 5 Jan Schmidt 2008-09-23 18:16:04 UTC
I think we should commit the unit test, and then be forced to fix it...
Comment 6 Jan Schmidt 2008-09-24 22:20:45 UTC
Created attachment 119338 [details] [review]
alternative patch

I think the real bug here is that capsfilter can't apply incompletely specified caps to a buffer, so in the case where it's been asked to (the incoming buffers have no caps, but capsfilter has some filtercaps) it should error out if the result is not completely fixed.

As an aside, I converted the assert in basetransform into a GST_FLOW_ERROR, because while the sub-class should be providing a buffer or erroring in all cases, it doesn't have to lead to an assert when a runtime error would be better.

Changed the unit test to check that in fact the test case does *not* succeed in producing output buffers.
Comment 7 Wim Taymans 2008-09-25 11:19:57 UTC
all right!
Comment 8 Aurelien Grimaud 2008-09-25 19:27:51 UTC
With previous revisions, such pipeline was RUNNING :
gst-launch -v  udpsrc ! capsfilter caps="application/x-rtp, media=(string)video, clock-rate=(int)90000, encoding-name=(string)MP4V-ES, profile-level-id=(string)1, config=(string)000001b001000001b58913000001000000012000c48d8800650584121463000001b24c61766335312e34382e30" ! rtpmp4vdepay ! fakesink
With new basetransform, Jan Schmidt GST_FLOW_ERROR occurs.

Moreover :
This, 

gst-launch -v  udpsrc ! capsfilter caps="application/x-rtp, media=(string)video, clock-rate=(int)90000, encoding-name=(string)MP4V-ES, profile-level-id=(string)1, config=(string)000001b001000001b58913000001000000012000c48d8800650584121463000001b24c61766335312e34382e30" ! rtpmp4vdepay ! ffdec_mpeg4 ! xvimagesink

does not work, but setting the caps property of udpsrc displays the video :

gst-launch -v  udpsrc caps="application/x-rtp, media=(string)video, clock-rate=(int)90000, encoding-name=(string)MP4V-ES, profile-level-id=(string)1, config=(string)000001b001000001b58913000001000000012000c48d8800650584121463000001b24c61766335312e34382e30" ! rtpmp4vdepay ! ffdec_mpeg4 ! xvimagesink

this works fine too :
gst-launch -v  udpsrc ! capsfilter caps="application/x-rtp, media=(string)video, clock-rate=(int)90000, encoding-name=(string)MP4V-ES, profile-level-id=(string)1, *payload=96*, config=(string)000001b001000001b58913000001000000012000c48d8800650584121463000001b24c61766335312e34382e30" ! rtpmp4vdepay ! ffdec_mpeg4 ! xvimagesink

And it works with whatever payload which is rtpmp4vdepay compatible (dynamic payload)

Do you think that not running is the right thing to do (because of caps negotiation failure on payload), or something was broken ?
Previously, basetransform was allocating buffer when subclass did not. Why no more ?
Comment 9 Jan Schmidt 2008-09-25 21:03:04 UTC
As you noticed, the top pipelines fail to supply the payload parameter. I'm not sure how that worked before. I expected that it should fail because the buffers didn't have a parameter that the sink pad template had requested.
Comment 10 Aurelien Grimaud 2008-09-26 07:40:37 UTC
Yes, of course, according to caps policy, this obviously must fail.
But payload is not necessary for the pipeline to work.
So payload cap in rtpmp4gdepay is not mandatory, this is just a restriction to dynamic payload.
Should payload be removed of rtp depacketizer / packetizer caps and considered optional as profile-level-id and config already are ?
I did not see any other case of such behavior, but considering a pipeline cannot work because of a non useful parameter is a bit weird.

I think it was working before, because basetransformed provided a buffer whenever the subclass did not, and payload is unused ny downstream elements ...
Comment 11 Jan Schmidt 2008-09-26 12:38:39 UTC
Anyone else want to weigh in here? Should I just restore the old behaviour for this case, or is the test case buggy?
Comment 12 Wim Taymans 2008-09-26 13:13:45 UTC
It's not unreasonable to fail when you specify an incomplete capsfilter so I'm happy with this. Theoretically, the depayloader should fail if there is no payload type set either, don't know why it does not fail...

Another option is to remove the remaining unfixed properties from the caps in capsfilter?
Comment 13 Wim Taymans 2008-09-26 13:14:49 UTC
Oh right, it does not fail because we check intersection on caps instead of a subset.
Comment 14 Wim Taymans 2008-09-26 13:29:01 UTC
Also, capsfilter is not meant to set caps on buffers but to _filter out_ the different possibilities. In this use case, a capsfilter is clearly the wrong thing to use.
Comment 15 Jan Schmidt 2008-09-26 13:39:12 UTC
so effectively a sink pad accepts any caps where the media type matches and
none of the supplied properties directly conflict?

We've deliberately enabled capsfilter to set caps in the specific case of the input buffers having no caps of their own, but in general - yes you're right.
Comment 16 Aurelien Grimaud 2008-09-26 13:51:25 UTC
This is not blocking for me, this was just a different behavior I wanted to point out.
Fell free to commit this new behavior.
Comment 17 Jan Schmidt 2008-09-27 00:51:33 UTC
OK, how about this version - it's the same as the first patch, but it restores the basetransform behaviour that a subclass can return OK without preparing a special buffer and it will fall back to pad_alloc.
Comment 18 Jan Schmidt 2008-09-27 01:04:47 UTC
Created attachment 119462 [details] [review]
updated patch
Comment 19 Jan Schmidt 2008-09-28 21:19:37 UTC
OK, committing as is and making a new pre-release. Can fix it and make another if you think it's not right.

2008-09-28  Jan Schmidt  <jan.schmidt@sun.com>

        * libs/gst/base/gstbasetransform.c:
        * plugins/elements/gstcapsfilter.c:
        * tests/check/Makefile.am:
        * tests/check/elements/.cvsignore:
        * tests/check/elements/capsfilter.c:
        Fix assertion in basetransform when the subclass chooses not to 
        allocate a buffer in prepare_buffer(), and make capsfilter error out
        cleanly if requested to apply caps that don't completely specify the 
        buffer. Fixes #551509