Re: [PATCH v13 07/10] bluetooth: Add more variants of SBC codec

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 02.02.20 14:08, Pali Rohár wrote:
On Thursday 23 January 2020 12:29:15 Georg Chini wrote:
On Tuesday 21 January 2020 16:17:16 Georg Chini wrote:
...
+}
+
+static bool can_accept_capabilities_xq2(const uint8_t *capabilities_buffer, uint8_t capabilities_size, bool for_encoding) {
+    return can_accept_capabilities_table(capabilities_buffer, capabilities_size, sbc_xq2_caps_table, PA_ELEMENTSOF(sbc_xq2_caps_table));
+}
+
+static const char *choose_remote_endpoint_table(const pa_hashmap *capabilities_hashmap, const pa_sample_spec *default_sample_spec, const a2dp_sbc_t capabilities_table[], unsigned capabilities_table_elements) {
        const pa_a2dp_codec_capabilities *a2dp_capabilities;
+    const a2dp_sbc_t *capabilities;
        const char *key;
        void *state;
+    unsigned i;
+    uint8_t table_range;
+    uint8_t current_range;
+    const char *best_key = NULL;
+    uint8_t best_range = 0;
+    uint8_t frequency = 0;
+    bool is_mono = (default_sample_spec->channels <= 1);
+
+    static const struct {
+        uint32_t rate;
+        uint8_t cap;
+    } freq_table[] = {
+        { 16000U, SBC_SAMPLING_FREQ_16000 },
+        { 32000U, SBC_SAMPLING_FREQ_32000 },
+        { 44100U, SBC_SAMPLING_FREQ_44100 },
+        { 48000U, SBC_SAMPLING_FREQ_48000 }
+    };
+
+    for (i = 0; i < PA_ELEMENTSOF(freq_table); i++) {
+        if (freq_table[i].rate == default_sample_spec->rate) {
Is an exact match necessary here? Or would >= be OK as well?
For fixed SBC profiles we need to check for exact frequency. As there
are specific bitpool values for specific frequency.
Sure, in the end we will use one of the specific frequencies.
But need the default_sample_spec already contain the exact frequency?
Or can we choose the one nearest to default_sample_spec->rate?
Here we compare SBC freq capability with pulseaudio internal frequency.
So non-exact nearest frequency could be used too.
 I think you do that for the other codecs as well.
+
+        table_range = capabilities_table[i].max_bitpool - capabilities_table[i].min_bitpool;
+
+        /* use current remote capabilities if its bitpool range is closer to bitpool range in table */
+        if (!best_key || abs((int)current_range - (int)(table_range)) < abs((int)best_range - (int)(table_range))) {
+            best_range = current_range;
+            best_key = key;
+        }
Does that best_key evaluation really make sense? current_range and
table_range will both be 0 in all
cases except for the sbc_auto_caps case and in that case there is only one
possible element in the table.
I written this code prior to defining final list of SBC profiles. So
code was prepared to work with any list of SBC definitions.

Maybe you are right that for currently defined SBC profiles it is not
needed, but I wanted that current code would work with any SBC profile
table definition.
I don't think that makes sense. In are_configs_compatible() you rely on the
fact that the table entries only contain a single bitpool value.
That is because all currently defined profiles (except auto) have fixed
bitpool value.
Do you intend to add any other profile which would require a variable bitpool?
If you don't have any use case, please drop it.

_______________________________________________
pulseaudio-discuss mailing list
pulseaudio-discuss@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss




[Index of Archives]     [Linux Audio Users]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux