[pulseaudio-discuss] review+pull-request: Passthrough support

[Colin Guthrie]

'Twas brillig, and Arun Raghavan at 13/04/11 13:53 did gyre and gimble:
> Hey folks,
> My passthrough work is now at a point where I think it's good to pull to
> master. All the core code for clients to signal that they are providing
> a compressed format, and sinks to signal what formats they support is
> there. The API is essentially the same as discussed earlier on-list,
> with some small tweaks along the way as necessary.
> 
> Still pending are proper detection/signalling of available formats on
> ALSA devices is to be done (Colin's looking at some of the plumbing for
> S/PDIF, and HDMI ELD/EDID-based lookups are something that we need as
> well), and better handling of monitors (we suspend them for now, which
> works), and volumes (Tanu's got some on-going work on this).
> 
> NOTE: given the above about ALSA, topmost patch should probably not be
> pulled, but is handy to have for testing.
> 
> In addition to the passthrough branch, there is a passthrough-bt branch
> which has changes to the bluetooth sink to support streaming MP3 to
> headsets that support it. This also needs some work before being ready
> to pull.
> 
> The changes needed to actually use this in GStreamer are also done, but
> not upstream yet. I need to do some rebasing to make this stuff good to
> push out, so details on this in a following mail.
> 
> Comments, feedback will be very much appreciated. :)


> are available in the git repository at:
>   git://git.collabora.co.uk/git/user/arun/pulseaudio.git passthrough


Not tried the code (yet), but will do this week. I have reviewed all the
code yet!

But I did review all the commits! Most of my comments are trivial tho'
(and some are already fixed it seems!)


In 4a839b3767ae5571c69bd591b5a59d7307cba13e

pa_format_info_to_sample_spec() and pa_format_info_to_sample_spec_fake()
some pa_assert's should pa_assert_se's. (Update: I think these are all
rectified in 38309769c5a630ee78ae73be6fb407624c1509fc)


In 7e8b65bc4f490628f20ab420a6c80bfa3a760bc6

Shouldn't pa_create_stream_callback() call
pa_tagstruct_get_format_info() up to n_format times and only bail if all
are invalid?

Minor typo: "Send back the format me negotiated" s/me/we/

Minor typo: "We're working not working with the extended API" s/working
not/not/

pa_sink_input_new_data_set_formats(): Please drop the last "else" and
just put the "return TRUE" as the last statement. It's trivial but I
believe this looks neater to always have a return at the top level of
the function.


In d885a39b9f1085e759ad69afcc939caffb1fbc5a

Minor typo: "but this is can very well be incorrect" s/this is/this/


In e193c2bf55326a48e2297bcacadc9d1848a40d7d and
948d0f19bef353208ffb5b1b8c520b6b489b94a6

Can you make sure that pactl and pacmd stay as in-sync as possible?


In bb7cc499f1815de1c90b0ef1850152224df96ff9

I don't see why this asserts in the current form nor what has actually
changed.


In 9d5e8867066e392fa40add0f0380374131dfc2de

Minor: pa_streq has a space before the (.


In d9e0f3bc0286249ced30a4728e4467f7a46f37af

Already merged in 837e0a960630251ce30c124da5e65079b748d978




Should we rebase before merge or is it wise to keep your tree as is for
ease of working with existing checkouts?




General Question:

Has this broken tunnels? (we manage to do this quite often with stream
protocol changes...



That is all!

Take care

Col

-- 

Colin Guthrie
gmane(at)colin.guthr.ie
http://colin.guthr.ie/

Day Job:
  Tribalogic Limited [http://www.tribalogic.net/]
Open Source:
  Mageia Contributor [http://www.mageia.org/]
  PulseAudio Hacker [http://www.pulseaudio.org/]
  Trac Hacker [http://trac.edgewall.org/]