2023-08-04 19:57:19

by Arkadiusz Bokowy

[permalink] [raw]
Subject: [PATCH SBC 1/1] sbc: Fix mSBC reinitialization

Commit 10310fb has introduced a function for mSBC reinitialization.
However, it did not set msbc private flag, which is checked e.g. by
sbc_set_defaults() and sbc_get_frame_length() functions.

This commit fixes this, so now after reinitialization this library
properly packs/unpacks mSBC frames and the sbc_get_frame_length()
function returns correct frame length.
---
sbc/sbc.c | 71 ++++++++++++++++++++++++++-----------------------------
1 file changed, 33 insertions(+), 38 deletions(-)

diff --git a/sbc/sbc.c b/sbc/sbc.c
index d059906..c35b564 100644
--- a/sbc/sbc.c
+++ b/sbc/sbc.c
@@ -1010,17 +1010,13 @@ static void sbc_set_defaults(sbc_t *sbc, unsigned long flags)
{
struct sbc_priv *priv = sbc->priv;

- if (priv->msbc) {
- priv->pack_frame = msbc_pack_frame;
- priv->unpack_frame = msbc_unpack_frame;
- } else {
- priv->pack_frame = sbc_pack_frame;
- priv->unpack_frame = sbc_unpack_frame;
- }
+ priv->pack_frame = sbc_pack_frame;
+ priv->unpack_frame = sbc_unpack_frame;

sbc->flags = flags;
sbc->frequency = SBC_FREQ_44100;
sbc->mode = SBC_MODE_STEREO;
+ sbc->allocation = SBC_AM_LOUDNESS;
sbc->subbands = SBC_SB_8;
sbc->blocks = SBC_BLK_16;
sbc->bitpool = 32;
@@ -1033,6 +1029,30 @@ static void sbc_set_defaults(sbc_t *sbc, unsigned long flags)
#endif
}

+static void sbc_set_defaults_msbc(sbc_t *sbc, unsigned long flags)
+{
+ struct sbc_priv *priv = sbc->priv;
+
+ priv->msbc = true;
+ priv->pack_frame = msbc_pack_frame;
+ priv->unpack_frame = msbc_unpack_frame;
+
+ sbc->flags = flags;
+ sbc->frequency = SBC_FREQ_16000;
+ sbc->mode = SBC_MODE_MONO;
+ sbc->allocation = SBC_AM_LOUDNESS;
+ sbc->subbands = SBC_SB_8;
+ sbc->blocks = MSBC_BLOCKS;
+ sbc->bitpool = 26;
+#if __BYTE_ORDER == __LITTLE_ENDIAN
+ sbc->endian = SBC_LE;
+#elif __BYTE_ORDER == __BIG_ENDIAN
+ sbc->endian = SBC_BE;
+#else
+#error "Unknown byte order"
+#endif
+}
+
SBC_EXPORT int sbc_init(sbc_t *sbc, unsigned long flags)
{
if (!sbc)
@@ -1056,33 +1076,13 @@ SBC_EXPORT int sbc_init(sbc_t *sbc, unsigned long flags)

SBC_EXPORT int sbc_init_msbc(sbc_t *sbc, unsigned long flags)
{
- struct sbc_priv *priv;
-
- if (!sbc)
- return -EIO;
-
- memset(sbc, 0, sizeof(sbc_t));
-
- sbc->priv_alloc_base = malloc(sizeof(struct sbc_priv) + SBC_ALIGN_MASK);
- if (!sbc->priv_alloc_base)
- return -ENOMEM;
-
- sbc->priv = (void *) (((uintptr_t) sbc->priv_alloc_base +
- SBC_ALIGN_MASK) & ~((uintptr_t) SBC_ALIGN_MASK));
-
- memset(sbc->priv, 0, sizeof(struct sbc_priv));
-
- priv = sbc->priv;
- priv->msbc = true;
+ int err;

- sbc_set_defaults(sbc, flags);
+ err = sbc_init(sbc, flags);
+ if (err < 0)
+ return err;

- sbc->frequency = SBC_FREQ_16000;
- sbc->blocks = MSBC_BLOCKS;
- sbc->subbands = SBC_SB_8;
- sbc->mode = SBC_MODE_MONO;
- sbc->allocation = SBC_AM_LOUDNESS;
- sbc->bitpool = 26;
+ sbc_set_defaults_msbc(sbc, flags);

return 0;
}
@@ -1095,12 +1095,7 @@ SBC_EXPORT int sbc_reinit_msbc(sbc_t *sbc, unsigned long flags)
if (err < 0)
return err;

- sbc->frequency = SBC_FREQ_16000;
- sbc->blocks = MSBC_BLOCKS;
- sbc->subbands = SBC_SB_8;
- sbc->mode = SBC_MODE_MONO;
- sbc->allocation = SBC_AM_LOUDNESS;
- sbc->bitpool = 26;
+ sbc_set_defaults_msbc(sbc, flags);

return 0;
}
--
2.39.2



2023-10-06 15:22:02

by Barnabás Pőcze

[permalink] [raw]
Subject: Re: [PATCH SBC 1/1] sbc: Fix mSBC reinitialization

Hi


2023. augusztus 4., péntek 21:25 keltezéssel, Arkadiusz Bokowy <[email protected]> írta:

