[pulseaudio-discuss] {PATCH][RFC] AC3 passthrough support

[Tanu Kaskinen]

On Fri, 2010-07-09 at 10:51 -0500, pl bossart wrote:
> > I agree that it's not really a good solution to require the user to
> > change the card profile manually to enable passthrough via spdif. But as
> > I said, a separate module could take care switching between the profiles
> > as needed.
> 
> I don't understand your proposal. What do you mean by separate
> 'module' and how would this work with the current mixer profiles?

PA_CORE_HOOK_SINK_INPUT_NEW is fired at the beginning of
pa_sink_input_new(). A module can connect to that hook and detect that
"ah, data->flags contains PA_SINK_INPUT_PASSTHROUGH, and data->sink is
NULL, I'll make sure that the passthrough sink is available".

Configuring (automatically) such module would need some extensions to
the card profile code so that this new module could figure out what
needs to be done in order to "make sure that the passthrough sink is
available". My feeling is/was that it's probably doable without too much
difficulty, and that's about as far as I thought about it.

Now, what if data->sink is not NULL? That means that when the client
created the stream, the correct profile was already activated. Or,
alternatively, data->sink points to some "ghost" of a currently
non-existing sink that can, however, be "made real" automatically
somehow. This latter scenario refers to functionality that doesn't
currently exist. Implementing such functionality would require big
changes to a lot of stuff.

I'm starting to like your approach more and more.

> > Having thought this for a while, the only thing that really irritates me
> > in your design is that the semantics of the PA_SINK_PASSTHROUGH become
> > messy: the flag says that "in addition to normal PCM streams, this sink
> > supports also passthrough streams, except at times when there already
> > exist PCM streams, and btw, while a passthrough stream is playing, PCM
> > streams cease to be supported". I guess that's not a very practical
> > problem, though.
> 
> It's not really my design, it's just how SPDIF/HDMI work...

Well, when defining the semantics of PA_SINK_PASSTHROUGH you're free to
choose between "combined PCM and passthrough" or "passthrough only".
Either can be made to work with spdif/hdmi. And I think it's not really
specific to spdif/hdmi - both semantics can be used with any passthrough
sink. For example, using your semantics, an A2DP sink can switch between
real-time SBC encoding and passthrough MP3 on the fly similarly how an
spdif sink can change between PCM and passthrough operation. The only
case where the "combined" semantics wouldn't work is when the sink just
plain can't work in any other mode than passthrough. I think it's quite
improbable that we'll ever see such sink.

> > Ah, but now a better argument popped into my mind. If a sink can enter
> > the passthrough mode on the fly, then that has a consequence that
> > affects any application that is interested in sink volumes: whether a
> > sink has adjustable volume can change any time. That's not very nice.
> 
> Duh? If we fixed the volume stuff so that that the volume control app
> disables its sliders when passthrough AC3 is enabled, what would be
> the issue. This is how VLC works, when you stream AC3 data the volume
> control is no longer enabled.

What I had in mind was that if changing between PCM and passthrough
modes would require changing profiles, then clients wouldn't need to be
prepared to disable and enable volume control for a sink on the fly.
They would only need to check whether a sink has volume when they
initially create the widget for the sink.

If volume can be disabled on the fly, that pushes more complexity to the
clients, which is not good. But actually I think we'll have to do this,
since the profile-switching module seems impractical.

It seems like all my ideas were crap :)

> > Even if we have only two formats, iec958 and mp3, I don't see why the
> > sink flags would the proper place to specify the format. I believe most
> > of the passthrough related code is only interested in the fact that the
> > stream data shall not be touched. If there are multiple flags for all
> > the different passthrough formats, then all that code will have to check
> > for two flags instead of one.
> 
> I guess we could have a two-step process, where you first detect if
> the sink and sink-input have support for PASSTHROUGH, and then in a
> second step you verify that the 'subtype' (iec958 or mp3) is
> compatible. The logic for rejecting connections is the same... You
> only need one last step to make sure types are compatible.
> 
> > There would of course be a standard implementation for sinks that don't
> > do any "strange" stuff. I thought individual sink implementations could
> > choose to amend the standard logic, but actually I'm not sure how it
> > could be done without duplicating code.
> >
> > Giving it a second thought, the "can this sink input be connected to
> > this sink" question can be thought equally well as a method of sinks or
> > sink inputs, or even as a plain old independent function. So, doing it
> > like you do it in your patch is fine by me (just don't use "iec958"
> > where the logic isn't really bound to the exact format).
> 
> As I said above, we can have a first pass implemented in the core that
> checks things are fine, then a second pass implemented by sinks who
> support passthrough mode to check fine-grained compatibility.

I think I'd just put a new variable in pa_sink and pa_sink_input (or
maybe pa_sample_spec?) that would store the codec information, and do
all checking in the core in one place.

> >> > The NOVOLUME flags can be exported to clients, so they can choose to not
> >> > show any volume controls. Legacy clients should get the
> >> > PA_ERR_NOTSUPPORTED error when trying to change the volume of streams or
> >> > sinks that have the NOVOLUME flag.
> >>
> >> Yes the volume needs more work. I don't really understand all this
> >> code through, so for now I disabled the set-volume() routines without
> >> notifying the client. You end-up in the situation where pavucontrol
> >> can change the volume but nothing happens, this is not user-friendly.
> >> Is this NOVOLUME flag an existing one, or something that would need to be added?
> >
> > It needs to be added.
> 
> There's already a detection in the mixer code that the hw_volume is
> available or not. Maybe we can use this as well.

No, I don't think so. HW volume is a different thing. If a sink doesn't
have HW volume, then it will have SW volume. What we want to tell the
clients is that "this sink or this stream doesn't have volume at all". I
think a new flag for that is a fine solution.

I'm not sure if everyone agrees that the flags should change on the fly,
though, which will happen if a sink can disable volume whenever it
wants. An alternative is to put a new boolean variable to the
pa_sink_info struct. I don't care which solution is chosen.

At server side the situation is the same - shall we add a NOVOLUME flag
to sinks and make it possible to change the flags on the fly, or shall
we add a new "has_volume" boolean variable to pa_sink.

-- 
Tanu Kaskinen