A2DP quality (bluetooth-alsa)

A2DP quality (bluetooth-alsa)

[qduaty]

Hi people,

I found that I cannot set A2DP bitpool to anything more than 53 in my
~/.asoundrc file. The bluetooth driver reports an error and keeps
reporting it (becomes useless) until it is restarted.

Bitpool values as high as 128 are possible on Windows Mobile devices
(including mine), which means they are possible in A2DP. Such a high
value is necessary for some genres of current music, which are highly
compressed (the problem was once discussed in one of Xiph Foundation's
articles). SBC is unable to encode such material properly if it
doesn't have enough bandwidth.

I don't understand Bluetooth. So I'm asking you for an advice:
1. Is it actually possible to set SBC bitpool in linux' A2DP to more than 53?
2. If it is, can I ask for a short success story in order to reproduce it?
2. If not, where should I start (in bluez source code) in order to fix
this problem?


Regards

--
Sebastian

Re: A2DP quality (bluetooth-alsa)

[Luiz Augusto von Dentz]

Hi,

On Mon, Oct 10, 2011 at 1:40 PM, qduaty@gmail.com <qduaty@gmail.com> wrote:
> Hi people,
>
> I found that I cannot set A2DP bitpool to anything more than 53 in my
> ~/.asoundrc file. The bluetooth driver reports an error and keeps
> reporting it (becomes useless) until it is restarted.

It can be 2 things, either a bug or the device maximum bitpool is
actually 53 so we cannot negotiate it.

> Bitpool values as high as 128 are possible on Windows Mobile devices
> (including mine), which means they are possible in A2DP. Such a high
> value is necessary for some genres of current music, which are highly
> compressed (the problem was once discussed in one of Xiph Foundation's
> articles). SBC is unable to encode such material properly if it
> doesn't have enough bandwidth.

PulseAudio uses 64 as bitpool maximum and it seems to work fine, but
that depends on available bandwidth, in fact PA reduces the bitpool if
it detects the audio is skipping.

> I don't understand Bluetooth. So I'm asking you for an advice:
> 1. Is it actually possible to set SBC bitpool in linux' A2DP to more than 53?
> 2. If it is, can I ask for a short success story in order to reproduce it?
> 2. If not, where should I start (in bluez source code) in order to fix
> this problem?

Take a look a PulseAudio or BlueZ source tree (audio/pcm_bluetooth.c)

-- 
Luiz Augusto von Dentz

Re: A2DP quality (bluetooth-alsa)

[Siarhei Siamashka]

On Mon, Oct 10, 2011 at 1:40 PM, qduaty@gmail.com <qduaty@gmail.com> wrote:
> Hi people,
>
> I found that I cannot set A2DP bitpool to anything more than 53 in my
> ~/.asoundrc file. The bluetooth driver reports an error and keeps
> reporting it (becomes useless) until it is restarted.
>
> Bitpool values as high as 128 are possible on Windows Mobile devices
> (including mine), which means they are possible in A2DP.

The SBC codec itself supports bitpool values higher 53, and
sbcenc/sbcdec command line tools can be used to experiment with
encoding/decoding audio data.

> Such a high value is necessary for some genres of current music, which are highly
> compressed (the problem was once discussed in one of Xiph Foundation's
> articles). SBC is unable to encode such material properly if it
> doesn't have enough bandwidth.

Could you please provide a link to the Xiph Foundation's article you
have mentioned? Also what does "SBC is unable to encode such material
properly" mean?

My understanding is that the highly compressed sources are mostly a
problem not for SBC encoder, but actually for decoder:
    http://git.kernel.org/?p=bluetooth/bluez.git;a=commit;h=3df7c3e66042f266dadace88488e532de92d4da9
When the original audio signal is using full range, then going to the
frequency domain, doing lossy compression, and going back to the time
domain may easily result in some sample values exceeding the valid
range. The reference binary SBC decoder seems to be simply clamping
the out-of-range values to bring them into valid range (based on the
output it produces). And since some time ago, bluez SBC decoder also
does the same. This not a perfect solution, because the shape of the
decoded signal abruptly changes to "flat" part at the top or bottom.
But not doing any clamping is a lot worse and letting the samples
overflow results in really bad audible clicks. However it's hard to
say how each particular A2DP headset deals with the out-of-range
samples. Most of them are apparently also doing clamping and the audio
sounds ok. But unfortunately sometimes this is not the case. I happen
to be an owner of Logitech FreePulse A2DP headset, and the
out-of-range samples can be clearly heard as clicking artifacts (in
the worst cases resembling 'frying pan' type of sound). I myself used
a hack by patching bluez SBC to also reduce audio volume as the part
of the encoding process so that the badly compressed audio sources are
less likely to have any out of range samples in the SBC encoded data
stream.

I guess that increasing bitpool would result in a less lossy encoding
with a better approximation of the original audio data. Hence less
likely to differ from the original data too much and go out of range.
But I don't think that it's a "proper" solution for the problem.
Normally, even low bitpool values should be still quite usable.

What kind of A2DP headset are you using yourself?

-- 
Best regards,
Siarhei Siamashka

Re: A2DP quality (bluetooth-alsa)

[qduaty]

Hi,

2011/10/10 Luiz Augusto von Dentz <luiz.dentz@gmail.com>:
> It can be 2 things, either a bug or the device maximum bitpool is
> actually 53 so we cannot negotiate it.

My headset indeed reports bitpool range up to 53. WinMo seems to
either override it, or present some features that cause the Nokia BT
chip to report a wider range. Unfortunately I don't know how to
disable encryption in Windows Mobile to capture its bluetooth session
(and how to capture it with Bluez). Can 'hcidump' do this?

> PulseAudio uses 64 as bitpool maximum and it seems to work fine

Yes, wIth movies, games etc. the quality is excellent.

2011/10/10 Siarhei Siamashka <siarhei.siamashka@gmail.com>:
> On Mon, Oct 10, 2011 at 1:40 PM, qduaty@gmail.com <qduaty@gmail.com> wrote:
> Could you please provide a link to the Xiph Foundation's article you
> have mentioned? Also what does "SBC is unable to encode such material
> properly" mean?

I remember it was their "About" article, but it seems either they
shortened it (removing fragments irrelevant to their mission) or it
has never been there and I've seen it somewhere else. Anyway, the
whole phenomena is called "Loudness war". CD's are recorded with
dynamics compression, so their sound has distortions introduced in
production and high energy in all subbands at the same time. Thus, SBC
must encode distortions as well as higher energy in all subbands.

> The SBC decoder seems to be clamping the out-of-range values [...]
> not doing any clamping is a lot worse and letting the samples
> overflow results in really bad audible clicks.

Maybe sin() would help, who knows? Obviously in the decoder, which
usually cannot be reprogrammed, but future /bluez devices/ could
benefit from this.

> What kind of A2DP headset are you using yourself?

Nokia BT-503. Its quality is enough to notice an obvious difference
between 'computer' bitpool (<=53) and that coming from Windows Mobile
(128 does not cause skipping yet). In presence of dynamics
compression, even two guitars with a vocal can be distinguished only
with higher bitpool. Another example is a symphony - I can hear
individual instruments from WinMo, and a "movie soundtrack" from
Bluez. I don't know how the device deals with clicks, but I never
heard them. Usual (and also annoying) distortion is a 'chirp' in some
bands.


Regards
--
Sebastian Olter

Re: A2DP quality (bluetooth-alsa)

[Siarhei Siamashka]

