[FFmpeg-devel] [PATCH 1/7] avcodec/wavpack: Fix leak and segfault on reallocation error

[FFmpeg-devel] [PATCH 1/7] avcodec/wavpack: Fix leak and segfault on reallocation error

[Andreas Rheinhardt]

av_realloc_f() frees the buffer it is given on allocation
failure. But in this case, the buffer is an array of
ownership pointers, causing leaks on error. Furthermore,
the count of pointers is unchanged on error and the codec's
close function uses it to free said ownership pointers,
causing a NPD.
This is a regression since 46412a8935e4632b2460988bfce4152c7dccce22.

Fix this by switching to av_realloc_array().

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
Actually, one only needs one WavpackFrameContext at a time, given
that this decoder does not do proper slice threading.
Alternatively, one could implement proper slice threading.

 libavcodec/wavpack.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/libavcodec/wavpack.c b/libavcodec/wavpack.c
index 7e60a1456a..36bd4662e8 100644
--- a/libavcodec/wavpack.c
+++ b/libavcodec/wavpack.c
@@ -973,9 +973,11 @@ static inline int wv_unpack_mono(WavpackFrameContext *s, GetBitContext *gb,
 
 static av_cold int wv_alloc_frame_context(WavpackContext *c)
 {
-    c->fdec = av_realloc_f(c->fdec, c->fdec_num + 1, sizeof(*c->fdec));
-    if (!c->fdec)
+    WavpackFrameContext **fdec = av_realloc_array(c->fdec, c->fdec_num + 1, sizeof(*c->fdec));
+
+    if (!fdec)
         return -1;
+    c->fdec = fdec;
 
     c->fdec[c->fdec_num] = av_mallocz(sizeof(**c->fdec));
     if (!c->fdec[c->fdec_num])
-- 
2.40.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 2/7] avcodec/dsd: Use double for LUTs

[Andreas Rheinhardt]

Needed to make DSD->PCM conversion bitexact across arches.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/dsd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/dsd.c b/libavcodec/dsd.c
index e039302c99..ec63a706f6 100644
--- a/libavcodec/dsd.c
+++ b/libavcodec/dsd.c
@@ -68,7 +68,7 @@ static const double htaps[HTAPS] = {
      3.423230509967409e-07,  1.244182214744588e-07,  3.130441005359396e-08
 };
 
-static float ctables[CTABLES][256];
+static double ctables[CTABLES][256];
 
 static av_cold void dsd_ctables_tableinit(void)
 {
-- 
2.40.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 3/7] fate/wavpack: Add test for DSD

[Andreas Rheinhardt]

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
Tested on aarch64, alpha, arm, mips, ppc and riscv via qemu;
of course also tested on x64 and x86.

 tests/fate/wavpack.mak              | 3 +++
 tests/ref/fate/wavpack-lossless-dsd | 1 +
 2 files changed, 4 insertions(+)
 create mode 100644 tests/ref/fate/wavpack-lossless-dsd

diff --git a/tests/fate/wavpack.mak b/tests/fate/wavpack.mak
index c684d93f99..41e1b0afad 100644
--- a/tests/fate/wavpack.mak
+++ b/tests/fate/wavpack.mak
@@ -18,6 +18,9 @@ fate-wavpack-lossless-32bit: CMD = md5pipe -i $(TARGET_SAMPLES)/wavpack/lossless
 FATE_WAVPACK_F32 += fate-wavpack-lossless-float
 fate-wavpack-lossless-float: CMD = md5pipe -i $(TARGET_SAMPLES)/wavpack/lossless/32bit_float-partial.wv -f f32le -af aresample
 
+FATE_WAVPACK_F32 += fate-wavpack-lossless-dsd
+fate-wavpack-lossless-dsd: CMD = md5pipe -i $(TARGET_SAMPLES)/wavpack/lossless/dsd.wv -f f32le -af aresample
+
 # lossy
 
 FATE_WAVPACK_S8 += fate-wavpack-lossy-8bit
diff --git a/tests/ref/fate/wavpack-lossless-dsd b/tests/ref/fate/wavpack-lossless-dsd
new file mode 100644
index 0000000000..eba76df518
--- /dev/null
+++ b/tests/ref/fate/wavpack-lossless-dsd
@@ -0,0 +1 @@
+0b33207f1ec7e47333878cb8420c21ce
-- 
2.40.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 4/7] avcodec/wavpack: Only initialize DSD data when encountering DSD

