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 657179 - pulse: New pulseaudiosink element to handle format changes
pulse: New pulseaudiosink element to handle format changes
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
unspecified
Other All
: Normal normal
: 0.10.31
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2011-08-23 17:12 UTC by Arun Raghavan
Modified: 2011-09-19 03:26 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
pulse: New pulsedecsink element to handle format changes (39.14 KB, patch)
2011-08-23 17:12 UTC, Arun Raghavan
none Details | Review
pulse: New pulseaudiosink element to handle format changes (39.43 KB, patch)
2011-09-12 12:17 UTC, Arun Raghavan
committed Details | Review
pulse: New pulseaudiosink element to handle format changes (39.43 KB, patch)
2011-09-15 11:42 UTC, Arun Raghavan
none Details | Review

Description Arun Raghavan 2011-08-23 17:12:38 UTC
Attaching a new bin element that wraps around pulsesink and makes passthrough handline smoother. This seems to be the shortest non-confusing name I can come up with, but I'm open to other suggestions (iow, let the bikeshedding begin! :)). Also, thanks to slomo for a first round of review.
Comment 1 Arun Raghavan 2011-08-23 17:12:41 UTC
Created attachment 194499 [details] [review]
pulse: New pulsedecsink element to handle format changes

This introduces a new bin which wraps around pulsesink and depending on
the formats supported by the sink, plugs in/out a decodebin2 as
required. This allows users to switch sinks on the stream and adapts
accordingly (for example, you could watch a movie in passthrough mode on
your receiver which supports AC3 decode, then plug out and switch to a
non-digital profile to continue uninterrupted on analog output).

The bin is required because doing the same with playbin2/playsink will
require API changes that cannot be made in 0.10. With 0.11/1.0, we
should be able to ask for upstream caps renegotiation to deal with all
this.
Comment 2 Stefan Sauer (gstreamer, gtkdoc dev) 2011-08-26 14:01:39 UTC
Is the bin specific to pulseaudio? Just wonder if the bin could work with other audiosinks too.
Comment 3 Arun Raghavan 2011-08-26 15:27:47 UTC
(In reply to comment #2)
> Is the bin specific to pulseaudio? Just wonder if the bin could work with other
> audiosinks too.

I don't know of any other sink that could allow this kind of dynamically switching of formats.
Comment 4 Sebastian Dröge (slomo) 2011-09-02 13:02:49 UTC
What should happen with this patch? I've reviewed it a long time ago and it looked good back then
Comment 5 Tim-Philipp Müller 2011-09-02 23:19:57 UTC
It should go in. But maybe we were waiting for a better name ;)
Comment 6 Tim-Philipp Müller 2011-09-12 10:57:10 UTC
I think we've settled for "pulseaudiosink" now.
Comment 7 Arun Raghavan 2011-09-12 12:17:21 UTC
Created attachment 196247 [details] [review]
pulse: New pulseaudiosink element to handle format changes

This introduces a new bin which wraps around pulsesink and depending on
the formats supported by the sink, plugs in/out a decodebin2 as
required. This allows users to switch sinks on the stream and adapts
accordingly (for example, you could watch a movie in passthrough mode on
your receiver which supports AC3 decode, then plug out and switch to a
non-digital profile to continue uninterrupted on analog output).

