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 677401 - [audioparsers][videoparsers] Return too strict caps in sink getcaps functions
[audioparsers][videoparsers] Return too strict caps in sink getcaps functions
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
0.11.x
Other Mac OS
: Normal major
: 0.11.x
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2012-06-04 16:56 UTC by Matej Knopp
Modified: 2013-12-03 21:28 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch (961 bytes, patch)
2012-06-04 16:56 UTC, Matej Knopp
committed Details | Review

Description Matej Knopp 2012-06-04 16:56:21 UTC
Created attachment 215565 [details] [review]
Patch

(there might be similar issue for other parsers)

AC3 parser has "audio/x-ac3" in sink template caps, but will refuse caps event until it contains channels and rate;

The caps get refused in event pre-check, which queries AC3 parser sink caps to test if new caps is subset of it; However the sink pads are created in gst_ac3_parse_get_sink_caps which copies the caps from SRC pad - where there is channels and rate already set.
Comment 1 Matej Knopp 2012-06-04 17:12:27 UTC
Actually, the channels and rate properties are range, should gst_caps_is_subset still return false in this case?

i.e.

gst_caps_is_subset("audio/x-ac3", "audio/x-ac3,channels = (int) [ 1, 6 ], rate = (int) [ 8000, 48000 ]") return false? has this changed recently?
Comment 2 Matej Knopp 2012-06-04 17:31:18 UTC
This patch is probably wrong; The behavior seems to be caused by fix for #677335
Comment 3 Matej Knopp 2012-06-04 21:51:26 UTC
So, AC3 parser has template SINK caps "audio/x-ac3; " "audio/x-eac3; " "audio/ac3" but if I send it CAPS event with "audio/x-ac3" and a buffer afterwards it will refuse the caps because of gst_ac3_parse_get_sink_caps.

gst_ac3_parse_get_sink_caps returns "audio/x-ac3, channels = (int) [ 1, 6 ], rate = (int) [ 8000, 48000 ]". 

The pre-check fails, because audio/x-ac3 is not a subset of audio/x-ac3, channels = (int) [ 1, 6 ], rate = (int) [ 8000, 48000 ]".

I was told the channels and rate is propagated from upstream so that a filter upstream can restrict the rate or channels.
Comment 4 Matej Knopp 2012-06-04 21:51:59 UTC
Similar issue with MPEG4 parser which now requires framerate and dimensions on sink caps otherwise will refuse it.
Comment 5 Sebastian Dröge (slomo) 2012-06-05 06:40:41 UTC
The parsers should probably return what they currently return plus "audio/x-ac3" or whatever without any unnecessary fields appended to the end.

This way upstream elements have the possibility to decide on the format that downstream wants but the parser still accepts unparsed AC3 (for the case when upstream does not know about the number of channels, etc).
Comment 6 Sebastian Dröge (slomo) 2012-06-05 07:31:10 UTC
commit ca4b5d795bf52ea434470475e2dac6a149282c10
Author: Sebastian Dröge <sebastian.droege@collabora.co.uk>
Date:   Tue Jun 5 09:18:12 2012 +0200

    audioparsers: Fix GstBaseParse::get_sink_caps() implementations
    
    They should take the filter caps into account and always return
    the template caps appended to the actual caps. Otherwise the
    parsers stop to accept unparsed streams where upstream does not
    know about channels, rate, etc.
    
    Fixes bug #677401.


commit 7c6093357b5904c2e5dc7f3c9140748c85575349
Author: Sebastian Dröge <sebastian.droege@collabora.co.uk>
Date:   Tue Jun 5 09:30:00 2012 +0200

    videoparsers: Fix GstBaseParse::get_sink_caps() implementations
    
    They should take the filter caps into account and always return
    the template caps appended to the actual caps. Otherwise the
    parsers stop to accept unparsed streams where upstream does not
    know about width, height, etc.
    
    Fixes bug #677401.
Comment 7 Wim Taymans 2013-12-03 17:38:26 UTC
This patch is not correct, I think:

The getcaps function should always proxy the (additional) constraints upstream
to make upstream elements select an acceptable format. You can't add the
template caps because then you simply ignore the contraints.

To also make the parser accept caps with missing fields (because it's a parser and it can figure that out itself) you would need to implement a custom acceptcaps function that checks if there is an _intersection_ of the input caps and the result of getcaps.
Comment 8 Wim Taymans 2013-12-03 21:28:57 UTC
commit e0a5c07e8d404e72654e016fa0a6c88d573f51f1
Author: Wim Taymans <wtaymans@redhat.com>
Date:   Tue Dec 3 21:41:28 2013 +0100

    audioparsers: use ACCEPT_INTERSECT flag
    
    The parser can accept input that is not completely specified. Use the
    ACCEPT_INTERSECT flag on the sinkpad to tweak the acceptcaps function to
    check for intersection only. This allows us to proxy downstream
    constraints while still allowing non-subset caps as input.
    We can then also remove the appended template caps workaround.
    Make a unit-test to check the new feature.
    
    This reverts commit 26040ee38cb9e7c42f3d9a0282b3e5cace7ca42d
    
    Fixes https://bugzilla.gnome.org/show_bug.cgi?id=705024