[-- Attachment #1: Type: text/plain, Size: 5079 bytes --]

On Mon, Oct 10, 2011 at 10:10 PM, qduaty@gmail.com <qduaty@gmail.com> wrote:
> 2011/10/10 Siarhei Siamashka <siarhei.siamashka@gmail.com>:
>> On Mon, Oct 10, 2011 at 1:40 PM, qduaty@gmail.com <qduaty@gmail.com> wrote:
>> Could you please provide a link to the Xiph Foundation's article you
>> have mentioned? Also what does "SBC is unable to encode such material
>> properly" mean?
>
> I remember it was their "About" article, but it seems either they
> shortened it (removing fragments irrelevant to their mission) or it
> has never been there and I've seen it somewhere else. Anyway, the
> whole phenomena is called "Loudness war". CD's are recorded with
> dynamics compression, so their sound has distortions introduced in
> production and high energy in all subbands at the same time. Thus, SBC
> must encode distortions as well as higher energy in all subbands.

Still I would suggest you to try the attached patch just for a test.
It reduces audio volume as part of the sbc encoding process and should
eliminate most of the possible clipping cases on the decoding side.
Some people on Hydrogenaudio forums have investigated the impact of
clipping on the perceived audio quality and found it at least audible:
    http://www.hydrogenaudio.org/forums/index.php?showtopic=82112
    http://en.wikipedia.org/wiki/Clipping_(audio)

>> The SBC decoder seems to be clamping the out-of-range values [...]
>> not doing any clamping is a lot worse and letting the samples
>> overflow results in really bad audible clicks.
>
> Maybe sin() would help, who knows? Obviously in the decoder, which
> usually cannot be reprogrammed, but future /bluez devices/ could
> benefit from this.

The headset is free to do whatever it likes with the samples falling
out of [-0x8000, 0x7FFF] range, like clipping them or adjusting the
volume or anything else. It does not have to produce files with 16-bit
audio samples but just needs to play some sound in the speakers.

>> What kind of A2DP headset are you using yourself?
>
> Nokia BT-503. Its quality is enough to notice an obvious difference
> between 'computer' bitpool (<=53) and that coming from Windows Mobile
> (128 does not cause skipping yet). In presence of dynamics
> compression, even two guitars with a vocal can be distinguished only
> with higher bitpool. Another example is a symphony - I can hear
> individual instruments from WinMo, and a "movie soundtrack" from
> Bluez. I don't know how the device deals with clicks, but I never
> heard them. Usual (and also annoying) distortion is a 'chirp' in some
> bands.

OK, thanks for the information. It looks like this is not the same
issue as I observed with Logitech FreePulse headset. In any case, I'm
glad that you are interested in A2DP audio quality and would
definitely appreciate any feedback and test results. As I see it,
there are two things to take care:
    1. make higher sbc bitpool settings usable in bluez with your headset
    2. make sure that sbc codec itself is well behaved and providing
the best results for any available bitpool
But I myself can only comment on the latter.

Using bitpool 128 is kind of weird for SBC codec, because the encoded
data stream has about the same size as the the original data and this
defeats the whole purpose of having any lossy compression at all. It
is also worth mentioning that bluez sbc encoder implementation uses a
bit lower precision analysis filter by default which allows SIMD
optimizations and this should be fine at normal encoding settings. But
uncommenting SBC_HIGH_PRECISION define may be important for getting
better audio quality at higher bitpool settings. Also trying to
enable/disable joint stereo, testing 4 vs. 8 subbands and snr vs.
loudness allocation method may probably have some impact on audio
quality for your use case. I still recommend you to try sbcenc/sbcdec
tools for the experiments with audio quality. They use a somewhat less
common big endian http://en.wikipedia.org/wiki/Au_file_format but you
can get your audio files converted to it using
http://sox.sourceforge.net/ command line tool. Also here I have some
ruby script, which can simplify sbc encoding/decoding testing:
    http://cgit.freedesktop.org/~siamashka/bluez/log/?h=sbc-tests

When doing the evaluation of encoding/decoding audio quality with
sbcenc/sbcdec, please keep in mind that the current bluez sbc
*decoder* implementation has some issues. Partially motivated by your
posts, I decided to finally attempt to bring the bluez sbc decoder
into a better shape and already sent one patch to the mailing list for
review. The rest of the fixes are available here and they improve sbc
decoder audio quality:
    http://cgit.freedesktop.org/~siamashka/bluez/log/?h=sbc-decoder-fixes
But the synthesis filter still needs to be converted to fixed point
and optimized for performance before really using it in bluez. I'll
try to do this work as time permits and will submit the patches later.
But quality-wise, it should be already possible to use my
'sbc-decoder-fixes' git branch for testing.

-- 
Best regards,
Siarhei Siamashka

[-- Attachment #2: 0001-HACK-sbc-reduce-volume-in-sbc-encoder.patch --]
[-- Type: text/x-patch, Size: 1864 bytes --]

From 123a021376a4fd66c88fc1ed6ca26a6e8ee5e365 Mon Sep 17 00:00:00 2001
From: Siarhei Siamashka <siarhei.siamashka@nokia.com>
Date: Tue, 18 Oct 2011 04:08:07 +0300
Subject: [PATCH] HACK: sbc: reduce volume in sbc encoder

Reducing audio volume when encoding helps to avoid clipping
in the decoder for the heavily dynamically compressed audio.
Also high precision mode is enabled in sbc encoder to get the
best possible audio quality.

Some references:
    http://en.wikipedia.org/wiki/Loudness_war
    http://www.hydrogenaudio.org/forums/index.php?showtopic=82112
---
 sbc/sbc_tables.h |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/sbc/sbc_tables.h b/sbc/sbc_tables.h
index 28c0d54..8d025e0 100644
--- a/sbc/sbc_tables.h
+++ b/sbc/sbc_tables.h
@@ -136,7 +136,7 @@ static const int32_t synmatrix8[16][8] = {
 
 /* Uncomment the following line to enable high precision build of SBC encoder */
 
-/* #define SBC_HIGH_PRECISION */
+#define SBC_HIGH_PRECISION
 
 #ifdef SBC_HIGH_PRECISION
 #define FIXED_A int64_t /* data type for fixed point accumulator */
@@ -208,7 +208,7 @@ static const FIXED_T _sbc_proto_fixed4[40] = {
  */
 #define SBC_COS_TABLE_FIXED4_SCALE \
 	((sizeof(FIXED_T) * CHAR_BIT - 1) + SBC_FIXED_EXTRA_BITS)
-#define F_COS4(x) (FIXED_A) ((x) * \
+#define F_COS4(x) (FIXED_A) ((x * 0.7) * \
 	((FIXED_A) 1 << (sizeof(FIXED_T) * CHAR_BIT - 1)) + 0.5)
 #define F(x) F_COS4(x)
 static const FIXED_T cos_table_fixed_4[32] = {
@@ -305,7 +305,7 @@ static const FIXED_T _sbc_proto_fixed8[80] = {
  */
 #define SBC_COS_TABLE_FIXED8_SCALE \
 	((sizeof(FIXED_T) * CHAR_BIT - 1) + SBC_FIXED_EXTRA_BITS)
-#define F_COS8(x) (FIXED_A) ((x) * \
+#define F_COS8(x) (FIXED_A) ((x * 0.7) * \
 	((FIXED_A) 1 << (sizeof(FIXED_T) * CHAR_BIT - 1)) + 0.5)
 #define F(x) F_COS8(x)
 static const FIXED_T cos_table_fixed_8[128] = {
-- 
1.7.3.4

Re: A2DP quality (bluetooth-alsa)

[qduaty]

2011/10/18 Siarhei Siamashka <siarhei.siamashka@gmail.com>:
> Still I would suggest you to try the attached patch just for a test.
> It reduces audio volume as part of the sbc encoding process and should
> eliminate most of the possible clipping cases on the decoding side.

Sorry for responding after a half year :) but I finally tested your
encoder patch applied to libbluetooth version 4.96, using AIMP3 and
pulseaudio. Unpatched encoder produces a "digital" distortion (which
is rather annoying) and sounds much like BlueSoleil on Windows. After
patching, music is slightly quieter (which can be expected if you
decreased its volume) and has an "analog overdrive" like through a
cheap FM radio. Anyway, the annoying "digital" distortion that was
present in unpatched encoder, is now gone.

Some fluctuations can be heard in higher bands in classical music, and
(as expected) instruments are indistinguishable, but it's acceptable
given the low bitpool.

For the "analog overdrive" I hear, it may also come from the device
itself (its proper name is Nokia BH-503). Considering its glue sealed
case, it might have been designed as a budget solution and its price
was "adjusted" due to no competition.

> Using bitpool 128 is kind of weird for SBC codec, because the encoded
> data stream has about the same size as the the original data and this
> defeats the whole purpose of having any lossy compression at all.

Yes, but who needs a lossy compression in an endpoint? SBC and MP3
were introduced to overcome the low throughput problem of the
bluetooth link, which is no longer the case since 2.0+EDR has plenty
of unused bits.

> I still recommend you to try sbcenc/sbcdec tools for the experiments with audio quality.

I tried them some time ago, but I believe ALSA backend may serve for
the same purpose (and is much easier to use). 8 blocks seems to sound
slightly better than 16 for classical music, full stereo eliminates
the floating of virtual sound sources, 48 kHz also seems to increase
quality a bit when compared to 44.1.

Regards

--
Sebastian Olter

Re: A2DP quality (bluetooth-alsa)

[Siarhei Siamashka]

On Tue, Mar 13, 2012 at 3:48 AM, qduaty <qduaty@gmail.com> wrote:
> 2011/10/18 Siarhei Siamashka <siarhei.siamashka@gmail.com>:
>> Still I would suggest you to try the attached patch just for a test.
>> It reduces audio volume as part of the sbc encoding process and should
>> eliminate most of the possible clipping cases on the decoding side.
>
> Sorry for responding after a half year :) but I finally tested your
> encoder patch applied to libbluetooth version 4.96, using AIMP3 and
> pulseaudio. Unpatched encoder produces a "digital" distortion (which
> is rather annoying) and sounds much like BlueSoleil on Windows. After
> patching, music is slightly quieter (which can be expected if you
> decreased its volume) and has an "analog overdrive" like through a
> cheap FM radio. Anyway, the annoying "digital" distortion that was
> present in unpatched encoder, is now gone.

Thanks for testing! One more experiment would be very much welcome. I
wonder if just defining SBC_HIGH_PRECISION without doing anything else
would have any noticeable effect on audio quality in listening tests.

> Some fluctuations can be heard in higher bands in classical music, and
> (as expected) instruments are indistinguishable, but it's acceptable
> given the low bitpool.
>
> For the "analog overdrive" I hear, it may also come from the device
> itself (its proper name is Nokia BH-503). Considering its glue sealed
> case, it might have been designed as a budget solution and its price
> was "adjusted" due to no competition.
>
>> Using bitpool 128 is kind of weird for SBC codec, because the encoded
>> data stream has about the same size as the the original data and this
>> defeats the whole purpose of having any lossy compression at all.
>
> Yes, but who needs a lossy compression in an endpoint? SBC and MP3
> were introduced to overcome the low throughput problem of the
> bluetooth link, which is no longer the case since 2.0+EDR has plenty
> of unused bits.
>
>> I still recommend you to try sbcenc/sbcdec tools for the experiments with audio quality.
>
> I tried them some time ago, but I believe ALSA backend may serve for
> the same purpose (and is much easier to use). 8 blocks seems to sound
> slightly better than 16 for classical music, full stereo eliminates
> the floating of virtual sound sources, 48 kHz also seems to increase
> quality a bit when compared to 44.1.

sbcenc/sbcdec tools can do file-to-file encoding/decoding and
eliminate the need to use any bluetooth device (which could
potentially be the source of some audio quality issues). However, as I
mentioned before, audio quality in the current bluez sbc decoder is
far from perfect and the synthesis filter needs to be replaced first.

-- 
Best regards,
Siarhei Siamashka

Re: A2DP quality (bluetooth-alsa)

[qduaty]

2012/3/13 Siarhei Siamashka <siarhei.siamashka@gmail.com>:
> On Tue, Mar 13, 2012 at 3:48 AM, qduaty <qduaty@gmail.com> wrote:
>> 2011/10/18 Siarhei Siamashka <siarhei.siamashka@gmail.com>:
> Thanks for testing! One more experiment would be very much welcome. I
> wonder if just defining SBC_HIGH_PRECISION without doing anything else
> would have any noticeable effect on audio quality in listening tests.

Well, before I move on, there is one thing that makes me sad:

< ACL data: handle 1 flags 0x02 dlen 7
    L2CAP(d): cid 0x006d len 3 [psm 25]
      AVDTP(s): Capabilities cmd: transaction 10 nsp 0x00
        ACP SEID 1
> HCI Event: Number of Completed Packets (0x13) plen 5
    handle 1 packets 1
> ACL data: handle 1 flags 0x02 dlen 16
    L2CAP(d): cid 0x0040 len 12 [psm 25]
      AVDTP(s): Capabilities rsp: transaction 10 nsp 0x00
        Media Transport
        Media Codec - SBC
          44.1kHz
          Stereo
          16 Blocks
          8 Subbands
          Loudness
          Bitpool Range 112-128
< ACL data: handle 1 flags 0x02 dlen 18
    L2CAP(d): cid 0x006d len 14 [psm 25]
      AVDTP(s): Set config cmd: transaction 11 nsp 0x00
        ACP SEID 1 - INT SEID 2
        Media Transport
        Media Codec - SBC
          44.1kHz
          Stereo
          16 Blocks
          8 Subbands
          Loudness
          Bitpool Range 112-53
> HCI Event: Number of Completed Packets (0x13) plen 5
    handle 1 packets 1
> ACL data: handle 1 flags 0x02 dlen 8
    L2CAP(d): cid 0x0040 len 4 [psm 25]
      AVDTP(s): Set config rej: transaction 11 nsp 0x00
        Media Codec
        Error code 204

The other device that tries to communicate at bitpool=112 and gives
up, is my phone. Now I'm listening to it with the headset and the
sound is perfectly clear, with no sign of that "analog" distortion I
mentioned earlier.

Bluez has MAX_BITPOOL=53 or 64 stored everywhere and who knows, maybe
it's even able to recover these magic numbers from bitrate. Should I
look for bitrate constant, like 320 or so?

> audio quality in the current bluez sbc decoder is
> far from perfect and the synthesis filter needs to be replaced first.

I'm not sure what you expect from sbcenc-sbcdec tests. Even if they
prove the encoding/decoding within Bluez has a reasonably good
quality, what then? Will it propagate among different devices,
bluetooth stacks?


Regards

--
Sebastian Olter

Re: A2DP quality (bluetooth-alsa)

[Siarhei Siamashka]

On Tue, Mar 13, 2012 at 2:45 PM, qduaty <qduaty@gmail.com> wrote:
> 2012/3/13 Siarhei Siamashka <siarhei.siamashka@gmail.com>:
>> On Tue, Mar 13, 2012 at 3:48 AM, qduaty <qduaty@gmail.com> wrote:
>>> 2011/10/18 Siarhei Siamashka <siarhei.siamashka@gmail.com>:
>> Thanks for testing! One more experiment would be very much welcome. I
>> wonder if just defining SBC_HIGH_PRECISION without doing anything else
>> would have any noticeable effect on audio quality in listening tests.
>
> Well, before I move on, there is one thing that makes me sad:
>
> < ACL data: handle 1 flags 0x02 dlen 7
>    L2CAP(d): cid 0x006d len 3 [psm 25]
>      AVDTP(s): Capabilities cmd: transaction 10 nsp 0x00
>        ACP SEID 1
>> HCI Event: Number of Completed Packets (0x13) plen 5
>    handle 1 packets 1
>> ACL data: handle 1 flags 0x02 dlen 16
>    L2CAP(d): cid 0x0040 len 12 [psm 25]
>      AVDTP(s): Capabilities rsp: transaction 10 nsp 0x00
>        Media Transport
>        Media Codec - SBC
>          44.1kHz
>          Stereo
>          16 Blocks
>          8 Subbands
>          Loudness
>          Bitpool Range 112-128
> < ACL data: handle 1 flags 0x02 dlen 18
>    L2CAP(d): cid 0x006d len 14 [psm 25]
>      AVDTP(s): Set config cmd: transaction 11 nsp 0x00
>        ACP SEID 1 - INT SEID 2
>        Media Transport
>        Media Codec - SBC
>          44.1kHz
>          Stereo
>          16 Blocks
>          8 Subbands
>          Loudness
>          Bitpool Range 112-53
>> HCI Event: Number of Completed Packets (0x13) plen 5
>    handle 1 packets 1
>> ACL data: handle 1 flags 0x02 dlen 8
>    L2CAP(d): cid 0x0040 len 4 [psm 25]
>      AVDTP(s): Set config rej: transaction 11 nsp 0x00
>        Media Codec
>        Error code 204

I'm only familiar with SBC codec in bluez, but not much with A2DP
protocol. Maybe somebody else could comment.

> The other device that tries to communicate at bitpool=112 and gives
> up, is my phone. Now I'm listening to it with the headset and the
> sound is perfectly clear, with no sign of that "analog" distortion I
> mentioned earlier.

Are you now comparing streaming of A2DP audio from your phone to your
PC (using bluez A2DP sink) and to your bluetooth headset?

> Bluez has MAX_BITPOOL=53 or 64 stored everywhere and who knows, maybe
> it's even able to recover these magic numbers from bitrate. Should I
> look for bitrate constant, like 320 or so?

Maybe. If there is some problem negotiating optimal bitpool setting,
then it is better to be fixed.

>> audio quality in the current bluez sbc decoder is
>> far from perfect and the synthesis filter needs to be replaced first.
>
> I'm not sure what you expect from sbcenc-sbcdec tests. Even if they
> prove the encoding/decoding within Bluez has a reasonably good
> quality, what then? Will it propagate among different devices,
> bluetooth stacks?

sbcenc-sbcdec tests are useful for testing bluez SBC codec. For any
given bitpool value, encoding/decoding roundtrip should preserve as
much audio quality as possible. And trying different possible
combinations of encoder/decoder (bluez / reference sbc codec /
bluetooth headset) should help to provide the answers to the questions
you are asking.

-- 
Best regards,
Siarhei Siamashka

Re: A2DP quality (bluetooth-alsa)

[qduaty]

[-- Attachment #1: Type: text/plain, Size: 883 bytes --]

2012/3/13 Siarhei Siamashka <siarhei.siamashka@gmail.com>:
> Are you now comparing streaming of A2DP audio from your phone to your
> PC (using bluez A2DP sink) and to your bluetooth headset?

I assume that the phone streams data to the headset (whose session is
hard to dump on Windows mobile) with bitpool = 112, ie. the same it
fails to set in Bluez and I'm saying it gives good results in terms of
perceiveable quality, from which Bluez cannot take advantage due to
its inherent bitpool=53 limitation.

Anyway, I did some additional listening tests (by listening to Bluez
SBC on an A2DP headset). SBC_HIGH_PRECISION alone (without your hack)
only slightly improves quality. Also I finally managed to force higher
bitpool in ALSA (patch attached). This makes further improvement in
quality, at least on headsets that do not choke on higher bitpools.


Regards


--
Sebastian Olter

[-- Attachment #2: pcm_bluetooth.c.diff --]
[-- Type: application/octet-stream, Size: 862 bytes --]

*** pcm_bluetooth.c	2011-12-21 23:53:54.000000000 +0100
--- /usr/src/bluez-4.99/audio/pcm_bluetooth.c	2012-03-14 12:52:34.299173684 +0100
***************
*** 680,690 ****
  	case BT_A2DP_BLOCK_LENGTH_16:
  		a2dp->sbc.blocks = SBC_BLK_16;
  		break;
  	}
  
! 	a2dp->sbc.bitpool = active_capabilities.max_bitpool;
  	a2dp->codesize = sbc_get_codesize(&a2dp->sbc);
  	a2dp->count = sizeof(struct rtp_header) + sizeof(struct rtp_payload);
  }
  
  static int bluetooth_a2dp_hw_params(snd_pcm_ioplug_t *io,
--- 678,689 ----
  	case BT_A2DP_BLOCK_LENGTH_16:
  		a2dp->sbc.blocks = SBC_BLK_16;
  		break;
  	}
  
! 	a2dp->sbc.bitpool = active_capabilities.max_bitpool * 2;
!  
  	a2dp->codesize = sbc_get_codesize(&a2dp->sbc);
  	a2dp->count = sizeof(struct rtp_header) + sizeof(struct rtp_payload);
  }
  
  static int bluetooth_a2dp_hw_params(snd_pcm_ioplug_t *io,

[PATCH] Re: A2DP quality (bluetooth-alsa)

[qduaty]

[-- Attachment #1: Type: text/plain, Size: 704 bytes --]

Hello,

I found that floating point processing eliminates all the quality
issues that were mentioned here. So I added a floating point mode to
SBC, which can be enabled with an #ifdef in sbc_tables.h, along with a
code for minimizing quantization errors. It reduces noise by several
dB and greatly improves the reproduction of higher bands.

There is room for further improvements, such as adaptive clipping
prevention and high resolution input format, but with current code
base, they are somewhat hard to implement.

The attached patch may cause some trouble because of one part being
rejected. I hope this will not prevent those who are interested, from
applying it and testing.


--

Sebastian Olter

[-- Attachment #2: float.diff --]
[-- Type: text/x-diff, Size: 16835 bytes --]

diff -bc5 bluez-4.99-orig/sbc//sbc_primitives.c bluez-4.99/sbc//sbc_primitives.c
*** bluez-4.99-orig/sbc//sbc_primitives.c	2010-11-20 21:25:14.000000000 +0100
--- bluez-4.99/sbc//sbc_primitives.c	2012-03-25 01:12:29.502123931 +0100
***************
*** 35,44 ****
--- 35,47 ----
  #include "sbc_primitives_mmx.h"
  #include "sbc_primitives_iwmmxt.h"
  #include "sbc_primitives_neon.h"
  #include "sbc_primitives_armv6.h"
  
+ #define ANTICLIP 0.8
+ 
+ 
  /*
   * A reference C code of analysis filter with SIMD-friendly tables
   * reordering and code layout. This code can be used to develop platform
   * specific SIMD optimizations. Also it may be used as some kind of test
   * for compiler autovectorization capabilities (who knows, if the compiler
***************
*** 59,73 ****
  static inline void sbc_analyze_four_simd(const int16_t *in, int32_t *out,
  							const FIXED_T *consts)
  {
  	FIXED_A t1[4];
  	FIXED_T t2[4];
! 	int hop = 0;
! 
  	/* rounding coefficient */
  	t1[0] = t1[1] = t1[2] = t1[3] =
  		(FIXED_A) 1 << (SBC_PROTO_FIXED4_SCALE - 1);
  
  	/* low pass polyphase filter */
  	for (hop = 0; hop < 40; hop += 8) {
  		t1[0] += (FIXED_A) in[hop] * consts[hop];
  		t1[0] += (FIXED_A) in[hop + 1] * consts[hop + 1];
--- 62,82 ----
  static inline void sbc_analyze_four_simd(const int16_t *in, int32_t *out,
  							const FIXED_T *consts)
  {
  	FIXED_A t1[4];
  	FIXED_T t2[4];
! 	int hop = 0, i = 0;
! #ifdef SBC_FLOAT
! #define SBC_PROTO_SCALE_FIXED4(x) x;
! 	/* rounding coefficient */
! 	t1[0] = t1[1] = t1[2] = t1[3] = 1;
! #else
! #define SBC_PROTO_SCALE_FIXED4(x) x >> SBC_PROTO_FIXED4_SCALE
  	/* rounding coefficient */
  	t1[0] = t1[1] = t1[2] = t1[3] =
   		(FIXED_A) 1 << (SBC_PROTO_FIXED4_SCALE - 1);
+ #endif
  
  	/* low pass polyphase filter */
  	for (hop = 0; hop < 40; hop += 8) {
  		t1[0] += (FIXED_A) in[hop] * consts[hop];
  		t1[0] += (FIXED_A) in[hop + 1] * consts[hop + 1];
***************
*** 78,91 ****
  		t1[3] += (FIXED_A) in[hop + 6] * consts[hop + 6];
  		t1[3] += (FIXED_A) in[hop + 7] * consts[hop + 7];
  	}
  
  	/* scaling */
! 	t2[0] = t1[0] >> SBC_PROTO_FIXED4_SCALE;
! 	t2[1] = t1[1] >> SBC_PROTO_FIXED4_SCALE;
! 	t2[2] = t1[2] >> SBC_PROTO_FIXED4_SCALE;
! 	t2[3] = t1[3] >> SBC_PROTO_FIXED4_SCALE;
  
  	/* do the cos transform */
  	t1[0]  = (FIXED_A) t2[0] * consts[40 + 0];
  	t1[0] += (FIXED_A) t2[1] * consts[40 + 1];
  	t1[1]  = (FIXED_A) t2[0] * consts[40 + 2];
--- 87,100 ----
  		t1[3] += (FIXED_A) in[hop + 6] * consts[hop + 6];
  		t1[3] += (FIXED_A) in[hop + 7] * consts[hop + 7];
  	}
  
  	/* scaling */
! 	t2[0] = SBC_PROTO_SCALE_FIXED4(t1[0]);
! 	t2[1] = SBC_PROTO_SCALE_FIXED4(t1[1]);
! 	t2[2] = SBC_PROTO_SCALE_FIXED4(t1[2]);
! 	t2[3] = SBC_PROTO_SCALE_FIXED4(t1[3]);
  
  	/* do the cos transform */
   	t1[0]  = (FIXED_A) t2[0] * consts[40 + 0];
   	t1[0] += (FIXED_A) t2[1] * consts[40 + 1];
   	t1[1]  = (FIXED_A) t2[0] * consts[40 + 2];
***************
*** 102,131 ****
  	t1[2] += (FIXED_A) t2[2] * consts[40 + 12];
  	t1[2] += (FIXED_A) t2[3] * consts[40 + 13];
  	t1[3] += (FIXED_A) t2[2] * consts[40 + 14];
  	t1[3] += (FIXED_A) t2[3] * consts[40 + 15];
  
! 	out[0] = t1[0] >>
! 		(SBC_COS_TABLE_FIXED4_SCALE - SCALE_OUT_BITS);
! 	out[1] = t1[1] >>
! 		(SBC_COS_TABLE_FIXED4_SCALE - SCALE_OUT_BITS);
! 	out[2] = t1[2] >>
! 		(SBC_COS_TABLE_FIXED4_SCALE - SCALE_OUT_BITS);
! 	out[3] = t1[3] >>
  		(SBC_COS_TABLE_FIXED4_SCALE - SCALE_OUT_BITS);
  }
  
  static inline void sbc_analyze_eight_simd(const int16_t *in, int32_t *out,
  							const FIXED_T *consts)
  {
  	FIXED_A t1[8];
  	FIXED_T t2[8];
  	int i, hop;
! 
  	/* rounding coefficient */
  	t1[0] = t1[1] = t1[2] = t1[3] = t1[4] = t1[5] = t1[6] = t1[7] =
  		(FIXED_A) 1 << (SBC_PROTO_FIXED8_SCALE-1);
  
  	/* low pass polyphase filter */
  	for (hop = 0; hop < 80; hop += 16) {
  		t1[0] += (FIXED_A) in[hop] * consts[hop];
  		t1[0] += (FIXED_A) in[hop + 1] * consts[hop + 1];
--- 111,154 ----
   	t1[2] += (FIXED_A) t2[2] * consts[40 + 12];
   	t1[2] += (FIXED_A) t2[3] * consts[40 + 13];
   	t1[3] += (FIXED_A) t2[2] * consts[40 + 14];
   	t1[3] += (FIXED_A) t2[3] * consts[40 + 15];
  
! #ifdef SBC_FLOAT
! 	/* Tries to redistribute quantization errors so that exactly half of them 
! 	 * is positive. This reduces overall distortion, especially in high bands
! 	 * and increases chances for clipping, hence the ANTICLIP coefficient. */
! 	double accumError = 0;
! 	for (i = 0; i < 4; i++){
! 		t1[i] *= (1 << SCALE_OUT_BITS) * ANTICLIP;
! 		accumError += t1[i] - (int32_t)t1[i];
! 	}
! 	for (i = 0; i < 4; i++)
! 		out[i] = t1[i] + accumError / 4;
! #else
! 	for (i = 0; i < 4; i++)
! 		out[i] = t1[i] >> 
  			(SBC_COS_TABLE_FIXED4_SCALE - SCALE_OUT_BITS);
+ #endif
  }
  
  static inline void sbc_analyze_eight_simd(const int16_t *in, int32_t *out,
  							    const FIXED_T *consts)
  {
  	FIXED_A t1[8];
  	FIXED_T t2[8];
  	int i, hop;
! #ifdef SBC_FLOAT
! #define SBC_PROTO_SCALE_FIXED8(x) x;
! 	/* rounding coefficient */
! 	t1[0] = t1[1] = t1[2] = t1[3] = t1[4] = t1[5] = t1[6] = t1[7] = 1;
! #else
! #define SBC_PROTO_SCALE_FIXED8(x) x >> SBC_PROTO_FIXED8_SCALE
  	/* rounding coefficient */
  	t1[0] = t1[1] = t1[2] = t1[3] = t1[4] = t1[5] = t1[6] = t1[7] =
  		 (FIXED_A) 1 << (SBC_PROTO_FIXED8_SCALE-1);
+ #endif
  
  	/* low pass polyphase filter */
  	for (hop = 0; hop < 80; hop += 16) {
  		t1[0] += (FIXED_A) in[hop] * consts[hop];
  		t1[0] += (FIXED_A) in[hop + 1] * consts[hop + 1];
***************
*** 144,161 ****
  		t1[7] += (FIXED_A) in[hop + 14] * consts[hop + 14];
  		t1[7] += (FIXED_A) in[hop + 15] * consts[hop + 15];
  	}
  
  	/* scaling */
! 	t2[0] = t1[0] >> SBC_PROTO_FIXED8_SCALE;
! 	t2[1] = t1[1] >> SBC_PROTO_FIXED8_SCALE;
! 	t2[2] = t1[2] >> SBC_PROTO_FIXED8_SCALE;
! 	t2[3] = t1[3] >> SBC_PROTO_FIXED8_SCALE;
! 	t2[4] = t1[4] >> SBC_PROTO_FIXED8_SCALE;
! 	t2[5] = t1[5] >> SBC_PROTO_FIXED8_SCALE;
! 	t2[6] = t1[6] >> SBC_PROTO_FIXED8_SCALE;
! 	t2[7] = t1[7] >> SBC_PROTO_FIXED8_SCALE;
  
  
  	/* do the cos transform */
  	t1[0] = t1[1] = t1[2] = t1[3] = t1[4] = t1[5] = t1[6] = t1[7] = 0;
  
--- 167,184 ----
  		t1[7] += (FIXED_A) in[hop + 14] * consts[hop + 14];
  		t1[7] += (FIXED_A) in[hop + 15] * consts[hop + 15];
  	}
  
  	/* scaling */
! 	t2[0] = SBC_PROTO_SCALE_FIXED8(t1[0]);
! 	t2[1] = SBC_PROTO_SCALE_FIXED8(t1[1]);
! 	t2[2] = SBC_PROTO_SCALE_FIXED8(t1[2]);
! 	t2[3] = SBC_PROTO_SCALE_FIXED8(t1[3]);
! 	t2[4] = SBC_PROTO_SCALE_FIXED8(t1[4]);
! 	t2[5] = SBC_PROTO_SCALE_FIXED8(t1[5]);
! 	t2[6] = SBC_PROTO_SCALE_FIXED8(t1[6]);
! 	t2[7] = SBC_PROTO_SCALE_FIXED8(t1[7]);
  
  
  	/* do the cos transform */
  	t1[0] = t1[1] = t1[2] = t1[3] = t1[4] = t1[5] = t1[6] = t1[7] = 0;
  
***************
*** 176,188 ****
--- 199,227 ----
   		t1[6] += (FIXED_A) t2[i * 2 + 1] * consts[80 + i * 16 + 13];
  		t1[7] += (FIXED_A) t2[i * 2 + 0] * consts[80 + i * 16 + 14];
   		t1[7] += (FIXED_A) t2[i * 2 + 1] * consts[80 + i * 16 + 15];
  	}
  
+ #ifdef SBC_FLOAT
+ 	/* Tries to redistribute quantization errors so that exactly half of them 
+ 	 * is positive. This reduces overall distortion, especially in high bands
+ 	 * and increases chances for clipping, hence the ANTICLIP coefficient. */
+ 	double accumError = 0;
+ 	for (i = 0; i < 8; i++){
+ 		t1[i] *= (1 << SCALE_OUT_BITS) * ANTICLIP;
+ 		accumError += t1[i] - (int32_t)t1[i];
+ 	}
+ 	for (i = 0; i < 8; i++)
+ 		out[i] = t1[i] + accumError / 8;
+ /*
+ 		out[i] = t1[i]; 
+ */
+ #else
  	for (i = 0; i < 8; i++)
  		out[i] = t1[i] >> 
  			(SBC_COS_TABLE_FIXED8_SCALE - SCALE_OUT_BITS);
+ #endif
  }
  
  static inline void sbc_analyze_4b_4s_simd(int16_t *x,
  	int32_t *out, int out_stride)
  {
diff -bc5 bluez-4.99-orig/sbc//sbc_tables.h bluez-4.99/sbc//sbc_tables.h
*** bluez-4.99-orig/sbc//sbc_tables.h	2011-12-21 23:53:54.000000000 +0100
--- bluez-4.99/sbc//sbc_tables.h	2012-03-21 18:08:15.559689237 +0100
***************
*** 136,154 ****
  	  SN8(0xfb8e3130), SN8(0xf8275a10), SN8(0xfe70747c), SN8(0x06a6d988) }
  };
  
  /* Uncomment the following line to enable high precision build of SBC encoder */
  
! /* #define SBC_HIGH_PRECISION */
  
  #ifdef SBC_HIGH_PRECISION
  #define FIXED_A int64_t /* data type for fixed point accumulator */
  #define FIXED_T int32_t /* data type for fixed point constants */
  #define SBC_FIXED_EXTRA_BITS 16
  #else
  #define FIXED_A int32_t /* data type for fixed point accumulator */
  #define FIXED_T int16_t /* data type for fixed point constants */
  #define SBC_FIXED_EXTRA_BITS 0
  #endif
  
  /* A2DP specification: Section 12.8 Tables
   *
--- 136,175 ----
  	  SN8(0xfb8e3130), SN8(0xf8275a10), SN8(0xfe70747c), SN8(0x06a6d988) }
  };
  
  /* Uncomment the following line to enable high precision build of SBC encoder */
  
! //#define SBC_HIGH_PRECISION
! #define SBC_FLOAT
  
  #ifdef SBC_HIGH_PRECISION
  #define FIXED_A int64_t /* data type for fixed point accumulator */
  #define FIXED_T int32_t /* data type for fixed point constants */
+ #define FIXED_T_BITS (sizeof(FIXED_T) * CHAR_BIT - 1)
+ #define F_PROTO4(x) (FIXED_A) ((x * 2) * (1LL << FIXED_T_BITS) + 0.5)
+ #define F_COS4(x) ((x) * (1LL << FIXED_T_BITS) + 0.5)
+ #define F_PROTO8(x) ((x * 2) * (1LL << FIXED_T_BITS) + 0.5)
+ #define F_COS8(x) ((x) * (1LL << FIXED_T_BITS) + 0.5)
  #define SBC_FIXED_EXTRA_BITS 16
+ #elif defined(SBC_FLOAT)
+ #define SBC_HIGH_PRECISION // disable MMX
+ #define FIXED_A double /* data type for fixed point accumulator */
+ #define FIXED_T float /* data type for fixed point constants */
+ #define FIXED_T_BITS (sizeof(FIXED_T) * CHAR_BIT - 1)
+ #define F_PROTO4(x) (x)
+ #define F_COS4(x) (x)
+ #define F_PROTO8(x) (x)
+ #define F_COS8(x) (x)
+ #define SBC_FIXED_EXTRA_BITS 0
  #else
  #define FIXED_A int32_t /* data type for fixed point accumulator */
  #define FIXED_T int16_t /* data type for fixed point constants */
+ #define FIXED_T_BITS (sizeof(FIXED_T) * CHAR_BIT - 1)
+ #define F_PROTO4(x) (FIXED_A) ((x * 2) * (1 << FIXED_T_BITS) + 0.5)
+ #define F_COS4(x) ((x) * (1 << FIXED_T_BITS) + 0.5)
+ #define F_PROTO8(x) ((x * 2) * (1 << FIXED_T_BITS) + 0.5)
+ #define F_COS8(x) ((x) * (1 << FIXED_T_BITS) + 0.5)
  #define SBC_FIXED_EXTRA_BITS 0
  #endif
  
  /* A2DP specification: Section 12.8 Tables
   *
***************
*** 156,191 ****
   * maximum which is possible without overflows)
   *
   * Note: in each block of 8 numbers sign was changed for elements 2 and 7
   * in order to compensate the same change applied to cos_table_fixed_4
   */
! #define SBC_PROTO_FIXED4_SCALE \
! 	((sizeof(FIXED_T) * CHAR_BIT - 1) - SBC_FIXED_EXTRA_BITS + 1)
! #define F_PROTO4(x) (FIXED_A) ((x * 2) * \
! 	((FIXED_A) 1 << (sizeof(FIXED_T) * CHAR_BIT - 1)) + 0.5)
  #define F(x) F_PROTO4(x)
  static const FIXED_T _sbc_proto_fixed4[40] = {
  	F(0.00000000E+00),  F(5.36548976E-04),
  	-F(1.49188357E-03),  F(2.73370904E-03),
  	F(3.83720193E-03),  F(3.89205149E-03),
  	F(1.86581691E-03),  F(3.06012286E-03),
- 
  	F(1.09137620E-02),  F(2.04385087E-02),
  	-F(2.88757392E-02),  F(3.21939290E-02),
  	F(2.58767811E-02),  F(6.13245186E-03),
  	-F(2.88217274E-02),  F(7.76463494E-02),
- 
  	F(1.35593274E-01),  F(1.94987841E-01),
  	-F(2.46636662E-01),  F(2.81828203E-01),
  	F(2.94315332E-01),  F(2.81828203E-01),
  	F(2.46636662E-01), -F(1.94987841E-01),
- 
  	-F(1.35593274E-01), -F(7.76463494E-02),
  	F(2.88217274E-02),  F(6.13245186E-03),
  	F(2.58767811E-02),  F(3.21939290E-02),
  	F(2.88757392E-02), -F(2.04385087E-02),
- 
  	-F(1.09137620E-02), -F(3.06012286E-03),
  	-F(1.86581691E-03),  F(3.89205149E-03),
  	F(3.83720193E-03),  F(2.73370904E-03),
  	F(1.49188357E-03), -F(5.36548976E-04),
  };
--- 177,205 ----
   * maximum which is possible without overflows)
   *
   * Note: in each block of 8 numbers sign was changed for elements 2 and 7
   * in order to compensate the same change applied to cos_table_fixed_4
   */
! #define SBC_PROTO_FIXED4_SCALE (FIXED_T_BITS - SBC_FIXED_EXTRA_BITS + 1)
  #define F(x) F_PROTO4(x)
  static const FIXED_T _sbc_proto_fixed4[40] = {
  	F(0.00000000E+00),  F(5.36548976E-04),	
          -F(1.49188357E-03),  F(2.73370904E-03),
  	F(3.83720193E-03),  F(3.89205149E-03),	
          F(1.86581691E-03),  F(3.06012286E-03),
  	F(1.09137620E-02),  F(2.04385087E-02),	
          -F(2.88757392E-02),  F(3.21939290E-02),
  	F(2.58767811E-02),  F(6.13245186E-03),	
          -F(2.88217274E-02),  F(7.76463494E-02),
  	F(1.35593274E-01),  F(1.94987841E-01),	
          -F(2.46636662E-01),  F(2.81828203E-01),
  	F(2.94315332E-01),  F(2.81828203E-01),	
          F(2.46636662E-01), -F(1.94987841E-01),
  	-F(1.35593274E-01), -F(7.76463494E-02),	
          F(2.88217274E-02),  F(6.13245186E-03),
  	F(2.58767811E-02),  F(3.21939290E-02),	
          F(2.88757392E-02), -F(2.04385087E-02),
  	-F(1.09137620E-02), -F(3.06012286E-03),	
          -F(1.86581691E-03),  F(3.89205149E-03),
  	F(3.83720193E-03),  F(2.73370904E-03),	
          F(1.49188357E-03), -F(5.36548976E-04),
  };
***************
*** 206,219 ****
   * Change of sign for element 2 allows to replace constant 1.0 (not
   * representable in Q15 format) with -1.0 (fine with Q15).
   * Changed sign for element 7 allows to have more similar constants
   * and simplify subband filter function code.
   */
! #define SBC_COS_TABLE_FIXED4_SCALE \
! 	((sizeof(FIXED_T) * CHAR_BIT - 1) + SBC_FIXED_EXTRA_BITS)
! #define F_COS4(x) (FIXED_A) ((x) * \
! 	((FIXED_A) 1 << (sizeof(FIXED_T) * CHAR_BIT - 1)) + 0.5)
  #define F(x) F_COS4(x)
  static const FIXED_T cos_table_fixed_4[32] = {
  	F(0.7071067812),  F(0.9238795325), -F(1.0000000000),  F(0.9238795325),
  	F(0.7071067812),  F(0.3826834324),  F(0.0000000000),  F(0.3826834324),
  
--- 220,230 ----
   * Change of sign for element 2 allows to replace constant 1.0 (not
   * representable in Q15 format) with -1.0 (fine with Q15).
   * Changed sign for element 7 allows to have more similar constants
   * and simplify subband filter function code.
   */
! #define SBC_COS_TABLE_FIXED4_SCALE (FIXED_T_BITS + SBC_FIXED_EXTRA_BITS)
  #define F(x) F_COS4(x)
  static const FIXED_T cos_table_fixed_4[32] = {
  	F(0.7071067812),  F(0.9238795325), -F(1.0000000000),  F(0.9238795325),
  	F(0.7071067812),  F(0.3826834324),  F(0.0000000000),  F(0.3826834324),
  
***************
*** 234,247 ****
   * maximum which is possible without overflows)
   *
   * Note: in each block of 16 numbers sign was changed for elements 4, 13, 14, 15
   * in order to compensate the same change applied to cos_table_fixed_8
   */
! #define SBC_PROTO_FIXED8_SCALE \
! 	((sizeof(FIXED_T) * CHAR_BIT - 1) - SBC_FIXED_EXTRA_BITS + 1)
! #define F_PROTO8(x) (FIXED_A) ((x * 2) * \
! 	((FIXED_A) 1 << (sizeof(FIXED_T) * CHAR_BIT - 1)) + 0.5)
  #define F(x) F_PROTO8(x)
  static const FIXED_T _sbc_proto_fixed8[80] = {
  	F(0.00000000E+00),  F(1.56575398E-04),
  	F(3.43256425E-04),  F(5.54620202E-04),
  	-F(8.23919506E-04),  F(1.13992507E-03),
--- 245,255 ----
   * maximum which is possible without overflows)
   *
   * Note: in each block of 16 numbers sign was changed for elements 4, 13, 14, 15
   * in order to compensate the same change applied to cos_table_fixed_8
   */