The bin is required because doing the same with playbin2/playsink will
require API changes that cannot be made in 0.10. With 0.11/1.0, we
should be able to ask for upstream caps renegotiation to deal with all
this.
Comment 8 Stefan Sauer (gstreamer, gtkdoc dev) 2011-09-12 12:57:03 UTC
(In reply to comment #6)
> I think we've settled for "pulseaudiosink" now.

Did you meant to suggest "pulse*auto*sink"? Having pulsesink and pulseaudiosink would imho be confusing.
Comment 9 Stefan Sauer (gstreamer, gtkdoc dev) 2011-09-12 13:00:14 UTC
(presses enter too quickly)

imho the new sink is not more 'audio' than the other. Thus I'd rather call it: pulseautosink, pulsedecsink, pulseconvsink, pulsesinkbin (may be not) or something like it.
Comment 10 Tim-Philipp Müller 2011-09-12 13:21:29 UTC
No, I did not mean to suggest pulseaudiosink.

We've discussed this on IRC multiple times now, and there just isn't a great solution.

Whatever we do will be confusing in some way.

The problem is this:

 - it's pretty much impossible to name the sink in
   a way that expresses what it does or what it's
   good for (pulsecompressedaudiopassthroughsink
   is a tad unwieldy).

 - naming it after some technical implementation
   detail that's not even directly related to the
   main purpose of the element in an obvious
   way (e.g. 'dec') is also confusing. In this case
   even more so, because the whole point is
   to actually not decode anything.

 - ideally we would just replace the existing
   "pulsesink" element and proxy the properties,
   but we can't do that, because there might
   be code out there that relies on pulsesink
   deriving from GstAudioSink. So we need to
   keep pulsesink and add a new element
   with higher rank for autoplugging.



> imho the new sink is not more 'audio' than the other

That's true, but not the point. It's just a different name without using the original one.


> pulseautosink, pulsedecsink, pulseconvsink, pulsesinkbin (may be not) or
> something like it

If it's a choice between those and pulsemagicsink, I'd still go for pulsemagicsink I think ;)

 - pulsedecsink: even more confusing, see above

 - pulsesinkbin: meh, why is this better than pulseaudiosink?
   It's worse IMHO

 - pulseconvsink: abbreviation, and what does it convert? Again,
   the whole point is not to decode/convert (the "convert" bit
   is a really low-level implementation detail, to a normal
   technical user you just "pass AC-3 from here to there")

 - pulseautosink: I could live with that, but don't really see
   why it's better than just pulsesink or pulseaudiosink. What
   does the 'auto' stand for. From a user point-of-view,
   pulseaudio supports these features, and our element
   should expose that, so the "auto" is really just what the
   default behaviour should be from a user-point-of-view IMHO.


Applying the KISS principle, I still think pulseaudiosink is the least worst, but if you find support for some other name, I could probably live with that as well.
Comment 11 Stefan Sauer (gstreamer, gtkdoc dev) 2011-09-12 14:06:25 UTC
pulseplaysink would be another option, but its not much better either. As said on irc, I am just worried that people will start using pulseaudiosink by default as it sounds more audio, which is wrong, as it mostly makes sense in players.

Anyway if no one else is concerned about the name here, I'll be silent now.
Comment 12 Arun Raghavan 2011-09-15 11:41:36 UTC
Okay, I think let's settle on pulseaudiosink as the least bad solution. This is something that's not going to stay with us forever (should be fixed in 1.0 to just work with pulsesink and playbin2). If there are no comments on the code itself, I'll commit in a while.
Comment 13 Arun Raghavan 2011-09-15 11:42:31 UTC
Created attachment 196608 [details] [review]
pulse: New pulseaudiosink element to handle format changes

This introduces a new bin which wraps around pulsesink and depending on
the formats supported by the sink, plugs in/out a decodebin2 as
required. This allows users to switch sinks on the stream and adapts
accordingly (for example, you could watch a movie in passthrough mode on
your receiver which supports AC3 decode, then plug out and switch to a
non-digital profile to continue uninterrupted on analog output).

The bin is required because doing the same with playbin2/playsink will
require API changes that cannot be made in 0.10. With 0.11/1.0, we
should be able to ask for upstream caps renegotiation to deal with all
this.
Comment 14 Arun Raghavan 2011-09-15 11:43:52 UTC
Comment on attachment 196608 [details] [review]
pulse: New pulseaudiosink element to handle format changes

Whoops, duplicate attachment.
Comment 15 Arun Raghavan 2011-09-19 03:26:58 UTC
commit 8ca420f547119f0a5a4a35eb3675f0d2f6b7c085
Author: Arun Raghavan <arun.raghavan@collabora.co.uk>
Date:   Tue Mar 29 12:09:18 2011 +0530

    pulse: New pulseaudiosink element to handle format changes
    
    This introduces a new bin which wraps around pulsesink and depending on
    the formats supported by the sink, plugs in/out a decodebin2 as
    required. This allows users to switch sinks on the stream and adapts
    accordingly (for example, you could watch a movie in passthrough mode on
    your receiver which supports AC3 decode, then plug out and switch to a
    non-digital profile to continue uninterrupted on analog output).
    
    The bin is required because doing the same with playbin2/playsink will
    require API changes that cannot be made in 0.10. With 0.11/1.0, we
    should be able to ask for upstream caps renegotiation to deal with all
    this.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=657179