[pulseaudio-discuss] [PATCHv2 31/60] bluetooth: Implement org.bluez.MediaEndpoint1.SelectConfiguration()

[João Paulo Rechi Vita]

On Wed, Sep 4, 2013 at 5:18 AM, Tanu Kaskinen
<tanu.kaskinen at linux.intel.com> wrote:
> On Tue, 2013-09-03 at 21:18 -0300, João Paulo Rechi Vita wrote:
>> On Sun, Aug 18, 2013 at 6:37 AM, Tanu Kaskinen
>> <tanu.kaskinen at linux.intel.com> wrote:
>> > On Tue, 2013-08-13 at 01:54 -0300, jprvita at gmail.com wrote:


>> >> +        pa_log_error("Endpoint SelectConfiguration(): %s", err.message);
>> >> +        dbus_error_free(&err);
>> >> +        goto fail;
>> >> +    }
>> >> +
>> >> +    if (size != sizeof(config)) {
>> >> +        pa_log_error("Capabilities array has invalid size");
>> >> +        goto fail;
>> >> +    }
>> >>
>> >> +    pa_zero(config);
>> >> +
>> >> +    /* Find the lowest freq that is at least as high as the requested sampling rate */
>> >> +    for (i = 0; (unsigned) i < PA_ELEMENTSOF(freq_table); i++)
>> >> +        if (freq_table[i].rate >= y->core->default_sample_spec.rate && (cap->frequency & freq_table[i].cap)) {
>> >> +            config.frequency = freq_table[i].cap;
>> >> +            break;
>> >> +        }
>> >> +
>> >> +    if ((unsigned) i == PA_ELEMENTSOF(freq_table)) {
>> >> +        for (--i; i >= 0; i--) {
>> >> +            if (cap->frequency & freq_table[i].cap) {
>> >> +                config.frequency = freq_table[i].cap;
>> >> +                break;
>> >> +            }
>> >> +        }
>> >> +
>> >> +        if (i < 0) {
>> >> +            pa_log_error("Not suitable sample rate");
>> >> +            goto fail;
>> >> +        }
>> >> +    }
>> >> +
>> >> +    pa_assert((unsigned) i < PA_ELEMENTSOF(freq_table));
>> >> +
>> >> +    if (y->core->default_sample_spec.channels <= 1) {
>> >> +        if (cap->channel_mode & SBC_CHANNEL_MODE_MONO)
>> >> +            config.channel_mode = SBC_CHANNEL_MODE_MONO;
>> >> +    }
>> >> +
>> >> +    if (y->core->default_sample_spec.channels >= 2) {
>> >> +        if (cap->channel_mode & SBC_CHANNEL_MODE_JOINT_STEREO)
>> >> +            config.channel_mode = SBC_CHANNEL_MODE_JOINT_STEREO;
>> >> +        else if (cap->channel_mode & SBC_CHANNEL_MODE_STEREO)
>> >> +            config.channel_mode = SBC_CHANNEL_MODE_STEREO;
>> >> +        else if (cap->channel_mode & SBC_CHANNEL_MODE_DUAL_CHANNEL)
>> >> +            config.channel_mode = SBC_CHANNEL_MODE_DUAL_CHANNEL;
>> >> +        else if (cap->channel_mode & SBC_CHANNEL_MODE_MONO) {
>> >> +            config.channel_mode = SBC_CHANNEL_MODE_MONO;
>> >> +        } else {
>> >> +            pa_log_error("No supported channel modes");
>> >> +            goto fail;
>> >> +        }
>> >> +    }
>> >
>> > You didn't address my earlier complaint: "If
>> > default_sample_spec.channels is 1, but cap doesn't contain
>> > SBC_CHANNEL_MODE_MONO, then config.channel_mode is left uninitialized
>> > (well, it's initialized to zero in the beginning). I believe this is not
>> > what you intended."
>> >
>>
>> Yes, I think it is better to return an error to the caller here.
>
> There's no need to return an error, we should allow any of the
> two-channel modes even if default_sample_spec.channels is 1.
>

Ok.

-- 
João Paulo Rechi Vita
http://about.me/jprvita