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

[Tanu Kaskinen]

On Fri, 2019-07-05 at 15:02 +0200, Pali Rohár wrote:
> SBC codec reset() method may fail, so propagate this failure to caller.
> ---
>  src/modules/bluetooth/a2dp-codec-api.h       |  4 ++--
>  src/modules/bluetooth/a2dp-codec-sbc.c       |  5 +++--
>  src/modules/bluetooth/module-bluez5-device.c | 18 ++++++++++++------
>  3 files changed, 17 insertions(+), 10 deletions(-)
> 
> diff --git a/src/modules/bluetooth/a2dp-codec-api.h b/src/modules/bluetooth/a2dp-codec-api.h
> index 881cc659b..bc4844596 100644
> --- a/src/modules/bluetooth/a2dp-codec-api.h
> +++ b/src/modules/bluetooth/a2dp-codec-api.h
> @@ -69,8 +69,8 @@ typedef struct pa_a2dp_codec {
>      void *(*init)(bool for_encoding, bool for_backchannel, const uint8_t *config_buffer, uint8_t config_size, pa_sample_spec *sample_spec);
>      /* Deinitialize and release codec info data in codec_info */
>      void (*deinit)(void *codec_info);
> -    /* Reset internal state of codec info data in codec_info */
> -    void (*reset)(void *codec_info);
> +    /* Reset internal state of codec info data in codec_info, returns non-zero on failure */
> +    int (*reset)(void *codec_info);
>  
>      /* Get read block size for codec, it is minimal size of buffer
>       * needed to decode read_link_mtu bytes of encoded data */
> diff --git a/src/modules/bluetooth/a2dp-codec-sbc.c b/src/modules/bluetooth/a2dp-codec-sbc.c
> index e2db91b63..f57c7b01a 100644
> --- a/src/modules/bluetooth/a2dp-codec-sbc.c
> +++ b/src/modules/bluetooth/a2dp-codec-sbc.c
> @@ -466,20 +466,21 @@ static void set_bitpool(struct sbc_info *sbc_info, uint8_t bitpool) {
>      pa_log_debug("Bitpool has changed to %u", sbc_info->sbc.bitpool);
>  }
>  
> -static void reset(void *codec_info) {
> +static int reset(void *codec_info) {
>      struct sbc_info *sbc_info = (struct sbc_info *) codec_info;
>      int ret;
>  
>      ret = sbc_reinit(&sbc_info->sbc, 0);
>      if (ret != 0) {
>          pa_log_error("SBC reinitialization failed: %d", ret);
> -        return;
> +        return -1;
>      }
>  
>      /* sbc_reinit() sets also default parameters, so reset them back */
>      set_params(sbc_info);
>  
>      sbc_info->seq_num = 0;
> +    return 0;
>  }
>  
>  static size_t get_block_size(void *codec_info, size_t link_mtu) {
> diff --git a/src/modules/bluetooth/module-bluez5-device.c b/src/modules/bluetooth/module-bluez5-device.c
> index 140ddb8fb..0b63cd44a 100644
> --- a/src/modules/bluetooth/module-bluez5-device.c
> +++ b/src/modules/bluetooth/module-bluez5-device.c
> @@ -762,22 +762,24 @@ static void transport_config_mtu(struct userdata *u) {
>  }
>  
>  /* Run from I/O thread */
> -static void setup_stream(struct userdata *u) {
> +static bool setup_stream(struct userdata *u) {

Failure should be indicated using a negative int rather than a boolean
value. I see a bool return value for setup_transport_and_stream() has
previously somehow passed review, which is probably why you opted for
bool here, but could you change this anyway?

-- 
Tanu

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