[pulseaudio-discuss] [WIP] Passthrough support

[Arun Raghavan]

On Sat, 2011-03-12 at 15:08 -0600, pl bossart wrote:
> Hi Arun,
> I looked at the passthrough branch. Looks pretty good. I wrote down some notes:
> 
> gstaudioiec61937:
> - need to check that frame is valid and synchronized on AC3 header
> before pushing iec payload

I'm expecting valid data from the parser - don't think it makes sense to
duplicate the code from the parser. Now that you mention it though, I'm
only using "framed=true" for the incoming caps, and this might not
actually force the parser to be plugged. Will look into this.

> - no code for mp3 and dts?

DTS is present now. MP3 should be there tomorrow.

> pulsesink:
> - do we really need two steps for payload and commit? Make
> things complicated and creates memory copies

As the comment there says, we can avoid the duplicate copy by doing the
alloc in the payloader. The base class for pulsesink works at sample
granularity (and not byte granularity), so it became necessary to do the
payloading near the beginning of the base class render function.

> - commit needs to prevent toy resampler (maybe this means the sink
> cannot work iwth a live source since no adjustment is possible.

Must've dropped your check somehow. Will put it back.

> -  spec.latency_time = GST_BASE_AUDIO_SINK (psink)->latency_time;
>   if (!gst_ring_buffer_parse_caps (&spec, caps))
>     goto out;
> 
> Does this work? spec is not initialized?

Yes, _parse_caps() fills the spec.

> - change of routing?

Working on this.

> - formats
> /* FIXME: Eventually, we want the list of supported formats to be set
> +         * as properties by the GUI based on what the user says is supported by
> +         * the receiver */
> +        /* FIXME: How do we figure out supported rates? :( */
> +        unsigned int i, rates[] = { 32000, 44100, 48000 };
> 
> For HDMI and SPDIF, 32,44.1 and 48 are mandatory. I think you can
> query the alsa layer
> to give a range of supported frequencies since this is known in
> the driver (either hard-coded or obtained with EDID/eld). I remember
> seeing some ALSA patches on this for the NVIDIA cards.

Great, I'll look this up.

> sink.c
> - how do we notify client that volume is disabled?

Tanu's started some work to make this possible. Will coordinate with him
and reuse the work already being done there.

> I wasn't too clear on the BT support. You mentioned that it was a
> different branch and that it stilll nneds my patches to gst-ugly. We'd
> need to have the payloader code in gsraudioiec61937 instead? I am also
> not sure how we handle the transition from A2DP w/ SBC to A2DP w/ MP3.
> In the current code for iec958, there's no need to reconfigure the
> link.
> 
> Last, I had 3 conflicts with git master that I had to solve by hand.

That's odd. I'll do a rebase and push tomorrow.

Thanks for the review!
Arun