GNOME Bugzilla – Bug 658541
[API change] caps negotiation failures
Last modified: 2011-09-09 10:57:07 UTC
Recent commits to core [*] changed some default caps acceptance behaviour. Incoming caps are now required to have all fields as indicated in accepted pad caps (e.g. pad template caps). Presently, this introduces lots of caps negotiation failures. E.g. ffmpeg decoders have width and height fields in caps, but can (and do) typically determine this from the data itself, whereas depayloaded caps usually do not hold width and height info. In practice, this may require stripping all sorts of pad templates of almost all fields (since they need not always be present and also often not necessarily needed) and to rely on subsequent (setcaps) negotiation to determine whether or not all needed is available. On the other hand, such stripping does not help auto-plugging. Last but not least and unfortunately (in case we care, as afaik we usually do) it could (and probably will) break 3rd party demuxers and/or decoders (or whatever) whose template/caps handling is not "fixed" according to the new behaviour. Some other example problems: ffdec_aac caps: audio/mpeg channels: [ 1, 6 ] rate: [ 4000, 96000 ] mpegversion: { 2, 4 } stream-format: { raw, adts, adif } but I would not bet on depayloaded caps to have stream-format set (or even rate or channels), or for this to be set on all demuxers caps or wherever it may be coming from. mad caps: audio/mpeg mpegversion: 1 layer: [ 1, 3 ] rate: { 8000, 11025, 12000, 16000, 22050, 24000, 32000, 44100, 48000 } channels: [ 1, 2 ] Same here basically, little hope for depayloaded caps, and not sure for all above details on other incoming caps "in the wild" either. [*] http://cgit.freedesktop.org/gstreamer/gstreamer/commit/?id=0c5d50207326d74a4805bcd898bfac887540f12b http://cgit.freedesktop.org/gstreamer/gstreamer/commit/?id=5e5cc5e89e7e2858a6352fa4c81a374f6e5a6297
It also breaks mpegvideoparse from -bad, which expects parsed=false in its sink. This breaks autoplugging when playing a MPEG TS stream, trying to set: video/mpeg, mpegversion=(int)2, systemstream=(boolean)false when the pad template wants: video/mpeg, mpegversion=(int)[ 1, 2 ], parsed=(boolean)false, systemstream=(boolean)false
Elements should only have fields on their sinkpad caps template that they really require and most (or almost all) elements are assuming that this is enforced by the core (but actually wasn't before). This is the documented behaviour. Unfortunately it wasn't enforced by the core before and some plugins were relying on this undocumented behaviour. I'd revert this for 0.10 then and add a g_warning() to acceptcaps() if it returns TRUE for non-subset caps but keep this behaviour for 0.11. Other opinions?
Created attachment 195969 [details] [review] mpegvideoparse: remove parsed=false from sink template caps This fixes autoplugging failing with the new change to determine if some caps are a subset of another, as the incoming caps would not include parsed.
commit 79b5e89015eadae378152a9d23ab9d3bf3b98b7a Author: Sebastian Dröge <sebastian.droege@collabora.co.uk> Date: Thu Sep 8 13:40:06 2011 +0200 pad: Print a g_warning() if pad accept caps that are not a subset of its caps In 0.11 only subsets are supported again as documented instead of also allowing non-empty intersections. commit 0bc6d49c950210bf422615fb8dc98c5adcd5e456 Author: Sebastian Dröge <sebastian.droege@collabora.co.uk> Date: Thu Sep 8 13:26:24 2011 +0200 Revert "basetransform: Use check for subsets and not non-empty intersection to check if caps are compatible" This reverts commit 5e5cc5e89e7e2858a6352fa4c81a374f6e5a6297. See bug #658541. commit 2bfada5581e35a2d37188f48a2c7442644f10bb3 Author: Sebastian Dröge <sebastian.droege@collabora.co.uk> Date: Thu Sep 8 13:26:01 2011 +0200 Revert "pad: Check for subsets, not non-empty intersections to check if caps are compatible" This reverts commit 0c5d50207326d74a4805bcd898bfac887540f12b. See bug #658541.
Created attachment 196068 [details] some new warnings attacched The attacched warnings
the attached warnings are generated using this pipeline: gst-launch -e videomixer name=mix sink_1::xpos=50 sink_1::ypos=20 sink_1::zorder=0 sink_3::xpos=80 sink_3::ypos=340 sink_3::zorder=0 sink_2::zorder=5 sink_0::xpos=400 sink_0::ypos=20 sink_0::zorder=0 ! ffmpegcolorspace ! videorate ! video/x-raw-yuv, framerate=\(fraction\)25/1 ! ffmpegcolorspace ! xvimagesink sync=false v4l2src ! queue ! videoscale ! videorate ! ffmpegcolorspace ! video/x-raw-yuv,framerate=\(fraction\)25/1, width=320, height=240 ! queue ! mix. multifilesrc do-timestamp=true location=space-04.png caps='image/png,framerate=1/1' ! pngdec ! alphacolor ! ffmpegcolorspace ! videorate ! video/x-raw-yuv,framerate=\(fraction\)25/1 ! queue ! mix. rtspsrc location=rtsp://192.168.2.136/axis-media/media.amp ! rtph264depay ! h264parse ! ffdec_h264 ! queue ! videoscale ! videorate ! ffmpegcolorspace ! video/x-raw-yuv, framerate=\(fraction\)25/1, width=320, height=240 ! queue ! mix. rtspsrc location=rtsp://192.168.2.17/udpstream ! rtpmp4vdepay ! ffdec_mpeg4 ! queue ! videoscale ! videorate ! ffmpegcolorspace ! video/x-raw-yuv, framerate=\(fraction\)25/1, width=320, height=240 ! queue ! mix.
Oops, should be better with this now: commit 22ba786807cbc43db17e0c816accd548ba531f7a Author: Sebastian Dröge <sebastian.droege@collabora.co.uk> Date: Fri Sep 9 12:56:20 2011 +0200 pad: Only do the subset check in gst_pad_accept_caps() if the pad claims to accept