! #define SBC_PROTO_FIXED8_SCALE (FIXED_T_BITS - SBC_FIXED_EXTRA_BITS + 1)
  #define F(x) F_PROTO8(x)
  static const FIXED_T _sbc_proto_fixed8[80] = {
  	F(0.00000000E+00),  F(1.56575398E-04),
  	F(3.43256425E-04),  F(5.54620202E-04),
  	-F(8.23919506E-04),  F(1.13992507E-03),
***************
*** 303,316 ****
   * Change of sign for element 4 allows to replace constant 1.0 (not
   * representable in Q15 format) with -1.0 (fine with Q15).
   * Changed signs for elements 13, 14, 15 allow to have more similar constants
   * and simplify subband filter function code.
   */
! #define SBC_COS_TABLE_FIXED8_SCALE \
! 	((sizeof(FIXED_T) * CHAR_BIT - 1) + SBC_FIXED_EXTRA_BITS)
! #define F_COS8(x) (FIXED_A) ((x) * \
! 	((FIXED_A) 1 << (sizeof(FIXED_T) * CHAR_BIT - 1)) + 0.5)
  #define F(x) F_COS8(x)
  static const FIXED_T cos_table_fixed_8[128] = {
  	F(0.7071067812),  F(0.8314696123),  F(0.9238795325),  F(0.9807852804),
  	-F(1.0000000000),  F(0.9807852804),  F(0.9238795325),  F(0.8314696123),
  	F(0.7071067812),  F(0.5555702330),  F(0.3826834324),  F(0.1950903220),
--- 311,321 ----
   * Change of sign for element 4 allows to replace constant 1.0 (not
   * representable in Q15 format) with -1.0 (fine with Q15).
   * Changed signs for elements 13, 14, 15 allow to have more similar constants
   * and simplify subband filter function code.
   */
! #define SBC_COS_TABLE_FIXED8_SCALE (FIXED_T_BITS + SBC_FIXED_EXTRA_BITS)
  #define F(x) F_COS8(x)
  static const FIXED_T cos_table_fixed_8[128] = {
  	F(0.7071067812),  F(0.8314696123),  F(0.9238795325),  F(0.9807852804),
  	-F(1.0000000000),  F(0.9807852804),  F(0.9238795325),  F(0.8314696123),
  	F(0.7071067812),  F(0.5555702330),  F(0.3826834324),  F(0.1950903220),

Re: [PATCH] Re: A2DP quality (bluetooth-alsa)

[Siarhei Siamashka]

On Sun, Mar 25, 2012 at 5:51 AM, qduaty <qduaty@gmail.com> wrote:
> Hello,
>
> I found that floating point processing eliminates all the quality
> issues that were mentioned here. So I added a floating point mode to
> SBC, which can be enabled with an #ifdef in sbc_tables.h, along with a
> code for minimizing quantization errors. It reduces noise by several
> dB and greatly improves the reproduction of higher bands.

Out of curiosity, how do you measure noise reduction?

Regarding your changes:

! #ifdef SBC_FLOAT
! #define SBC_PROTO_SCALE_FIXED4(x) x;
! 	/* rounding coefficient */
! 	t1[0] = t1[1] = t1[2] = t1[3] = 1;

This does not look right. For the fixed point code, rounding
coefficient is needed because we are shifting results after the first
multiplication pass. Simply right shifting the data would discard the
lowest bits, effectively doing rounding down. Introducing rounding
coefficient allows rounding to nearest: "(x + (1 <<
(SBC_PROTO_FIXED4_SCALE - 1))) >> SBC_PROTO_FIXED4_SCALE"
Floating point code does not need any rounding coefficient.

