Re: [PATCH v13 05/10] bluetooth: Add A2DP aptX and aptX HD codecs support

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

 



On 02.12.19 15:47, Tanu Kaskinen wrote:
On Mon, 2019-12-02 at 13:00 +0100, Pali Rohár wrote:
On Sunday 01 December 2019 12:24:07 Georg Chini wrote:
On 30.11.19 23:39, Pali Rohár wrote:
On Saturday 30 November 2019 22:43:47 Georg Chini wrote:
On 06.10.19 19:58, Pali Rohár wrote:
This patch provides support for aptX and aptX HD codecs in bluetooth A2DP
profile. It uses open source LGPLv2.1+ licensed libopenaptx library which
can be found at https://github.com/pali/libopenaptx.

aptX for s24 stereo samples provides fixed 6:1 compression ratio and
bitrate 352.8 kbit/s, aptX HD provides fixed 4:1 compression ratio and
bitrate 529.2 kbit/s.

According to soundexpert research, aptX codec used in bluetooth A2DP is no
better than SBC High Quality settings. And you cannot hear difference
between aptX and SBC High Quality, aptX is just a copper-less overpriced
audio cable.

aptX HD is high-bitrate version of aptX. It has clearly noticeable increase
in sound quality (not dramatic though taking into account the increase in
bitrate).

http://soundexpert.org/news/-/blogs/audio-quality-of-bluetooth-aptx
One general remark: I would consider passing the a2dp_codec as argument
to the API functions. This way you can avoid having to create one function
per codec variant. Instead you can use the vendor ID/codec ID or just the
codec name as a key to a "case" or "if" instruction to distinguish between
different codec variants. I think especially in the SBC case the code would
be better readable. In this patch you could avoid duplicating functions
if you can read the vendor and codec ID from the a2dp_codec instead of
hard coding them.
I do not think that passing codec structure into every function is a big
win. In your suggestion, instead of N functions (one for each codec) you
would have just one function with N if-then-else-else-else... branches,
one branch for each codec. Currently common parts for all codecs is
already in sub-function with suffix _common (so code is not duplicated).
I do not think it would produce less code, but the code would be
easier to read.
Now you have to check which codec variant uses which function,
then you have to check this function for the arguments of the
common function and then you can look what the common function
does.
With passing the codec that could be avoided: All (or at least most)
codec variants would use the same function
That is not fully truth. SBC XQ variant needs to check that remote side
supports configuration for SBC XQ. SBC LQ needs to check that remote
side supports configuration for SBC LQ.

If variants would be same, then one function for checking would be
enough. But they are variants of just because codecs are not same.

and you could directly
see what happens for each variant in this function. Effectively, multiple
functions would be replaced by if or case instructions as you already
said, but for me that would be the purpose of the change.

Maybe we should ask Tanu for an opinion? I can live with the current
approach but would prefer passing the codec.
Both approaches are (if implemented) correctly should provide equivalent
set of features. So there is no functionality lost when choosing either
my our your implementation variant.

So maybe Tanu (or somebody else) could give us some comments? If my
currently implemented variant of code API is OK or if Georg's variant
should be used and therefore changing existing code and code in my
patches to it...
I'm fine with either. I vote for keeping the current API, since that
means less work.

OK, so let's keep it.

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




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

  Powered by Linux