Re: [pulseaudio-discuss] [PATCH v12 00/13] Bluetooth A2DP codecs

2019-07-18 Thread Tanu Kaskinen 
On Fri, 2019-07-05 at 15:02 +0200, Pali Rohár wrote:
> Changes in v12:
> * Renamed SBC UHQ to SBC XQ to match naming convention from
>   
> http://soundexpert.org/articles/-/blogs/audio-quality-of-sbc-xq-bluetooth-audio-codec
> * Throw error when PA receive fragmented SBC frame
> * Log "Couldn't find SO_TIMESTAMP" warning message only once
> * Update comment for SBC bitpool selection
> * Add more checks for return values from libsbc
> * Propagate error value from sbc_reinit() to module-bluez5-device.c
> * Add checks for SBC frame count
> * Completely rework/fix MTU, buffer sizes and return values of encode/decode 
> methods
> 
> Pali Rohár (13):
>   bluetooth: Fix usage of MTU, buffer sizes and return values of
> encode/decode methods
>   bluetooth: Change A2DP codec API of reset() method to indicate failure
>   bluetooth: Fix usage of RTP structures in SBC codec
>   bluetooth: Implement reading SO_TIMESTAMP for A2DP source
>   bluetooth: Print SO_TIMESTAMP warning for SCO source only once
>   bluetooth: Parse remote timestamp from A2DP RTP packets when available
>   bluetooth: Set initial A2DP profile which bluez already activated
>   bluetooth: Add A2DP aptX and aptX HD codecs support
>   bluetooth: Add A2DP FastStream codec support
>   bluetooth: Add more variants of SBC codec
>   bluetooth: policy: Reflect a2dp profile names
>   bluetooth: Implement A2DP codec switching and backchannel support
>   bluetooth: policy: Treat bi-directional A2DP profiles as suitable for
> VOIP
> 
>  configure.ac|  36 ++
>  src/Makefile.am |   8 +
>  src/modules/bluetooth/a2dp-codec-api.h  |  14 +-
>  src/modules/bluetooth/a2dp-codec-aptx.c | 448 +
>  src/modules/bluetooth/a2dp-codec-faststream.c   | 454 +
>  src/modules/bluetooth/a2dp-codec-sbc.c  | 806 
> +++-
>  src/modules/bluetooth/a2dp-codec-util.c |  26 +-
>  src/modules/bluetooth/bluez5-util.c | 476 +-
>  src/modules/bluetooth/bluez5-util.h |  39 +-
>  src/modules/bluetooth/meson.build   |   1 +
>  src/modules/bluetooth/module-bluetooth-policy.c | 127 ++--
>  src/modules/bluetooth/module-bluez5-device.c| 587 -
>  src/modules/bluetooth/module-bluez5-discover.c  |   3 +-
>  src/modules/bluetooth/rtp.h |  58 +-
>  14 files changed, 2644 insertions(+), 439 deletions(-)
>  create mode 100644 src/modules/bluetooth/a2dp-codec-aptx.c
>  create mode 100644 src/modules/bluetooth/a2dp-codec-faststream.c

Can you fix and resubmit the first three patches? Those are the ones we
want in the upcoming release.

-- 
Tanu

https://www.patreon.com/tanuk
https://liberapay.com/tanuk

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

Re: [pulseaudio-discuss] [PATCH v12 03/13] bluetooth: Fix usage of RTP structures in SBC codec

2019-07-18 Thread Tanu Kaskinen 
On Fri, 2019-07-05 at 15:02 +0200, Pali Rohár wrote:
> Rename struct rtp_payload to rtp_sbc_payload as it is specific for SBC
> codec payload.
> 
> Add proper checks for endianity in rtp.h header and use uint8_t type
> where appropriated.
> 
> Field frame_count is only 4 bit number, so add checks to prevent overflow.
> 
> And because is_fragmented field is not parsed by decoder there is no
> support for decoding fragmented SBC frames. So throw an error in this case.
> ---
>  src/modules/bluetooth/a2dp-codec-sbc.c | 35 ++--
>  src/modules/bluetooth/rtp.h| 58 
> +++---
>  2 files changed, 58 insertions(+), 35 deletions(-)

Looks good to me.

-- 
Tanu

https://www.patreon.com/tanuk
https://liberapay.com/tanuk

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

Re: [pulseaudio-discuss] [PATCH v12 02/13] bluetooth: Change A2DP codec API of reset() method to indicate failure

2019-07-18 Thread Tanu Kaskinen 
On Fri, 2019-07-05 at 15:02 +0200, Pali Rohár wrote:
>  /* Run from I/O thread */
> -static void setup_stream(struct userdata *u) {
> +static bool setup_stream(struct userdata *u) {
>  struct pollfd *pollfd;
>  int one;
>  
>  /* return if stream is already set up */
>  if (u->stream_setup_done)
> -return;
> +return true;
>  
>  pa_log_info("Transport %s resuming", u->transport->path);
>  
>  if (u->profile == PA_BLUETOOTH_PROFILE_A2DP_SINK) {
>  pa_assert(u->a2dp_codec);
> -u->a2dp_codec->reset(u->encoder_info);
> +if (u->a2dp_codec->reset(u->encoder_info) != 0)
> +return false;
>  } else if (u->profile == PA_BLUETOOTH_PROFILE_A2DP_SOURCE) {
>  pa_assert(u->a2dp_codec);
> -u->a2dp_codec->reset(u->decoder_info);
> +if (u->a2dp_codec->reset(u->decoder_info) != 0)

"< 0" is the usual convention rather than "!= 0".

(Sorry for this extra mail, I originally wrote this in the first review
mail, but it seems that before sending it, I trimmed too much from the
bottom of the email and accidentally deleted this bit.)

-- 
Tanu

https://www.patreon.com/tanuk
https://liberapay.com/tanuk

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