! #ifdef SBC_FLOAT
! 	/* Tries to redistribute quantization errors so that exactly half of them
! 	 * is positive. This reduces overall distortion, especially in high bands
! 	 * and increases chances for clipping, hence the ANTICLIP coefficient. */
! 	double accumError = 0;
! 	for (i = 0; i < 4; i++){
! 		t1[i] *= (1 << SCALE_OUT_BITS) * ANTICLIP;
! 		accumError += t1[i] - (int32_t)t1[i];
! 	}
! 	for (i = 0; i < 4; i++)
! 		out[i] = t1[i] + accumError / 4;

I find it hard to believe that this accumError would make any
noticeable difference on the output (neither good nor bad).

The analysis filter produces fixed point values with SCALE_OUT_BITS
fractional bits (selected to be 15 currently). So the output is in
17.15 fixed point format. 15 extra bits have been selected just
because we can, and this allows to use full range of 32-bit variables.
If we further divide it by 2 as needed on quantization step later,
then we actually have 16.16 format. These extra bits are needed to
avoid precision loss on normalization. If quantized value is for
example in [-127, 127] range, then it needs to be stored as 8 bits or
[0, 255] range in the bitstream, which gives a rather nasty 255/256
scale ratio and this is a potential source of rounding errors unless
we preserve as many bits as possible. From A2DP spec:
    "levels[ch][sb] = pow(2.0, bits[ch][sb]) - 1"
    "scalefactor[ch][ sb] = pow(2.0, (scale_factor[ch][sb] + 1))"
    "quantized_sb_sample[blk][ch][sb] = ((sb_sample[blk][ch][sb] /