[Andreas Rheinhardt]

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/wavpack.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/libavcodec/wavpack.c b/libavcodec/wavpack.c
index 36bd4662e8..c96c8e0583 100644
--- a/libavcodec/wavpack.c
+++ b/libavcodec/wavpack.c
@@ -1049,8 +1049,6 @@ static av_cold int wavpack_decode_init(AVCodecContext *avctx)
     if (!s->curr_frame.f || !s->prev_frame.f)
         return AVERROR(ENOMEM);
 
-    ff_init_dsd_data();
-
     return 0;
 }
 
@@ -1529,6 +1527,7 @@ static int wavpack_decode_block(AVCodecContext *avctx, int block_no,
                 return ret;
             }
             ff_thread_release_ext_buffer(&wc->curr_frame);
+            ff_init_dsd_data();
         }
         av_channel_layout_copy(&avctx->ch_layout, &new_ch_layout);
         avctx->sample_rate         = new_samplerate;
-- 
2.40.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 5/7] avcodec/dsddec: Fix decoding LSBF samples

[Andreas Rheinhardt]

ff_dsd2pcm_translate() works internally by converting LSBF input
to MSBF upon reading; its buffer is therefore always MSBF
and should therefore be initialized with MSBF silence;
but this is not true since e3d8963c3cb5b8cd31460dd9b3b9dba2a2343bf5
which this patch effectively reverts.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/dsddec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/dsddec.c b/libavcodec/dsddec.c
index 22009c70ef..2bb2e73b75 100644
--- a/libavcodec/dsddec.c
+++ b/libavcodec/dsddec.c
@@ -56,7 +56,7 @@ static av_cold int decode_init(AVCodecContext *avctx)
     if (!s)
         return AVERROR(ENOMEM);
 
-    silence = avctx->codec_id == AV_CODEC_ID_DSD_LSBF || avctx->codec_id == AV_CODEC_ID_DSD_LSBF_PLANAR ? DSD_SILENCE_REVERSED : DSD_SILENCE;
+    silence = DSD_SILENCE;
     for (i = 0; i < avctx->ch_layout.nb_channels; i++) {
         s[i].pos = 0;
         memset(s[i].buf, silence, sizeof(s[i].buf));
-- 
2.40.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 5/7] avcodec/dsddec: Fix decoding LSBF samples

[Peter Ross]

