From: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
To: "Pali Rohár" <pali.rohar@gmail.com>
Cc: "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>
Subject: Re: [PATCH] btmon: Parse information about A2DP codecs: FastStream, aptX Low Latency and aptX HD
Date: Sat, 22 Dec 2018 20:04:21 -0300 [thread overview]
Message-ID: <CABBYNZ+ntTPASbv4434zJf7q_KRtiGbduSORbhNoC__ihZW-cA@mail.gmail.com> (raw)
In-Reply-To: <20181219211735.17563-1-pali.rohar@gmail.com>
Hi Pali,
On Wed, Dec 19, 2018 at 7:17 PM Pali Rohár <pali.rohar@gmail.com> wrote:
>
> ---
The commit message should be formated to fit in 80 columns including
the spaces/tabs added by the likes of git shortlog, you can probably
move part of the subject like to the actual description.
> monitor/a2dp.c | 230 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 230 insertions(+)
>
> diff --git a/monitor/a2dp.c b/monitor/a2dp.c
> index 6a271217b..6e82500d4 100644
> --- a/monitor/a2dp.c
> +++ b/monitor/a2dp.c
> @@ -3,6 +3,7 @@
> * BlueZ - Bluetooth protocol stack for Linux
> *
> * Copyright (C) 2015 Andrzej Kaczmarek <andrzej.kaczmarek@codecoup.pl>
> + * Copyright (C) 2018 Pali Rohár <pali.rohar@gmail.com>
> *
> *
> * This library is free software; you can redistribute it and/or
> @@ -51,6 +52,12 @@
> /* Vendor Specific A2DP Codecs */
> #define APTX_VENDOR_ID 0x0000004f
> #define APTX_CODEC_ID 0x0001
> +#define FASTSTREAM_VENDOR_ID 0x0000000a
> +#define FASTSTREAM_CODEC_ID 0x0001
> +#define APTX_LL_VENDOR_ID 0x0000000a
> +#define APTX_LL_CODEC_ID 0x0002
> +#define APTX_HD_VENDOR_ID 0x000000D7
> +#define APTX_HD_CODEC_ID 0x0024
> #define LDAC_VENDOR_ID 0x0000012d
> #define LDAC_CODEC_ID 0x00aa
>
> @@ -187,6 +194,23 @@ static const struct bit_desc aptx_channel_mode_table[] = {
> { }
> };
>
> +static const struct bit_desc faststream_direction_table[] = {
> + { 0, "Sink" },
> + { 1, "Source" },
> + { }
> +};
> +
> +static const struct bit_desc faststream_sink_frequency_table[] = {
> + { 1, "44100" },
> + { 0, "48000" },
> + { }
> +};
> +
> +static const struct bit_desc faststream_source_frequency_table[] = {
> + { 5, "16000" },
> + { }
> +};
> +
> static void print_value_bits(uint8_t indent, uint32_t value,
> const struct bit_desc *table)
> {
> @@ -215,6 +239,12 @@ static const char *vndcodec2str(uint32_t vendor_id, uint16_t codec_id)
> {
> if (vendor_id == APTX_VENDOR_ID && codec_id == APTX_CODEC_ID)
> return "aptX";
> + else if (vendor_id == FASTSTREAM_VENDOR_ID && codec_id == FASTSTREAM_CODEC_ID)
> + return "FastStream";
> + else if (vendor_id == APTX_LL_VENDOR_ID && codec_id == APTX_LL_CODEC_ID)
> + return "aptX Low Latency";
> + else if (vendor_id == APTX_HD_VENDOR_ID && codec_id == APTX_HD_CODEC_ID)
> + return "aptX HD";
> else if (vendor_id == LDAC_VENDOR_ID && codec_id == LDAC_CODEC_ID)
> return "LDAC";
I wonder if it wouldn't be better to have a table with these as well,
so we define a struct with vendor id, codec id and string name.
> @@ -508,6 +538,98 @@ static bool codec_vendor_aptx_cap(uint8_t losc, struct l2cap_frame *frame)
> return true;
> }
>
> +static bool codec_vendor_faststream_cap(uint8_t losc, struct l2cap_frame *frame)
> +{
> + uint8_t cap = 0;
> +
> + if (losc != 2)
> + return false;
> +
> + l2cap_frame_get_u8(frame, &cap);
> +
> + print_field("%*cDirection: 0x%02x", BASE_INDENT + 2, ' ', cap);
> + print_value_bits(BASE_INDENT + 2, cap, faststream_direction_table);
> +
> + l2cap_frame_get_u8(frame, &cap);
> +
> + print_field("%*cSink Frequency: 0x%02x", BASE_INDENT + 2, ' ', cap & 0x0f);
> + print_value_bits(BASE_INDENT + 2, cap & 0x0f, faststream_sink_frequency_table);
> +
> + print_field("%*cSource Frequency: 0x%02x", BASE_INDENT + 2, ' ', cap & 0xf0);
> + print_value_bits(BASE_INDENT + 2, cap & 0xf0, faststream_source_frequency_table);
> +
> + return true;
> +}
> +
> +static bool codec_vendor_aptx_ll_cap(uint8_t losc, struct l2cap_frame *frame)
> +{
> + uint8_t cap = 0;
> + uint16_t level = 0;
> +
> + if (losc != 2 && losc != 11)
> + return false;
> +
> + l2cap_frame_get_u8(frame, &cap);
> +
> + print_field("%*cFrequency: 0x%02x", BASE_INDENT + 2, ' ', cap & 0xf0);
> + print_value_bits(BASE_INDENT + 2, cap & 0xf0, aptx_frequency_table);
> +
> + print_field("%*cChannel Mode: 0x%02x", BASE_INDENT + 2, ' ',
> + cap & 0x0f);
> + print_value_bits(BASE_INDENT + 2, cap & 0x0f, aptx_channel_mode_table);
> +
> + l2cap_frame_get_u8(frame, &cap);
> +
> + print_field("%*cBidirectional link: %s", BASE_INDENT, ' ', (cap & 1) ? "Yes" : "No");
> +
> + if ((cap & 2) && losc == 11) {
> + /* reserved */
> + l2cap_frame_get_u8(frame, &cap);
> +
> + l2cap_frame_get_le16(frame, &level);
> + print_field("%*cTarget codec buffer level: %u (0x%02x)", BASE_INDENT + 2, ' ', level, level);
> +
> + l2cap_frame_get_le16(frame, &level);
> + print_field("%*cInitial codec buffer level: %u (0x%02x)", BASE_INDENT + 2, ' ', level, level);
> +
> + l2cap_frame_get_u8(frame, &cap);
> + print_field("%*cSRA max rate: %g (0x%02x)", BASE_INDENT + 2, ' ', cap / 10000.0, cap);
> +
> + l2cap_frame_get_u8(frame, &cap);
> + print_field("%*cSRA averaging time: %us (0x%02x)", BASE_INDENT + 2, ' ', cap, cap);
> +
> + l2cap_frame_get_le16(frame, &level);
> + print_field("%*cGood working codec buffer level: %u (0x%02x)", BASE_INDENT + 2, ' ', level, level);
I suspect many of these lines are above 80 columns, please fix them.
> + }
> +
> + return true;
> +}
> +
> +static bool codec_vendor_aptx_hd_cap(uint8_t losc, struct l2cap_frame *frame)
> +{
> + uint8_t cap = 0;
> +
> + if (losc != 5)
> + return false;
> +
> + l2cap_frame_get_u8(frame, &cap);
> +
> + print_field("%*cFrequency: 0x%02x", BASE_INDENT + 2, ' ', cap & 0xf0);
> + print_value_bits(BASE_INDENT + 2, cap & 0xf0, aptx_frequency_table);
> +
> + print_field("%*cChannel Mode: 0x%02x", BASE_INDENT + 2, ' ',
> + cap & 0x0f);
> + print_value_bits(BASE_INDENT + 2, cap & 0x0f, aptx_channel_mode_table);
> +
> + /* reserved */
> + l2cap_frame_get_u8(frame, &cap);
> + l2cap_frame_get_u8(frame, &cap);
> + l2cap_frame_get_u8(frame, &cap);
> + l2cap_frame_get_u8(frame, &cap);
> +
> + return true;
> +}
> +
> static bool codec_vendor_ldac(uint8_t losc, struct l2cap_frame *frame)
> {
> uint16_t cap = 0;
> @@ -543,6 +665,12 @@ static bool codec_vendor_cap(uint8_t losc, struct l2cap_frame *frame)
>
> if (vendor_id == APTX_VENDOR_ID && codec_id == APTX_CODEC_ID)
> return codec_vendor_aptx_cap(losc, frame);
> + else if (vendor_id == FASTSTREAM_VENDOR_ID && codec_id == FASTSTREAM_CODEC_ID)
> + return codec_vendor_faststream_cap(losc, frame);
> + else if (vendor_id == APTX_LL_VENDOR_ID && codec_id == APTX_LL_CODEC_ID)
> + return codec_vendor_aptx_ll_cap(losc, frame);
> + else if (vendor_id == APTX_HD_VENDOR_ID && codec_id == APTX_HD_CODEC_ID)
> + return codec_vendor_aptx_hd_cap(losc, frame);
> else if (vendor_id == LDAC_VENDOR_ID && codec_id == LDAC_CODEC_ID)
> return codec_vendor_ldac(losc, frame);
If we go ahead with the idea of defining a table of vendor codecs the
struct could also define a function callback which parses these so one
just have to include new entries to table.
> @@ -572,6 +700,102 @@ static bool codec_vendor_aptx_cfg(uint8_t losc, struct l2cap_frame *frame)
> return true;
> }
>
> +static bool codec_vendor_faststream_cfg(uint8_t losc, struct l2cap_frame *frame)
> +{
> + uint8_t cap = 0;
> +
> + if (losc != 2)
> + return false;
> +
> + l2cap_frame_get_u8(frame, &cap);
> +
> + print_field("%*cDirection: %s (0x%02x)", BASE_INDENT + 2, ' ',
> + find_value_bit(cap, faststream_direction_table),
> + cap);
> +
> + l2cap_frame_get_u8(frame, &cap);
> +
> + print_field("%*cSink Frequency: %s (0x%02x)", BASE_INDENT + 2, ' ',
> + find_value_bit(cap & 0x0f, faststream_sink_frequency_table),
> + cap & 0x0f);
> +
> + print_field("%*cSource Frequency: %s (0x%02x)", BASE_INDENT + 2, ' ',
> + find_value_bit(cap & 0xf0, faststream_source_frequency_table),
> + cap & 0xf0);
> +
> + return true;
> +}
> +
> +static bool codec_vendor_aptx_ll_cfg(uint8_t losc, struct l2cap_frame *frame)
> +{
> + uint8_t cap = 0;
> + uint16_t level = 0;
> +
> + if (losc != 2 && losc != 11)
> + return false;
> +
> + l2cap_frame_get_u8(frame, &cap);
> +
> + print_field("%*cFrequency: %s (0x%02x)", BASE_INDENT + 2, ' ',
> + find_value_bit(cap & 0xf0, aptx_frequency_table),
> + cap & 0xf0);
> +
> + print_field("%*cChannel Mode: %s (0x%02x)", BASE_INDENT + 2, ' ',
> + find_value_bit(cap & 0x0f, aptx_channel_mode_table),
> + cap & 0x0f);
> +
> + l2cap_frame_get_u8(frame, &cap);
> +
> + print_field("%*cBidirectional link: %s", BASE_INDENT, ' ', (cap & 1) ? "Yes" : "No");
> +
> + if ((cap & 2) && losc == 11) {
> + l2cap_frame_get_u8(frame, &cap);
> +
> + l2cap_frame_get_le16(frame, &level);
> + print_field("%*cTarget codec buffer level: %u (0x%02x)", BASE_INDENT + 2, ' ', level, level);
> +
> + l2cap_frame_get_le16(frame, &level);
> + print_field("%*cInitial codec buffer level: %u (0x%02x)", BASE_INDENT + 2, ' ', level, level);
> +
> + l2cap_frame_get_u8(frame, &cap);
> + print_field("%*cSRA max rate: %g (0x%02x)", BASE_INDENT + 2, ' ', cap / 10000.0, cap);
> +
> + l2cap_frame_get_u8(frame, &cap);
> + print_field("%*cSRA averaging time: %us (0x%02x)", BASE_INDENT + 2, ' ', cap, cap);
> +
> + l2cap_frame_get_le16(frame, &level);
> + print_field("%*cGood working codec buffer level: %u (0x%02x)", BASE_INDENT + 2, ' ', level, level);
> + }
> +
> + return true;
> +}
> +
> +static bool codec_vendor_aptx_hd_cfg(uint8_t losc, struct l2cap_frame *frame)
> +{
> + uint8_t cap = 0;
> +
> + if (losc != 5)
> + return false;
> +
> + l2cap_frame_get_u8(frame, &cap);
> +
> + print_field("%*cFrequency: %s (0x%02x)", BASE_INDENT + 2, ' ',
> + find_value_bit(cap & 0xf0, aptx_frequency_table),
> + cap & 0xf0);
> +
> + print_field("%*cChannel Mode: %s (0x%02x)", BASE_INDENT + 2, ' ',
> + find_value_bit(cap & 0x0f, aptx_channel_mode_table),
> + cap & 0x0f);
> +
> + /* reserved */
> + l2cap_frame_get_u8(frame, &cap);
> + l2cap_frame_get_u8(frame, &cap);
> + l2cap_frame_get_u8(frame, &cap);
> + l2cap_frame_get_u8(frame, &cap);
> +
> + return true;
> +}
> +
> static bool codec_vendor_cfg(uint8_t losc, struct l2cap_frame *frame)
> {
> uint32_t vendor_id = 0;
> @@ -593,6 +817,12 @@ static bool codec_vendor_cfg(uint8_t losc, struct l2cap_frame *frame)
>
> if (vendor_id == APTX_VENDOR_ID && codec_id == APTX_CODEC_ID)
> return codec_vendor_aptx_cfg(losc, frame);
> + else if (vendor_id == FASTSTREAM_VENDOR_ID && codec_id == FASTSTREAM_CODEC_ID)
> + return codec_vendor_faststream_cfg(losc, frame);
> + else if (vendor_id == APTX_LL_VENDOR_ID && codec_id == APTX_LL_CODEC_ID)
> + return codec_vendor_aptx_ll_cfg(losc, frame);
> + else if (vendor_id == APTX_HD_VENDOR_ID && codec_id == APTX_HD_CODEC_ID)
> + return codec_vendor_aptx_hd_cfg(losc, frame);
> else if (vendor_id == LDAC_VENDOR_ID && codec_id == LDAC_CODEC_ID)
> return codec_vendor_ldac(losc, frame);
Ditto.
>
> --
> 2.11.0
>
--
Luiz Augusto von Dentz
next prev parent reply other threads:[~2018-12-22 23:04 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-12-19 21:17 [PATCH] btmon: Parse information about A2DP codecs: FastStream, aptX Low Latency and aptX HD Pali Rohár
2018-12-22 23:04 ` Luiz Augusto von Dentz [this message]
2018-12-23 10:00 ` [PATCH v2] btmon: Parse new A2DP codecs Pali Rohár
2018-12-28 18:24 ` Luiz Augusto von Dentz
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CABBYNZ+ntTPASbv4434zJf7q_KRtiGbduSORbhNoC__ihZW-cA@mail.gmail.com \
--to=luiz.dentz@gmail.com \
--cc=linux-bluetooth@vger.kernel.org \
--cc=pali.rohar@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).