[pulseaudio-discuss] [PATCH] raop: Don't use incompatible default sample spec parameters

[Felipe Sateler]

On Sun, Aug 24, 2014 at 5:09 AM, Tanu Kaskinen
<tanu.kaskinen at linux.intel.com> wrote:
> On Tue, 2014-08-19 at 12:37 +0100, Colin Guthrie wrote:
>> Tanu Kaskinen wrote on 19/08/14 12:12:
>> > On Tue, 2014-08-19 at 14:08 +0300, Tanu Kaskinen wrote:
>> >> Since the RAOP sink supports only some formats and channel counts, we
>> >> shouldn't blindly use pa_core.default_sample_spec. This patch changes
>> >> things so that we default to PA_SAMPLE_S16NE and 2 channels, and only
>> >> take the sample rate from pa_core.default_sample_spec.
>> >> ---
>> >>  src/modules/raop/module-raop-sink.c | 8 +++++---
>> >>  1 file changed, 5 insertions(+), 3 deletions(-)
>> >>
>> >> diff --git a/src/modules/raop/module-raop-sink.c b/src/modules/raop/module-raop-sink.c
>> >> index 2d08645..13036d9 100644
>> >> --- a/src/modules/raop/module-raop-sink.c
>> >> +++ b/src/modules/raop/module-raop-sink.c
>> >> @@ -518,15 +518,17 @@ int pa__init(pa_module*m) {
>> >>          goto fail;
>> >>      }
>> >>
>> >> -    ss = m->core->default_sample_spec;
>> >> +    ss.format = PA_SAMPLE_S16NE;
>> >> +    ss.channels = 2;
>> >> +    ss.rate = m->core->default_sample_spec.rate;
>> >>      if (pa_modargs_get_sample_spec(ma, &ss) < 0) {
>> >>          pa_log("invalid sample format specification");
>> >>          goto fail;
>> >>      }
>> >>
>> >> -    if ((/*ss.format != PA_SAMPLE_U8 &&*/ ss.format != PA_SAMPLE_S16NE) ||
>> >> +    if ((ss.format != PA_SAMPLE_S16NE) ||
>> >
>> > Colin, as the author of the original code, do you remember why
>> > "ss.format != PA_SAMPLE_U8" was commented out?
>>
>> Hmm, not really :(
>>
>> I guess it was probably a trial and error thing - perhaps I couldn't get
>> it to work with the U8 sample sizes due to the device limitations... I
>> cannot really say for sure as memory is definitely fuzzy here :( Patch
>> looks sensible overall tho'.
>
> I interpret the last sentence so that the patch has passed review, so I
> pushed it now.
>
> Balint, this should make the Debian patch unnecessary ("raop: Use two
> channels by deafult in raop-discovery").

Thanks, we will update the patch for the next upload.

-- 

Saludos,
Felipe Sateler