scalefactor[ch][sb] + 1.0) * levels[ch][sb]) / 2.0"
"quantized_sb_sample" is rounded towards minus infinity, "levels" are
powers of two minus one and scalefactor is a power of two.

But even less than 15 extra bits for SCALE_OUT_BITS seems to be also
fine in practice (you can do some experiments with reducing it),
assuming that the values are quantized even a little bit. With your
change, you are only messing around the lowest bit in the extra
fractional part.

Scaling down the input with ANTICLIP should get rid of clipping (on
the decoder side) with heavily compressed audio sources as expected,
but this is not anything new.

> There is room for further improvements, such as adaptive clipping
> prevention and high resolution input format, but with current code
> base, they are somewhat hard to implement.

If you have any questions about the current code, just ask.

> The attached patch may cause some trouble because of one part being
> rejected. I hope this will not prevent those who are interested, from
> applying it and testing.

Could you please at least use unified diff (-u option)? And the best
would be to use "git commit" and then "git format-patch".

In any case, the ability to reduce the audio volume as part of SBC
encoding process looks like a useful improvement to me (and maybe it's
a good idea to even have it used by default). This should help to
workaround the audio quality problems with some A2DP headsets.

-- 
Best regards,
Siarhei Siamashka

Re: [PATCH] Re: A2DP quality (bluetooth-alsa)

[qduaty]

2012/3/26 Siarhei Siamashka <siarhei.siamashka@gmail.com>:
> On Sun, Mar 25, 2012 at 5:51 AM, qduaty <qduaty@gmail.com> wrote:
> Out of curiosity, how do you measure noise reduction?

It seems to reduce noise making it less audible. I didn't perform
objective measurements because it's not relevant as long as the
(decoded) stream is not used for encoding.

> Floating point code does not need any rounding coefficient.

You have right, I missed the comment. t1 should be initialized to 0.

> I find it hard to believe that this accumError would make any
> noticeable difference on the output (neither good nor bad).

I don't believe it either, but it is a trick. Perhaps my results are
biased, but it always seems that adding 0.5 before rounding adds some
noise, while adding accumError calculated this way, does the opposite
(removes some noise). Objective measurements could give the right
answer. Anyway, if you're interested in the patch, I'd be careful with
removing the part of code involving accumError - it's computationally
cheap and at least one listener claims it improves quality. Three
"blind" listeners could give you meaningful statistical data.

> With your change, you are only messing around the lowest bit in the extra
> fractional part.

Now I'm surprised that it actually works.

> If you have any questions about the current code, just ask.

