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