[-- Attachment #1.1: Type: text/plain, Size: 1480 bytes --]

On Tue, Apr 02, 2024 at 03:37:06AM +0200, Andreas Rheinhardt wrote:
> ff_dsd2pcm_translate() works internally by converting LSBF input
> to MSBF upon reading; its buffer is therefore always MSBF
> and should therefore be initialized with MSBF silence;
> but this is not true since e3d8963c3cb5b8cd31460dd9b3b9dba2a2343bf5
> which this patch effectively reverts.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
>  libavcodec/dsddec.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavcodec/dsddec.c b/libavcodec/dsddec.c
> index 22009c70ef..2bb2e73b75 100644
> --- a/libavcodec/dsddec.c
> +++ b/libavcodec/dsddec.c
> @@ -56,7 +56,7 @@ static av_cold int decode_init(AVCodecContext *avctx)
>      if (!s)
>          return AVERROR(ENOMEM);
>  
> -    silence = avctx->codec_id == AV_CODEC_ID_DSD_LSBF || avctx->codec_id == AV_CODEC_ID_DSD_LSBF_PLANAR ? DSD_SILENCE_REVERSED : DSD_SILENCE;
> +    silence = DSD_SILENCE;
>      for (i = 0; i < avctx->ch_layout.nb_channels; i++) {
>          s[i].pos = 0;
>          memset(s[i].buf, silence, sizeof(s[i].buf));
> -- 

ok.

with this patch, DSD_SILENCE, DSD_SILENCE_REVERSED macros and even the silence
var are no longer neccessary.

i suggest reverting the memset line and attached comment back to way it was in
the initial commit (5f4f9ee99f4e9ab980bb18475009c701ba47a74f).

-- Peter
(A907 E02F A6E5 0CD2 34CD 20D2 6760 79C5 AC40 DD6B)

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 5/7] avcodec/dsddec: Fix decoding LSBF samples

[Andreas Rheinhardt]

Peter Ross:
> On Tue, Apr 02, 2024 at 03:37:06AM +0200, Andreas Rheinhardt wrote:
>> ff_dsd2pcm_translate() works internally by converting LSBF input
>> to MSBF upon reading; its buffer is therefore always MSBF
>> and should therefore be initialized with MSBF silence;
>> but this is not true since e3d8963c3cb5b8cd31460dd9b3b9dba2a2343bf5
>> which this patch effectively reverts.
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
>> ---
>>  libavcodec/dsddec.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/libavcodec/dsddec.c b/libavcodec/dsddec.c
>> index 22009c70ef..2bb2e73b75 100644
>> --- a/libavcodec/dsddec.c
>> +++ b/libavcodec/dsddec.c
>> @@ -56,7 +56,7 @@ static av_cold int decode_init(AVCodecContext *avctx)
>>      if (!s)
>>          return AVERROR(ENOMEM);
>>  
>> -    silence = avctx->codec_id == AV_CODEC_ID_DSD_LSBF || avctx->codec_id == AV_CODEC_ID_DSD_LSBF_PLANAR ? DSD_SILENCE_REVERSED : DSD_SILENCE;
>> +    silence = DSD_SILENCE;
>>      for (i = 0; i < avctx->ch_layout.nb_channels; i++) {
>>          s[i].pos = 0;
>>          memset(s[i].buf, silence, sizeof(s[i].buf));
>> -- 
> 
> ok.
> 
> with this patch, DSD_SILENCE, DSD_SILENCE_REVERSED macros and even the silence
> var are no longer neccessary.
> 
> i suggest reverting the memset line and attached comment back to way it was in
> the initial commit (5f4f9ee99f4e9ab980bb18475009c701ba47a74f).
> 

I see you haven't made it till the next commit yet.

- Andreas

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 6/7] avcodec/dsd: Hoist branch out of loop

[Andreas Rheinhardt]

This is possible by not converting from LSBF to MSBF; instead
add LSBF LUTs.
This approach necessitates reversing the initial values.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/dsd.c    | 12 ++++++++----
 libavcodec/dsddec.c |  3 ++-
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/libavcodec/dsd.c b/libavcodec/dsd.c
index ec63a706f6..1093c5e2dd 100644
--- a/libavcodec/dsd.c
+++ b/libavcodec/dsd.c
@@ -68,7 +68,8 @@ static const double htaps[HTAPS] = {
      3.423230509967409e-07,  1.244182214744588e-07,  3.130441005359396e-08
 };
 
-static double ctables[CTABLES][256];
+static double ctables_lsbf[CTABLES][256];
+static double ctables_msbf[CTABLES][256];
 
 static av_cold void dsd_ctables_tableinit(void)
 {
@@ -81,8 +82,10 @@ static av_cold void dsd_ctables_tableinit(void)
             for (t = 0; t < CTABLES; ++t)
                 acc[t] += sign * htaps[t * 8 + m];
         }
-        for (t = 0; t < CTABLES; ++t)
-            ctables[CTABLES - 1 - t][e] = acc[t];
+        for (t = 0; t < CTABLES; ++t) {
+            ctables_msbf[CTABLES - 1 - t][e] = acc[t];
+            ctables_lsbf[CTABLES - 1 - t][ff_reverse[e]] = acc[t];
+        }
     }
 }
 