How to call sbc_encode(), sbc_decode() and again sbc_encode() in a
single function? It seems they cannot use the same instance of sbc_t
struct. I tried to estimate the amount of attenuation for adaptive
clipping prevention (obviously this solution is suboptimal but can be
a good starting point).


Regards

--
Sebastian Olter

Re: [PATCH] Re: A2DP quality (bluetooth-alsa)

[qduaty]

> 2012/3/26 Siarhei Siamashka <siarhei.siamashka@gmail.com>:
>> Floating point code does not need any rounding coefficient.

I'm very likely to agreed that I was wrong with initializing t1
elements to 1, but seemingly it helps with quality, especially in
conjunction with accumError. So either I discovered something new, or
I repeatedly "hear" my changes in C code instead of actual difference.
Maybe I should look at sbcdec output.

Regarding the bitpool:

>> Do you have a hcidump log before and after patching bluez for the part
>> where A2DP settings are negotiated?

Actually I just forced bluez to use higher bitpool for SBC than
negotiated. This is the hcidump after the change, but it does not
differ from what has been before.

> ACL data: handle 1 flags 0x02 dlen 20
    L2CAP(d): cid 0x0040 len 16 [psm 25]
      AVDTP(s): Capabilities rsp: transaction 4 nsp 0x00
        Media Transport
        Media Codec - SBC
          16kHz 32kHz 44.1kHz 48kHz
          Mono DualChannel Stereo JointStereo
          4 8 12 16 Blocks
          4 8 Subbands
          SNR Loudness
          Bitpool Range 2-53
        Content Protection
          02 00
< ACL data: handle 1 flags 0x02 dlen 18
    L2CAP(d): cid 0x0060 len 14 [psm 25]
      AVDTP(s): Set config cmd: transaction 5 nsp 0x00
        ACP SEID 1 - INT SEID 1
        Media Transport
        Media Codec - SBC
          44.1kHz
          Stereo
          16 Blocks
          8 Subbands
          Loudness
          Bitpool Range 46-46

The headset reports bltpool range of 2-53 and bluez reports 46-46, but
according to Blueman, it streams at 99 KB/s. Also, I removed calls to
default_bitpool(), which is now reported as unused function, and
increased MAX_BITPOOL everywhere. It does not change anything.

When bitpool is not set in ./asoundrc, Bluez responds with the same
values it received from the device. There is a difference between
Bluez and WinMo, which first sends its own capabilities and then asks
the device (see my previous hcidump). I don't know what BH-503
answers, but it's possible that it adapts to higher limits of the
phone and reports higher bitpools than 53, or the phone overrides
negotiated settings and streams at any bitpool it likes. I tested it
with bitpools up to 250 and it always worked. Contrary to that, Bluez
copies the (fake or backward-compatible) value of 53 that was received
from the device and limits itself instead of reporting its real
capabilities.

--
Sebastian Olter

Re: [PATCH] Re: A2DP quality (bluetooth-alsa)

[Siarhei Siamashka]

[-- Attachment #1: Type: text/plain, Size: 3835 bytes --]

On Tue, Mar 27, 2012 at 4:29 AM, qduaty <qduaty@gmail.com> wrote:
>> 2012/3/26 Siarhei Siamashka <siarhei.siamashka@gmail.com>:
>>> Floating point code does not need any rounding coefficient.
>
> I'm very likely to agreed that I was wrong with initializing t1
> elements to 1, but seemingly it helps with quality, especially in
> conjunction with accumError. So either I discovered something new, or
> I repeatedly "hear" my changes in C code instead of actual difference.

I think Double Blind listening Test is necessary, otherwise everything
is very subjective:
    http://www.hydrogenaudio.org/forums/index.php?showtopic=16295

> Maybe I should look at sbcdec output.

You could also look at fixing sbc decoder, it still does have some
real audio quality problems :)

> Regarding the bitpool:
>
>>> Do you have a hcidump log before and after patching bluez for the part
>>> where A2DP settings are negotiated?
>
> Actually I just forced bluez to use higher bitpool for SBC than
> negotiated. This is the hcidump after the change, but it does not
> differ from what has been before.
>
>> ACL data: handle 1 flags 0x02 dlen 20
>    L2CAP(d): cid 0x0040 len 16 [psm 25]
>      AVDTP(s): Capabilities rsp: transaction 4 nsp 0x00
>        Media Transport
>        Media Codec - SBC
>          16kHz 32kHz 44.1kHz 48kHz
>          Mono DualChannel Stereo JointStereo
>          4 8 12 16 Blocks
>          4 8 Subbands
>          SNR Loudness
>          Bitpool Range 2-53
>        Content Protection
>          02 00
> < ACL data: handle 1 flags 0x02 dlen 18
>    L2CAP(d): cid 0x0060 len 14 [psm 25]
>      AVDTP(s): Set config cmd: transaction 5 nsp 0x00
>        ACP SEID 1 - INT SEID 1
>        Media Transport
>        Media Codec - SBC
>          44.1kHz
>          Stereo
>          16 Blocks
>          8 Subbands
>          Loudness
>          Bitpool Range 46-46
>
> The headset reports bltpool range of 2-53 and bluez reports 46-46, but
> according to Blueman, it streams at 99 KB/s

This is getting really confusing. If I run blueman, it reports ~40
KB/s for bitpool 53, ~48 KB/s for bitpool 64 and ~36 KB/s for bitpool
46 (if I add 'bitpool "46"' line to my .asoundrc).

> Also, I removed calls to
> default_bitpool(), which is now reported as unused function, and
> increased MAX_BITPOOL everywhere. It does not change anything.

I have attached two test patches.

The first patch adds debug prints to syslog from sbc encoder, just to
clearly confirm the real bitpool used by the encoder. If the encoder
is encoding data with some certain bitpool settings and you hear
something from your headset, then there is no way for the bitpool to
be magically different from that.

The second patch removes default_bitpool() limitation, and this boosts
bitpool from 53 to 64 for me. Confirmed by sbc encoder debug prints
from my first patch, hcidump log and blueman streaming rate
statistics.

> When bitpool is not set in ./asoundrc, Bluez responds with the same
> values it received from the device. There is a difference between
> Bluez and WinMo, which first sends its own capabilities and then asks
> the device (see my previous hcidump). I don't know what BH-503
> answers, but it's possible that it adapts to higher limits of the
> phone and reports higher bitpools than 53, or the phone overrides
> negotiated settings and streams at any bitpool it likes. I tested it
> with bitpools up to 250 and it always worked. Contrary to that, Bluez
> copies the (fake or backward-compatible) value of 53 that was received
> from the device and limits itself instead of reporting its real
> capabilities.

-- 
Best regards,
Siarhei Siamashka