> Commit 10310fb has introduced a function for mSBC reinitialization.
> However, it did not set msbc private flag, which is checked e.g. by
> sbc_set_defaults() and sbc_get_frame_length() functions.
>
> This commit fixes this, so now after reinitialization this library
> properly packs/unpacks mSBC frames and the sbc_get_frame_length()
> function returns correct frame length.

I would like to see this issue fixed. Is there any way we can move
forward? If there are any issues preventing the merging of this
change, I would be happy to address them if Arkadiusz has no time.


Regards,
Barnabás Pőcze


> ---
> sbc/sbc.c | 71 ++++++++++++++++++++++++++-----------------------------
> 1 file changed, 33 insertions(+), 38 deletions(-)
>
> diff --git a/sbc/sbc.c b/sbc/sbc.c
> index d059906..c35b564 100644
> --- a/sbc/sbc.c
> +++ b/sbc/sbc.c
> @@ -1010,17 +1010,13 @@ static void sbc_set_defaults(sbc_t *sbc, unsigned long flags)
> {
> struct sbc_priv *priv = sbc->priv;
>
> - if (priv->msbc) {
> - priv->pack_frame = msbc_pack_frame;
> - priv->unpack_frame = msbc_unpack_frame;
> - } else {
> - priv->pack_frame = sbc_pack_frame;
> - priv->unpack_frame = sbc_unpack_frame;
> - }
> + priv->pack_frame = sbc_pack_frame;
> + priv->unpack_frame = sbc_unpack_frame;
>
> sbc->flags = flags;
> sbc->frequency = SBC_FREQ_44100;
> sbc->mode = SBC_MODE_STEREO;
> + sbc->allocation = SBC_AM_LOUDNESS;
> sbc->subbands = SBC_SB_8;
> sbc->blocks = SBC_BLK_16;
> sbc->bitpool = 32;
> @@ -1033,6 +1029,30 @@ static void sbc_set_defaults(sbc_t *sbc, unsigned long flags)
> #endif
> }
>
> +static void sbc_set_defaults_msbc(sbc_t *sbc, unsigned long flags)
> +{
> + struct sbc_priv *priv = sbc->priv;
> +
> + priv->msbc = true;
> + priv->pack_frame = msbc_pack_frame;
> + priv->unpack_frame = msbc_unpack_frame;
> +
> + sbc->flags = flags;
> + sbc->frequency = SBC_FREQ_16000;
> + sbc->mode = SBC_MODE_MONO;
> + sbc->allocation = SBC_AM_LOUDNESS;
> + sbc->subbands = SBC_SB_8;
> + sbc->blocks = MSBC_BLOCKS;
> + sbc->bitpool = 26;
> +#if __BYTE_ORDER == __LITTLE_ENDIAN
> + sbc->endian = SBC_LE;
> +#elif __BYTE_ORDER == __BIG_ENDIAN
> + sbc->endian = SBC_BE;
> +#else
> +#error "Unknown byte order"
> +#endif
> +}
> +
> SBC_EXPORT int sbc_init(sbc_t *sbc, unsigned long flags)
> {
> if (!sbc)
> @@ -1056,33 +1076,13 @@ SBC_EXPORT int sbc_init(sbc_t *sbc, unsigned long flags)
>
> SBC_EXPORT int sbc_init_msbc(sbc_t *sbc, unsigned long flags)
> {
> - struct sbc_priv *priv;
> -
> - if (!sbc)
> - return -EIO;
> -
> - memset(sbc, 0, sizeof(sbc_t));
> -
> - sbc->priv_alloc_base = malloc(sizeof(struct sbc_priv) + SBC_ALIGN_MASK);
> - if (!sbc->priv_alloc_base)
> - return -ENOMEM;
> -
> - sbc->priv = (void *) (((uintptr_t) sbc->priv_alloc_base +
> - SBC_ALIGN_MASK) & ~((uintptr_t) SBC_ALIGN_MASK));
> -
> - memset(sbc->priv, 0, sizeof(struct sbc_priv));
> -
> - priv = sbc->priv;
> - priv->msbc = true;
> + int err;
>
> - sbc_set_defaults(sbc, flags);
> + err = sbc_init(sbc, flags);
> + if (err < 0)
> + return err;
>
> - sbc->frequency = SBC_FREQ_16000;
> - sbc->blocks = MSBC_BLOCKS;
> - sbc->subbands = SBC_SB_8;
> - sbc->mode = SBC_MODE_MONO;
> - sbc->allocation = SBC_AM_LOUDNESS;
> - sbc->bitpool = 26;
> + sbc_set_defaults_msbc(sbc, flags);
>
> return 0;
> }
> @@ -1095,12 +1095,7 @@ SBC_EXPORT int sbc_reinit_msbc(sbc_t *sbc, unsigned long flags)
> if (err < 0)
> return err;
>
> - sbc->frequency = SBC_FREQ_16000;
> - sbc->blocks = MSBC_BLOCKS;
> - sbc->subbands = SBC_SB_8;
> - sbc->mode = SBC_MODE_MONO;
> - sbc->allocation = SBC_AM_LOUDNESS;
> - sbc->bitpool = 26;
> + sbc_set_defaults_msbc(sbc, flags);
>
> return 0;
> }
> --
> 2.39.2
>