@@ -100,13 +103,14 @@ void ff_dsd2pcm_translate(DSDContext* s, size_t samples, int lsbf,
     unsigned pos, i;
     uint8_t* p;
     double sum;
+    const double (*const ctables)[256] = lsbf ? ctables_lsbf : ctables_msbf;
 
     pos = s->pos;
 
     memcpy(buf, s->buf, sizeof(buf));
 
     while (samples-- > 0) {
-        buf[pos] = lsbf ? ff_reverse[*src] : *src;
+        buf[pos] = *src;
         src += src_stride;
 
         p = buf + ((pos - CTABLES) & FIFOMASK);
diff --git a/libavcodec/dsddec.c b/libavcodec/dsddec.c
index 2bb2e73b75..dd1ea83784 100644
--- a/libavcodec/dsddec.c
+++ b/libavcodec/dsddec.c
@@ -56,7 +56,8 @@ static av_cold int decode_init(AVCodecContext *avctx)
     if (!s)
         return AVERROR(ENOMEM);
 
-    silence = DSD_SILENCE;
+    silence = avctx->codec_id == AV_CODEC_ID_DSD_LSBF_PLANAR ||
+              avctx->codec_id == AV_CODEC_ID_DSD_LSBF ? DSD_SILENCE_REVERSED : DSD_SILENCE;
     for (i = 0; i < avctx->ch_layout.nb_channels; i++) {
         s[i].pos = 0;
         memset(s[i].buf, silence, sizeof(s[i].buf));
-- 
2.40.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 7/7] avcodec/wavpack: Remove always-false check

[Andreas Rheinhardt]

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/wavpack.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/libavcodec/wavpack.c b/libavcodec/wavpack.c
index c96c8e0583..73d69d66ff 100644
--- a/libavcodec/wavpack.c
+++ b/libavcodec/wavpack.c
@@ -1095,11 +1095,6 @@ static int wavpack_decode_block(AVCodecContext *avctx, int block_no,
     }
 
     s = wc->fdec[block_no];
-    if (!s) {
-        av_log(avctx, AV_LOG_ERROR, "Context for block %d is not present\n",
-               block_no);
-        return AVERROR_INVALIDDATA;
-    }
 
     memset(s->decorr, 0, MAX_TERMS * sizeof(Decorr));
     memset(s->ch, 0, sizeof(s->ch));
-- 
2.40.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/7] avcodec/wavpack: Fix leak and segfault on reallocation error

[Andreas Rheinhardt]

Andreas Rheinhardt:
> av_realloc_f() frees the buffer it is given on allocation
> failure. But in this case, the buffer is an array of
> ownership pointers, causing leaks on error. Furthermore,
> the count of pointers is unchanged on error and the codec's
> close function uses it to free said ownership pointers,
> causing a NPD.
> This is a regression since 46412a8935e4632b2460988bfce4152c7dccce22.
> 
> Fix this by switching to av_realloc_array().
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
> Actually, one only needs one WavpackFrameContext at a time, given
> that this decoder does not do proper slice threading.
> Alternatively, one could implement proper slice threading.
> 
>  libavcodec/wavpack.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/libavcodec/wavpack.c b/libavcodec/wavpack.c
> index 7e60a1456a..36bd4662e8 100644
> --- a/libavcodec/wavpack.c
> +++ b/libavcodec/wavpack.c
> @@ -973,9 +973,11 @@ static inline int wv_unpack_mono(WavpackFrameContext *s, GetBitContext *gb,
>  
>  static av_cold int wv_alloc_frame_context(WavpackContext *c)
>  {
> -    c->fdec = av_realloc_f(c->fdec, c->fdec_num + 1, sizeof(*c->fdec));
> -    if (!c->fdec)
> +    WavpackFrameContext **fdec = av_realloc_array(c->fdec, c->fdec_num + 1, sizeof(*c->fdec));
> +
> +    if (!fdec)
>          return -1;
> +    c->fdec = fdec;
>  
>      c->fdec[c->fdec_num] = av_mallocz(sizeof(**c->fdec));
>      if (!c->fdec[c->fdec_num])

Will apply this patchset tomorrow unless there are objections.

- Andreas

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".