GNOME Bugzilla – Bug 657179
pulse: New pulseaudiosink element to handle format changes
Last modified: 2011-09-19 03:26:58 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.
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.
Is the bin specific to pulseaudio? Just wonder if the bin could work with other audiosinks too.
(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.
What should happen with this patch? I've reviewed it a long time ago and it looked good back then
It should go in. But maybe we were waiting for a better name ;)
I think we've settled for "pulseaudiosink" now.
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.
(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.
(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.
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.
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.
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.
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 on attachment 196608 [details] [review] pulse: New pulseaudiosink element to handle format changes Whoops, duplicate attachment.
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