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 417420 - [autoaudiosink] add "caps" property to filter sinks by caps
[autoaudiosink] add "caps" property to filter sinks by caps
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other All
: Normal enhancement
: 0.10.7
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2007-03-12 11:01 UTC by Stefan Sauer (gstreamer, gtkdoc dev)
Modified: 2007-12-05 16:03 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
adds caps property and logic to handle it (5.05 KB, patch)
2007-03-12 11:02 UTC, Stefan Sauer (gstreamer, gtkdoc dev)
none Details | Review
gstautoaudiosink_filter_caps.patch (4.86 KB, patch)
2007-03-13 08:27 UTC, Tommi Myöhänen
committed Details | Review

Description Stefan Sauer (gstreamer, gtkdoc dev) 2007-03-12 11:01:47 UTC
The patch adds a "caps" property to autoaudiosink. If this property is set, choosing the sink will be solely based on the caps. If it's not set the operation will be as usual: autoaudiosink will choose the sink it can set to ready state successfully. 

This will avoid two problems:
- we don't want to switch dspsinks from null to ready and back to null again during autodetect, since some dspsinks open the dsp node at null to ready state change. This would result in opening dsp node twice and slowing down the startup
- originally playbin doesn't link the autoaudiosink to decodebin2 before setting it to paused/playing, so the actual sink needs to be chosen while unlinked. However with compressed audio autoaudiosink still needs to get caps information from somewhere in order to choose the specific correct sink.
Comment 1 Stefan Sauer (gstreamer, gtkdoc dev) 2007-03-12 11:02:48 UTC
Created attachment 84422 [details] [review]
adds caps property and logic to handle it
Comment 2 Tim-Philipp Müller 2007-03-12 11:43:55 UTC
I think the state change issue should already be fixed in CVS:

  2007-03-09  Jan Schmidt  <thaytan@mad.scientist.com>

        * gst/autodetect/gstautoaudiosink.c: (gst_auto_audio_sink_find_best):
          Tim and I can't think of any reason the child audio sink needs to 
          be set back to NULL after successfully determining that it can 
          reach READY - it gets immediately set back to READY by the caller
          anyway, causing an unnecessary close/open of any audio devices
          involved.



> - originally playbin doesn't link the autoaudiosink to decodebin2 before
> setting it to paused/playing, so the actual sink needs to be chosen while
> unlinked. However with compressed audio autoaudiosink still needs to get caps
> information from somewhere in order to choose the specific correct sink.

Why 'originally'? Are you using a modified version of playbin? Where do the caps come from in your context (your modified playbin?)