[-- Attachment #2: 0001-HACK-report-bitpool-to-syslog-from-sbc-encoder.patch --]
[-- Type: text/x-patch, Size: 1116 bytes --]

From 94a7494f063b3e22c8e2c3556a021c5315598e86 Mon Sep 17 00:00:00 2001
From: Siarhei Siamashka <siarhei.siamashka@gmail.com>
Date: Tue, 27 Mar 2012 10:03:00 +0300
Subject: [PATCH 1/2] HACK: report bitpool to syslog from sbc encoder

---
 sbc/sbc.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/sbc/sbc.c b/sbc/sbc.c
index c5015ab..0273035 100644
--- a/sbc/sbc.c
+++ b/sbc/sbc.c
@@ -42,6 +42,7 @@
 #include <stdlib.h>
 #include <sys/types.h>
 #include <limits.h>
+#include <syslog.h>
 
 #include "sbc_math.h"
 #include "sbc_tables.h"
@@ -1060,9 +1061,11 @@ ssize_t sbc_encode(sbc_t *sbc, const void *input, size_t input_len,
 
 		sbc_encoder_init(&priv->enc_state, &priv->frame);
 		priv->init = 1;
+		syslog(LOG_USER | LOG_INFO, "sbc_encode: bitpool set to %d\n", sbc->bitpool);
 	} else if (priv->frame.bitpool != sbc->bitpool) {
 		priv->frame.length = sbc_get_frame_length(sbc);
 		priv->frame.bitpool = sbc->bitpool;
+		syslog(LOG_USER | LOG_INFO, "sbc_encode: bitpool changed to %d\n", sbc->bitpool);
 	}
 
 	/* input must be large enough to encode a complete frame */
-- 
1.7.3.4


[-- Attachment #3: 0002-HACK-remove-default_bitpool-limitation.patch --]
[-- Type: text/x-patch, Size: 1424 bytes --]

From bbd5102a185d8c80b3668864618f34359c2d2139 Mon Sep 17 00:00:00 2001
From: Siarhei Siamashka <siarhei.siamashka@gmail.com>
Date: Tue, 27 Mar 2012 10:10:27 +0300
Subject: [PATCH 2/2] HACK: remove default_bitpool limitation

---
 audio/a2dp.c          |    3 +--
 audio/pcm_bluetooth.c |    4 +---
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/audio/a2dp.c b/audio/a2dp.c
index 623a0bc..8153819 100644
--- a/audio/a2dp.c
+++ b/audio/a2dp.c
@@ -1839,8 +1839,7 @@ static gboolean select_sbc_params(struct sbc_codec_cap *cap,
 		cap->allocation_method = SBC_ALLOCATION_SNR;
 
 	min_bitpool = MAX(MIN_BITPOOL, supported->min_bitpool);
-	max_bitpool = MIN(default_bitpool(cap->frequency, cap->channel_mode),
-							supported->max_bitpool);
+	max_bitpool = MAX(min_bitpool, supported->max_bitpool);
 
 	cap->min_bitpool = min_bitpool;
 	cap->max_bitpool = max_bitpool;
diff --git a/audio/pcm_bluetooth.c b/audio/pcm_bluetooth.c
index 85566a3..9130e35 100644
--- a/audio/pcm_bluetooth.c
+++ b/audio/pcm_bluetooth.c
@@ -609,9 +609,7 @@ static int bluetooth_a2dp_init(struct bluetooth_data *data,
 		min_bitpool = max_bitpool = cfg->bitpool;
 	else {
 		min_bitpool = MAX(MIN_BITPOOL, cap->min_bitpool);
-		max_bitpool = MIN(default_bitpool(cap->frequency,
-					cap->channel_mode),
-					cap->max_bitpool);
+		max_bitpool = MAX(min_bitpool, cap->max_bitpool);
 	}
 
 	cap->min_bitpool = min_bitpool;
-- 
1.7.3.4

[PATCH] Re: A2DP quality (bluetooth-alsa)

[qduaty]

[-- Attachment #1: Type: text/plain, Size: 1267 bytes --]

ALSA bitpool patch is attached.

2012/3/27 Siarhei Siamashka <siarhei.siamashka@gmail.com>:
> The first patch adds debug prints to syslog from sbc encoder, just to
> clearly confirm the real bitpool used by the encoder.

It obviously worked:

Mar 27 17:49:41 machina-turbo pulseaudio[2355]: sbc_encode: bitpool set to 138

> The second patch removes default_bitpool() limitation, and this boosts
> bitpool from 53 to 64 for me.

I found it was my device that prevented Bluez from using higher
bitpools. Bluez assumes that requesting a wider range of bitpools from
a device will cause an error, and indeed, my device cannot be
configured with a bitpool range wider than 2-53. So the only way to
achieve higher bitrates is to force them.

There is (was) a rather serious problem in the ALSA module, when it
tried to set bitpool exceding device capabilities, it ended with an
error and next tries, even with a valid bitpool, resulted in "Invalid
seid 0" in syslog and required bluetooth service to be restarted. With
my patch, bluez only uses the configured bitpool for sbc, without
sending it to the device. So, any bitpool value not exceeding true
device and adapter capabilities will work (and these can be much
higher than negotiated).


Regards
--

Sebastian Olter

[-- Attachment #2: alsa_use_arbitrary_bitpool.diff --]
[-- Type: text/x-diff, Size: 1991 bytes --]

--- bluez-4.99-orig/audio/pcm_bluetooth.c	2011-12-21 23:53:54.000000000 +0100
+++ bluez-4.99/audio/pcm_bluetooth.c	2012-03-28 01:20:41.977829080 +0200
@@ -78,7 +78,7 @@
 # define MAX(x, y) ((x) > (y) ? (x) : (y))
 #endif
 
-#define MAX_BITPOOL 64
+#define MAX_BITPOOL 250
 #define MIN_BITPOOL 2
 
 /* adapted from glibc sys/time.h timersub() macro */
@@ -606,13 +606,12 @@
 		cap->allocation_method = BT_A2DP_ALLOCATION_SNR;
 
 	if (cfg->has_bitpool)
-		min_bitpool = max_bitpool = cfg->bitpool;
-	else {
-		min_bitpool = MAX(MIN_BITPOOL, cap->min_bitpool);
-		max_bitpool = MIN(default_bitpool(cap->frequency,
-					cap->channel_mode),
-					cap->max_bitpool);
-	}
+		data->a2dp.sbc.bitpool = cfg->bitpool;
+	else /* Initialize to MIN_BITPOOL in order to replace it with negotiated maximum */
+		data->a2dp.sbc.bitpool = MIN_BITPOOL;
+
+	min_bitpool = MAX(MIN_BITPOOL, cap->min_bitpool);
+	max_bitpool = MIN(MAX_BITPOOL, cap->max_bitpool);
 
 	cap->min_bitpool = min_bitpool;
 	cap->max_bitpool = max_bitpool;
@@ -623,11 +622,15 @@
 static void bluetooth_a2dp_setup(struct bluetooth_a2dp *a2dp)
 {
 	sbc_capabilities_t active_capabilities = a2dp->sbc_capabilities;
+	int stored_bitpool = a2dp->sbc.bitpool;
 
 	if (a2dp->sbc_initialized)
 		sbc_reinit(&a2dp->sbc, 0);
 	else
 		sbc_init(&a2dp->sbc, 0);
+
+	/* restore configured (or minimum) bitpool */
+	a2dp->sbc.bitpool = stored_bitpool;
 	a2dp->sbc_initialized = 1;
 
 	if (active_capabilities.frequency & BT_SBC_SAMPLING_FREQ_16000)
@@ -682,7 +685,10 @@
 		break;
 	}
 
-	a2dp->sbc.bitpool = active_capabilities.max_bitpool;
+	/* Increase the bitpool to allowed maximum, if it was not configured */
+	if(a2dp->sbc.bitpool <= MIN_BITPOOL)
+	    a2dp->sbc.bitpool = active_capabilities.max_bitpool;
+ 
 	a2dp->codesize = sbc_get_codesize(&a2dp->sbc);
 	a2dp->count = sizeof(struct rtp_header) + sizeof(struct rtp_payload);
 }
@@ -1714,7 +1720,7 @@
 		goto failed;
 
 	bluetooth_parse_capabilities(data, rsp);
-
+	
 	return 0;
 
 failed:

Re: [PATCH] Re: A2DP quality (bluetooth-alsa)

[Siarhei Siamashka]

On Wed, Mar 28, 2012 at 2:28 AM, qduaty <qduaty@gmail.com> wrote:
> ALSA bitpool patch is attached.
>
> 2012/3/27 Siarhei Siamashka <siarhei.siamashka@gmail.com>:
>> The first patch adds debug prints to syslog from sbc encoder, just to
>> clearly confirm the real bitpool used by the encoder.
>
> It obviously worked:
>
> Mar 27 17:49:41 machina-turbo pulseaudio[2355]: sbc_encode: bitpool set to 138
>
>> The second patch removes default_bitpool() limitation, and this boosts
>> bitpool from 53 to 64 for me.
>
> I found it was my device that prevented Bluez from using higher
> bitpools. Bluez assumes that requesting a wider range of bitpools from
> a device will cause an error, and indeed, my device cannot be
> configured with a bitpool range wider than 2-53. So the only way to
> achieve higher bitrates is to force them.

I guess this depends on the headset. Your patch hacks around and
allows you to use higher bitpool with your Nokia BH-503 headset. With
my Logitech FreePulse headset, your patch does not seem to break audio
playback, but on the other hand such radical changes are not needed
(just removing default_bitpool() is enough to be able to set bitpool
to 128 via ./asoundrc). For somebody else your patch might possibly
even break audio playback if their headset does not like to be forced.

> There is (was) a rather serious problem in the ALSA module, when it
> tried to set bitpool exceding device capabilities, it ended with an
> error and next tries, even with a valid bitpool, resulted in "Invalid
> seid 0" in syslog and required bluetooth service to be restarted. With
> my patch, bluez only uses the configured bitpool for sbc, without
> sending it to the device. So, any bitpool value not exceeding true
> device and adapter capabilities will work (and these can be much
> higher than negotiated).

Apparently this works a bit different in my case and I don't get the
same "Invalid seid 0" error.

In any case, what we have here is that (some?) A2DP headsets can
support a lot higher SBC bitpool settings than they are normally
reporting. Now the big question is how to probe for this information
without violating negotiation protocol in any way and not causing any
regressions. The next question is how to decide which bitpool to use
by default, because in my case bitpool 128 occasionally causes some
skips in music playback which did not happen with lower bitpool
settings.

Could you provide a short summary for this whole discussion? The
discussion has been changing from bitpool negotiation to SBC codec
improvements back and forth. And now it's becoming hard to follow and
see which problems are still relevant and need to be solved. Also do
you want to work on some code yourself or mostly trying to escalate
the problems?

A2DP is in the list of ideas for BlueZ GSoC, so maybe that's a good
chance to get some improvements implemented:
http://www.bluez.org/development/gsoc/gsoc-ideas-list-2012/

-- 
Best regards,
Siarhei Siamashka

Re: [PATCH] Re: A2DP quality (bluetooth-alsa)

[qduaty]

2012/4/2 Siarhei Siamashka <siarhei.siamashka@gmail.com>:
> Your patch hacks around and
> allows you to use higher bitpool with your Nokia BH-503 headset. With
> my Logitech FreePulse headset, your patch does not seem to break audio
> playback, but on the other hand such radical changes are not needed
> (just removing default_bitpool() is enough to be able to set bitpool
> to 128 via ./asoundrc). For somebody else your patch might possibly
> even break audio playback if their headset does not like to be forced.

This means your headset is capable of decoding at higher bitpool than
it claims, as well as mine. So there is 50% probability that all A2DP
headsets support higher bitpools than they report.

My patch won't break anyone's playback unless they specify extremely
high bitpool values; most users won't do it due to the lack of
knowledge that it's possible (and has any purpose), and the rest will
know exactly what they're doing.

The solution works as well in pulseaudio (though, it is hard to
specify the bitpool and I have it currently hardcoded in
setup_a2dp()).

> In any case, what we have here is that (some?) A2DP headsets can
> support a lot higher SBC bitpool settings than they are normally
> reporting. Now the big question is how to probe for this information
> without violating negotiation protocol in any way and not causing any
> regressions. The next question is how to decide which bitpool to use
> by default

Just use default_bitpool(), but let users override it. I didn't post a
modified patch because it seemed obvious, but here it is (a
modification of bluetooth_a2dp_setup()):

	/* If the bitpool was not configured, use default */
	if(a2dp->sbc.bitpool <= MIN_BITPOOL)
	    a2dp->sbc.bitpool = MIN(default_bitpool(active_capabilities.frequency,
					active_capabilities.channel_mode),
					active_capabilities.max_bitpool);

With this change, Bluez will use default_bitpool() unless a user
specifies a bitpool in the config file.

Regarding the detection - I think any way of probing the maximum is
likely to be magic and can mysteriously fail with an unsupported
device.

>, because in my case bitpool 128 occasionally causes some
> skips in music playback which did not happen with lower bitpool
> settings.

L2CAP speed tests that I've seen, show that no 2.0+EDR device is
capable of transmitting even 2 Mb/s. In my case, a bitrate of 800kbps
(bitpool=138, 44100 Hz, 16 blocks) is achievable in A2DP.

> Could you provide a short summary for this whole discussion? The
> discussion has been changing from bitpool negotiation to SBC codec
> improvements back and forth. And now it's becoming hard to follow and
> see which problems are still relevant and need to be solved. Also do
> you want to work on some code yourself or mostly trying to escalate
> the problems?
>
> A2DP is in the list of ideas for BlueZ GSoC, so maybe that's a good
> chance to get some improvements implemented:
> http://www.bluez.org/development/gsoc/gsoc-ideas-list-2012/

Ok. This is the summary.
1. It was found that Bluez can occasionally limit available bitpool
because some devices report a narrow bitpool range that does not
reflect their real capabilities. A solution was proposed by means of
encoding at a higher bitpool than negotiated, and it was confirmed to
both work and not break compatibility, unless it is misused.
2. It was found that Bluez audio sink has quality problems even on
high bitpools. An SBC encoder fix was proposed, which (re)enables
floating point processing. Tests are needed to confirm whether it
improves quality in all cases.
3. It was (previously) found and now confirmed that reducing volume in
SBC encoder by a factor of 0.7-0.8 improves quality by eliminating
audible effects of clipping in the decoder.

For myself, I'm likely to be done with it. I've already solved the
problems that prevented me from using A2DP on Linux. Further work
would require more time and I cannot benefit from it (but students
obviously can).

My proposals:
1. Introduce a less restrictive input format for the audio sink, such
as float32. Currently it supports only S16LE, which is not suitable
for audio processing (ALSA softvol is a known example).
2. Find out what quality problems the SBC decoder has and fix them.
Possibly introduce floating point output to eliminate clipping.
3. Implement proper (adaptive?) clipping prevention in the SBC encoder.


Regards

--
Sebastian Olter

Re: [PATCH] Re: A2DP quality (bluetooth-alsa)

[Siarhei Siamashka]

On Tue, Apr 3, 2012 at 3:56 AM, qduaty <qduaty@gmail.com> wrote:
> 2012/4/2 Siarhei Siamashka <siarhei.siamashka@gmail.com>:
>> Could you provide a short summary for this whole discussion? The
>> discussion has been changing from bitpool negotiation to SBC codec
>> improvements back and forth. And now it's becoming hard to follow and
>> see which problems are still relevant and need to be solved. Also do
>> you want to work on some code yourself or mostly trying to escalate
>> the problems?
>>
>> A2DP is in the list of ideas for BlueZ GSoC, so maybe that's a good
>> chance to get some improvements implemented:
>> http://www.bluez.org/development/gsoc/gsoc-ideas-list-2012/
>
> Ok. This is the summary.
> 1. It was found that Bluez can occasionally limit available bitpool
> because some devices report a narrow bitpool range that does not
> reflect their real capabilities. A solution was proposed by means of
> encoding at a higher bitpool than negotiated, and it was confirmed to
> both work and not break compatibility, unless it is misused.
> 2. It was found that Bluez audio sink has quality problems even on
> high bitpools. An SBC encoder fix was proposed, which (re)enables
> floating point processing. Tests are needed to confirm whether it
> improves quality in all cases.
> 3. It was (previously) found and now confirmed that reducing volume in
> SBC encoder by a factor of 0.7-0.8 improves quality by eliminating
> audible effects of clipping in the decoder.

OK, thanks.

> For myself, I'm likely to be done with it. I've already solved the
> problems that prevented me from using A2DP on Linux. Further work
> would require more time and I cannot benefit from it (but students
> obviously can).

Fair enough.

> My proposals:
> 1. Introduce a less restrictive input format for the audio sink, such
> as float32. Currently it supports only S16LE, which is not suitable
> for audio processing (ALSA softvol is a known example).
> 2. Find out what quality problems the SBC decoder has and fix them.
> Possibly introduce floating point output to eliminate clipping.
> 3. Implement proper (adaptive?) clipping prevention in the SBC encoder.

-- 
Best regards,
Siarhei Siamashka

Re: [PATCH] Re: A2DP quality (bluetooth-alsa)

[Luiz Augusto von Dentz]

Hi Siarhei,

On Wed, Apr 4, 2012 at 11:12 AM, Siarhei Siamashka
<siarhei.siamashka@gmail.com> wrote:
> On Tue, Apr 3, 2012 at 3:56 AM, qduaty <qduaty@gmail.com> wrote:
>> 2012/4/2 Siarhei Siamashka <siarhei.siamashka@gmail.com>:
>>> Could you provide a short summary for this whole discussion? The
>>> discussion has been changing from bitpool negotiation to SBC codec
>>> improvements back and forth. And now it's becoming hard to follow and
>>> see which problems are still relevant and need to be solved. Also do
>>> you want to work on some code yourself or mostly trying to escalate
>>> the problems?
>>>
>>> A2DP is in the list of ideas for BlueZ GSoC, so maybe that's a good
>>> chance to get some improvements implemented:
>>> http://www.bluez.org/development/gsoc/gsoc-ideas-list-2012/
>>
>> Ok. This is the summary.
>> 1. It was found that Bluez can occasionally limit available bitpool
>> because some devices report a narrow bitpool range that does not
>> reflect their real capabilities. A solution was proposed by means of
>> encoding at a higher bitpool than negotiated, and it was confirmed to
>> both work and not break compatibility, unless it is misused.
>> 2. It was found that Bluez audio sink has quality problems even on
>> high bitpools. An SBC encoder fix was proposed, which (re)enables
>> floating point processing. Tests are needed to confirm whether it
>> improves quality in all cases.
>> 3. It was (previously) found and now confirmed that reducing volume in
>> SBC encoder by a factor of 0.7-0.8 improves quality by eliminating
>> audible effects of clipping in the decoder.
>
> OK, thanks.
>
>> For myself, I'm likely to be done with it. I've already solved the
>> problems that prevented me from using A2DP on Linux. Further work
>> would require more time and I cannot benefit from it (but students
>> obviously can).
>
> Fair enough.
>
>> My proposals:
>> 1. Introduce a less restrictive input format for the audio sink, such
>> as float32. Currently it supports only S16LE, which is not suitable
>> for audio processing (ALSA softvol is a known example).
>> 2. Find out what quality problems the SBC decoder has and fix them.
>> Possibly introduce floating point output to eliminate clipping.
>> 3. Implement proper (adaptive?) clipping prevention in the SBC encoder.

Is there anyone working on this proposals? Specially the decoder seems
to need some optimization as it consumes quite a bit of CPU (6-8% on
my i7) and some people seems to notice some quality issues.


-- 
Luiz Augusto von Dentz

Re: [PATCH] Re: A2DP quality (bluetooth-alsa)

[Siarhei Siamashka]

On Tue, 31 Jul 2012 13:09:19 +0300
Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:

> Hi Siarhei,
> 
> On Wed, Apr 4, 2012 at 11:12 AM, Siarhei Siamashka
> <siarhei.siamashka@gmail.com> wrote:
> > On Tue, Apr 3, 2012 at 3:56 AM, qduaty <qduaty@gmail.com> wrote:
> >> 2012/4/2 Siarhei Siamashka <siarhei.siamashka@gmail.com>:
> >>> Could you provide a short summary for this whole discussion? The
> >>> discussion has been changing from bitpool negotiation to SBC codec
> >>> improvements back and forth. And now it's becoming hard to follow
> >>> and see which problems are still relevant and need to be solved.
> >>> Also do you want to work on some code yourself or mostly trying
> >>> to escalate the problems?
> >>>
> >>> A2DP is in the list of ideas for BlueZ GSoC, so maybe that's a
> >>> good chance to get some improvements implemented:
> >>> http://www.bluez.org/development/gsoc/gsoc-ideas-list-2012/
> >>
> >> Ok. This is the summary.
> >> 1. It was found that Bluez can occasionally limit available bitpool
> >> because some devices report a narrow bitpool range that does not
> >> reflect their real capabilities. A solution was proposed by means
> >> of encoding at a higher bitpool than negotiated, and it was
> >> confirmed to both work and not break compatibility, unless it is
> >> misused. 2. It was found that Bluez audio sink has quality
> >> problems even on high bitpools. An SBC encoder fix was proposed,
> >> which (re)enables floating point processing. Tests are needed to
> >> confirm whether it improves quality in all cases.
> >> 3. It was (previously) found and now confirmed that reducing
> >> volume in SBC encoder by a factor of 0.7-0.8 improves quality by
> >> eliminating audible effects of clipping in the decoder.
> >
> > OK, thanks.
> >
> >> For myself, I'm likely to be done with it. I've already solved the
> >> problems that prevented me from using A2DP on Linux. Further work
> >> would require more time and I cannot benefit from it (but students
> >> obviously can).
> >
> > Fair enough.
> >
> >> My proposals:
> >> 1. Introduce a less restrictive input format for the audio sink,
> >> such as float32. Currently it supports only S16LE, which is not
> >> suitable for audio processing (ALSA softvol is a known example).
> >> 2. Find out what quality problems the SBC decoder has and fix them.
> >> Possibly introduce floating point output to eliminate clipping.
> >> 3. Implement proper (adaptive?) clipping prevention in the SBC
> >> encoder.

Sorry for a late reply, I have missed this one.
 
> Is there anyone working on this proposals?

These were all qduaty's ideas. If (s)he is not working on this stuff,
then I guess nobody does.

> Specially the decoder seems to need some optimization as it consumes
> quite a bit of CPU (6-8% on my i7) 

Yes, there are lots of low hanging fruits. For example, getting rid of
integer division and better bitstream reading should provide a
significant performance boost for the decoder. I might do that
eventually.

> and some people seems to notice some quality issues.

Are there some links to more information? Or to some audio quality
tests, which can be confirmed/reproduced?

The precision of fixed point calculations in the decoder is not
very good, so it definitely can be improved. But it should not be
too bad either.

The audio quality should be quite good for the encoder. Except for the
clamping problem, which may be happening on the decoder side with some
heavily compressed audio data and a poor/buggy A2DP headset. This
can be workarounded by reducing audio volume in the encoder by 20-30%.
The SBC encoder may get these extra volume adjustment settings, but the
big unresolved question is who and when is going to enable it? Would it
be some configuration file (pulseaudio/alsa/anything else)? Or would it
be some automatic heuristics kicking in for known bad headsets?

-- 
Best regards,
Siarhei Siamashka

Re: [PATCH] Re: A2DP quality (bluetooth-alsa)

[Siarhei Siamashka]

On Fri, Sep 28, 2012 at 1:20 AM, Siarhei Siamashka
<siarhei.siamashka@gmail.com> wrote:
> On Tue, 31 Jul 2012 13:09:19 +0300
> Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
>
>> Hi Siarhei,

By the way, thanks for moving libsbc into its own library. This
definitely looks like the right thing.

-- 
Best regards,
Siarhei Siamashka

Re: A2DP quality (bluetooth-alsa)

[Siarhei Siamashka]

On Wed, Mar 14, 2012 at 2:53 PM, qduaty <qduaty@gmail.com> wrote:
> 2012/3/13 Siarhei Siamashka <siarhei.siamashka@gmail.com>:
>> Are you now comparing streaming of A2DP audio from your phone to your
>> PC (using bluez A2DP sink) and to your bluetooth headset?
>
> I assume that the phone streams data to the headset (whose session is
> hard to dump on Windows mobile) with bitpool = 112, ie. the same it
> fails to set in Bluez and I'm saying it gives good results in terms of
> perceiveable quality, from which Bluez cannot take advantage due to
> its inherent bitpool=53 limitation.
>
> Anyway, I did some additional listening tests (by listening to Bluez
> SBC on an A2DP headset). SBC_HIGH_PRECISION alone (without your hack)
> only slightly improves quality. Also I finally managed to force higher
> bitpool in ALSA (patch attached). This makes further improvement in
> quality, at least on headsets that do not choke on higher bitpools.

Do you have a hcidump log before and after patching bluez for the part
where A2DP settings are negotiated? For normal bluez 4.99, this is
what I get with Logitech FreePulse headset:

> ACL data: handle 11 flags 0x02 dlen 12
    L2CAP(d): cid 0x0040 len 8 [psm 25]
      AVDTP(s): Discover rsp: transaction 11 nsp 0x00
        ACP SEID 1 - Audio Sink
        ACP SEID 2 - Audio Sink
        ACP SEID 3 - Audio Sink
< ACL data: handle 11 flags 0x00 dlen 7
    L2CAP(d): cid 0x0200 len 3 [psm 25]
      AVDTP(s): Capabilities cmd: transaction 12 nsp 0x00
        ACP SEID 1
> HCI Event: Number of Completed Packets (0x13) plen 5
    handle 11 packets 1
> ACL data: handle 11 flags 0x02 dlen 20
    L2CAP(d): cid 0x0040 len 16 [psm 25]
      AVDTP(s): Capabilities rsp: transaction 12 nsp 0x00
        Media Transport
        Media Codec - SBC
          16kHz 32kHz 44.1kHz 48kHz
          Mono DualChannel Stereo JointStereo
          8 12 16 Blocks
          4 8 Subbands
          SNR Loudness
          Bitpool Range 2-64
        Content Protection
          02 00
< ACL data: handle 11 flags 0x00 dlen 7
    L2CAP(d): cid 0x0200 len 3 [psm 25]
      AVDTP(s): Capabilities cmd: transaction 13 nsp 0x00
        ACP SEID 2
> ACL data: handle 11 flags 0x02 dlen 20
    L2CAP(d): cid 0x0040 len 16 [psm 25]
      AVDTP(s): Capabilities rsp: transaction 13 nsp 0x00
        Media Transport
        Media Codec - SBC
          16kHz 32kHz 44.1kHz 48kHz
          Mono DualChannel Stereo JointStereo
          4 Blocks
          4 8 Subbands
          SNR Loudness
          Bitpool Range 2-32
        Content Protection
          02 00
< ACL data: handle 11 flags 0x00 dlen 7
    L2CAP(d): cid 0x0200 len 3 [psm 25]
      AVDTP(s): Capabilities cmd: transaction 14 nsp 0x00
        ACP SEID 3
> HCI Event: Number of Completed Packets (0x13) plen 5
    handle 11 packets 2
> ACL data: handle 11 flags 0x02 dlen 24
    L2CAP(d): cid 0x0040 len 20 [psm 25]
      AVDTP(s): Capabilities rsp: transaction 14 nsp 0x00
        Media Transport
        Media Codec - non-A2DP
          27 00 00 00 01 00 F7 77 02 56 00 FA
< ACL data: handle 11 flags 0x00 dlen 18
    L2CAP(d): cid 0x0200 len 14 [psm 25]
      AVDTP(s): Set config cmd: transaction 15 nsp 0x00
        ACP SEID 1 - INT SEID 1
        Media Transport
        Media Codec - SBC
          44.1kHz
          JointStereo
          16 Blocks
          8 Subbands
          Loudness
          Bitpool Range 2-53

The headset reports 2-64 bitpool capabilities, but bluez sets bitpool
to 53 because it does not use anything higher than suggested by
'default_bitpool' function (actually in two places):
    "max_bitpool = MIN(default_bitpool(cap->frequency,
cap->channel_mode), supported->max_bitpool);"
If I change this code, bitpool 64 can be used, however my ears are not
trained enough to hear any differences between 53 and 64. By the way,
high bitpool settings are causing problems for some users, so not
everything is so clear:
    https://bugs.launchpad.net/ubuntu/+source/bluez/+bug/424215

Would it be a good idea if you come up with a clean patch, making
bitpool selection strategy tunable via a configuration file? Or
implement some kind of heuristics based on whether EDR is supported or
not?

-- 
Best regards,
Siarhei Siamashka