Some nitpicking regarding the patch:

 - I'd use gst_value_{set|get}_caps() instead of the boxed
   functions (it's clearer and won't require changing when
   GstCaps is changed to some other base type like
   GstMiniObject)

 - I think the property name should be more specific than
   just "caps" (don't ask me what though)

 - my gut feeling is that the state change check shouldn't
   be skipped when filter caps are specified (you did that
   to avoid the double state change, so that shouldn't be
   necessary any longer, right?), ie. no specified caps
   should result in the same behaviour as filter caps "ANY"
   IMHO.

Comment 3 Tommi Myöhänen 2007-03-12 15:16:00 UTC
(In reply to comment #2)
> Why 'originally'? Are you using a modified version of playbin? Where do the
> caps come from in your context (your modified playbin?)

Yes, playbin has been modified. It just passes src caps from decodebin2 to autoaudiosink. It would be nice to use normal caps negotiation here instead of a property, but as long as sink needs to be in same state (paused/playing) with existing pipeline when linking, it doesn't work.

>  - I'd use gst_value_{set|get}_caps() instead of the boxed
>    functions (it's clearer and won't require changing when
>    GstCaps is changed to some other base type like
>    GstMiniObject)

Yes, let's do this.

>  - I think the property name should be more specific than
>    just "caps" (don't ask me what though)

How about "filter_caps"? Or "sink_candidate_filter_caps"?
   
>  - my gut feeling is that the state change check shouldn't
>    be skipped when filter caps are specified (you did that
>    to avoid the double state change, so that shouldn't be
>    necessary any longer, right?), ie. no specified caps
>    should result in the same behaviour as filter caps "ANY"
>    IMHO.

This is ok too.

Thanks for comments
Comment 4 Jan Schmidt 2007-03-12 15:41:44 UTC
Just to chime in with my 2c:

I have no problem with using a set of filtering caps a la decodebin2, I just wanted to point out that the other aspect of decodebin2's (fluctuating) API might be useful too: the autoplug-sort signal.

Using such a signal, it'd be possible to attempt to choose a dspsink when possible, or if not, fall back to a standard PCM based sink and continue plugging within decodebin2 to do software decoding.

If it turns out that such a thing would be desirable, it can of course be added later... I'm just rambling.
Comment 5 Tommi Myöhänen 2007-03-13 08:24:23 UTC
(In reply to comment #4)
> Just to chime in with my 2c:
> 
> I have no problem with using a set of filtering caps a la decodebin2, I just
> wanted to point out that the other aspect of decodebin2's (fluctuating) API
> might be useful too: the autoplug-sort signal.

This looks like one alternative, but I still like the idea of keeping autoaudiosink responsible for choosing the sink. With autoplug-sort I think playbin or application would require logic to choose the sink.

Comment 6 Tommi Myöhänen 2007-03-13 08:27:02 UTC
Created attachment 84487 [details] [review]
gstautoaudiosink_filter_caps.patch

Updated patch
Comment 7 René Stadler 2007-03-13 13:34:59 UTC
What's the use case beside using this in your modified version of playbin?
Comment 8 Tommi Myöhänen 2007-03-13 13:53:55 UTC
(In reply to comment #7)
> What's the use case beside using this in your modified version of playbin?
> 

It can be used to restrict the set of sinks from which autoaudiosink chooses the actual sink. I'm not sure if anyone else wants to use this feature, maybe if a system has a large amount of audio sinks and someone wants to have more control over what actual sink the autoaudiosink chooses.
Comment 9 René Stadler 2007-03-13 20:04:45 UTC
(In reply to comment #8)
...
> It can be used to restrict the set of sinks from which autoaudiosink chooses
> the actual sink. I'm not sure if anyone else wants to use this feature, maybe
> if a system has a large amount of audio sinks and someone wants to have more
> control over what actual sink the autoaudiosink chooses.

Yeah I got that.  What I cannot find is a specific example of where this gives any benefit.  Non-raw sinks with a rank >= MARGINAL currently cause problems; the whole auto-plugging concept is geared towards what is needed for the playbin/decodebin/auto*sink elements to work together.  For playback of raw caps, converters are needed between decodebin and sinks because sinks don't support all raw format variants.  Playbin does just that, but ... ! audioresample ! audioconvert ! autoaudiosink fatally breaks for non-raw sinks.

The fact that you use non-raw sinks for autoplugging implies that these sinks have a rank >= MARGINAL.  Since such a setup breaks an unmodified playbin, the patch at least has to be modified such that the default value for the property is GST_AUDIO_INT_PAD_TEMPLATE_CAPS instead, which makes a lot of sense to me.

I'm still unsure that adding the property is the right way to do this.  The big picture here is that the auto-plugging to non-raw caps that decodebin2 introduces forces one to rethink the way the whole auto-plugging concept works.  You already mentioned the gist of it: It would be better to use caps nego.  This is because with non-raw sinks, the automatic sink selection additionally depends on the caps it should process.

Neither playbin nor autoaudiosink were designed to make full use of decodebin2.  What's really in order is a playbin2 and a reshaped autosink.
Comment 10 Tommi Myöhänen 2007-03-14 15:04:53 UTC
Yes, this is about supporting audio sinks consuming compressed audio and also playbin needs tweaking to support them. How does playbin2 look like, can it plug autosinks so that they get caps information via normal caps negotiation and can use this information when choosing the actual sink?
Comment 11 Jan Schmidt 2007-03-14 16:08:51 UTC
At the moment, playbin2 is modelled on a large pipe that delivers dreams. ie, it doesn't look like anything yet.

I think it is extremely unlikely that caps negotiation and autopluggin will ever interact in a way that would allow this behaviour - the semantics of this scenario as I see them are: 

a) wait until decodebin2 indicates that encoded audio is a possibility, in a format that you *might* have a hardware decoding sink for.
b) go try your sinks and see if one manages to attain READY state
then
c1) if you found a sink, tell decodebin2 you're happy to accept the encoded audio directly so it will output it, then connect that to your hardware sink
or
c2) none of the sinks worked/are available and you need software decoding so you should get decodebin2 to continue plugging decoders on this stream and get autoaudiosink to find you a decoder for a raw audio stream instead.

Similar logic applies to the (also imaginary at the moment) case of using an XvMC sink to collaborate in decoding MPEG-2.

Further complications arise if we want to consider how you'd handle switching between software decoding and hardware decoding on the fly - for example switching between an mp3 track and an AC-3 track within one media file. I don't know how that would work, either technically or functionally from a user's perspective.
Comment 12 Wim Taymans 2007-12-04 12:15:50 UTC
ok, I need to move this forward now. The caps property would indeed the solve the problem at hand but it is not the way to go. It would be better if the autoaudiosink would wait for capsnego on the sinkpad before trying to select and plug a working sink. 

For selecting a sink, we would then use a regular capsfilter instead of a caps property. 

about Commment #9: yes, we'll have to add ranks to the non-raw audio sinks to make  them autopluggable in such a way that the parsers are still put in front of them. It seems doing the capsnego on the sinkpad and also exposing a sane template caps on the sinkpad (based on available sink element factories) would work.

Comment #11: This is how decodebin2 and playbin2 would negotiate the sinks. Unfortunatly when you add visualisations and subtitles, the non-raw sink matter becomes a bit more difficult again (do you software-decode only for the visualisation or not).

Comment 13 Wim Taymans 2007-12-05 14:29:59 UTC
Going to apply the caps property with a default caps equal to raw int and float audio. I fear delaying the sink selection to capsnego is going to be too much of a change for a stable element.
Comment 14 Wim Taymans 2007-12-05 16:03:14 UTC
        Patch by: Tommi Myöhänen  <ext-tommi dot myohanen at nokia dot com>

        * gst/autodetect/gstautoaudiosink.c:
        (gst_auto_audio_sink_class_init), (gst_auto_audio_sink_dispose),
        (gst_auto_audio_sink_init), (gst_auto_audio_sink_find_best),
        (gst_auto_audio_sink_set_property),
        (gst_auto_audio_sink_get_property):
        * gst/autodetect/gstautoaudiosink.h:
        Add property to filter sinks based on caps. Only select raw audio sinks
        by default for backwards compat.  Fixes #417420.
        API: GstAutoAudioSink::filter-caps