[FFmpeg-devel] [PATCH 1/2] avcodec/apedec: remove unused variable

[FFmpeg-devel] [PATCH 1/2] avcodec/apedec: remove unused variable

[Michael Niedermayer]

Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/apedec.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/libavcodec/apedec.c b/libavcodec/apedec.c
index a9d5eb7f33..8bd625ca05 100644
--- a/libavcodec/apedec.c
+++ b/libavcodec/apedec.c
@@ -145,8 +145,6 @@ typedef struct APEPredictor64 {
     uint64_t coeffsA[2][4];  ///< adaption coefficients
     uint64_t coeffsB[2][5];  ///< adaption coefficients
     int64_t historybuffer[HISTORY_SIZE + PREDICTOR_SIZE];
-
-    unsigned int sample_pos;
 } APEPredictor64;
 
 /** Decoder context */
@@ -860,8 +858,6 @@ static void init_predictor_decoder(APEContext *ctx)
     p64->lastA[0]   = p64->lastA[1]   = 0;
 
     p->sample_pos = 0;
-
-    p64->sample_pos = 0;
 }
 
 /** Get inverse sign of integer (-1 for positive, 1 for negative and 0 for zero) */
-- 
2.17.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/2] avcodec/apedec: Implement interim mode detection

[Michael Niedermayer]

Fixes: NoLegacy.ape
Found-by: Matt Ashland <mail@monkeysaudio.com>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/apedec.c | 106 +++++++++++++++++++++++++++++++++++---------
 1 file changed, 84 insertions(+), 22 deletions(-)

diff --git a/libavcodec/apedec.c b/libavcodec/apedec.c
index 8bd625ca05..249fc22e24 100644
--- a/libavcodec/apedec.c
+++ b/libavcodec/apedec.c
@@ -171,6 +171,9 @@ typedef struct APEContext {
     int32_t *decoded_buffer;
     int decoded_size;
     int32_t *decoded[MAX_CHANNELS];          ///< decoded data for each channel
+    int32_t *interim_buffer;
+    int interim_size;
+    int32_t *interim[MAX_CHANNELS];          ///< decoded data for each channel
     int blocks_per_loop;                     ///< maximum number of samples to decode for each call
 
     int16_t* filterbuf[APE_FILTER_LEVELS];   ///< filter memory
@@ -187,6 +190,7 @@ typedef struct APEContext {
     const uint8_t *ptr;                      ///< current position in frame data
 
     int error;
+    int interim_mode;
 
     void (*entropy_decode_mono)(struct APEContext *ctx, int blockstodecode);
     void (*entropy_decode_stereo)(struct APEContext *ctx, int blockstodecode);
@@ -223,6 +227,7 @@ static av_cold int ape_decode_close(AVCodecContext *avctx)
         av_freep(&s->filterbuf[i]);
 
     av_freep(&s->decoded_buffer);
+    av_freep(&s->interim_buffer);
     av_freep(&s->data);
     s->decoded_size = s->data_size = 0;
 
@@ -248,12 +253,15 @@ static av_cold int ape_decode_init(AVCodecContext *avctx)
     switch (s->bps) {
     case 8:
         avctx->sample_fmt = AV_SAMPLE_FMT_U8P;
+        s->interim_mode = 0;
         break;
     case 16:
         avctx->sample_fmt = AV_SAMPLE_FMT_S16P;
+        s->interim_mode = 0;
         break;
     case 24:
         avctx->sample_fmt = AV_SAMPLE_FMT_S32P;
+        s->interim_mode = -1;
         break;
     default:
         avpriv_request_sample(avctx,
@@ -1181,7 +1189,7 @@ static av_always_inline int predictor_update_filter(APEPredictor64 *p,
                                                     const int decoded, const int filter,
                                                     const int delayA,  const int delayB,
                                                     const int adaptA,  const int adaptB,
-                                                    int compression_level)
+                                                    int interim_mode)
 {
     int64_t predictionA, predictionB;
     int32_t sign;
@@ -1209,7 +1217,7 @@ static av_always_inline int predictor_update_filter(APEPredictor64 *p,
                   p->buf[delayB - 3] * p->coeffsB[filter][3] +
                   p->buf[delayB - 4] * p->coeffsB[filter][4];
 
-    if (compression_level < COMPRESSION_LEVEL_INSANE) {
+    if (interim_mode < 1) {
         predictionA = (int32_t)predictionA;
         predictionB = (int32_t)predictionB;
         p->lastA[filter] = decoded + ((int32_t)(predictionA + (predictionB >> 1)) >> 10);
@@ -1234,33 +1242,74 @@ static av_always_inline int predictor_update_filter(APEPredictor64 *p,
 
 static void predictor_decode_stereo_3950(APEContext *ctx, int count)
 {
-    APEPredictor64 *p = &ctx->predictor64;
-    int32_t *decoded0 = ctx->decoded[0];
-    int32_t *decoded1 = ctx->decoded[1];
+    APEPredictor64 *p_default = &ctx->predictor64;
+    APEPredictor64 p_interim;
+    int lcount = count;
+    int num_passes = 1;
 
     ape_apply_filters(ctx, ctx->decoded[0], ctx->decoded[1], count);
+    if (ctx->interim_mode == -1) {
+        p_interim = *p_default;
+        num_passes ++;
+        memcpy(ctx->interim[0], ctx->decoded[0], sizeof(*ctx->interim[0])*count);
+        memcpy(ctx->interim[1], ctx->decoded[1], sizeof(*ctx->interim[1])*count);
+    }
 
-    while (count--) {
-        /* Predictor Y */
-        *decoded0 = predictor_update_filter(p, *decoded0, 0, YDELAYA, YDELAYB,
-                                            YADAPTCOEFFSA, YADAPTCOEFFSB,
-                                            ctx->compression_level);
-        decoded0++;
-        *decoded1 = predictor_update_filter(p, *decoded1, 1, XDELAYA, XDELAYB,
-                                            XADAPTCOEFFSA, XADAPTCOEFFSB,
-                                            ctx->compression_level);
-        decoded1++;
+    for(int pass = 0; pass < num_passes; pass++) {
+        int32_t *decoded0, *decoded1;
+        int interim_mode = ctx->interim_mode > 0 || pass;
+        APEPredictor64 *p;
 
-        /* Combined */
-        p->buf++;
+        if (pass) {
+            p        = &p_interim;
+            decoded0 = ctx->interim[0];
+            decoded1 = ctx->interim[1];
+        } else {
+            p        = p_default;
+            decoded0 = ctx->decoded[0];
+            decoded1 = ctx->decoded[1];
+        }
+        p->buf = p->historybuffer;
+
+        count = lcount;
+        while (count--) {
+            /* Predictor Y */
+            int32_t a0 = predictor_update_filter(p, *decoded0, 0, YDELAYA, YDELAYB,
+                                                YADAPTCOEFFSA, YADAPTCOEFFSB,
+                                                interim_mode);
+            int32_t a1 = predictor_update_filter(p, *decoded1, 1, XDELAYA, XDELAYB,
+                                                XADAPTCOEFFSA, XADAPTCOEFFSB,
+                                                interim_mode);
+            *decoded0++ = a0;
+            *decoded1++ = a1;
+            if (num_passes > 1) {
+                int32_t left  = a1 - (unsigned)(a0 / 2);
+                int32_t right = left + a0;
+
+                if (FFMAX(FFABS(left), FFABS(right)) > (1<<23)) {
+                    ctx->interim_mode = !interim_mode;
+                    av_log(ctx->avctx, AV_LOG_VERBOSE, "Interim mode: %d\n", ctx->interim_mode);
+                    break;
+                }
+            }
 
-        /* Have we filled the history buffer? */
-        if (p->buf == p->historybuffer + HISTORY_SIZE) {
-            memmove(p->historybuffer, p->buf,
-                    PREDICTOR_SIZE * sizeof(*p->historybuffer));
-            p->buf = p->historybuffer;
+            /* Combined */
+            p->buf++;
+
+            /* Have we filled the history buffer? */
+            if (p->buf == p->historybuffer + HISTORY_SIZE) {
+                memmove(p->historybuffer, p->buf,
+                        PREDICTOR_SIZE * sizeof(*p->historybuffer));
+                p->buf = p->historybuffer;
+            }
         }
     }
+    if (num_passes > 1 && ctx->interim_mode > 0) {
+        memcpy(ctx->decoded[0], ctx->interim[0], sizeof(*ctx->interim[0])*lcount);
+        memcpy(ctx->decoded[1], ctx->interim[1], sizeof(*ctx->interim[1])*lcount);
+        *p_default = p_interim;
+        p_default->buf = p_default->historybuffer;
+    }
 }
 
 static void predictor_decode_mono_3950(APEContext *ctx, int count)
@@ -1590,6 +1639,19 @@ static int ape_decode_frame(AVCodecContext *avctx, AVFrame *frame,
     s->decoded[0] = s->decoded_buffer;
     s->decoded[1] = s->decoded_buffer + FFALIGN(blockstodecode, 8);
 
+    if (s->interim_mode < 0) {
+        av_fast_malloc(&s->interim_buffer, &s->interim_size, decoded_buffer_size);
+        if (!s->interim_buffer)
+            return AVERROR(ENOMEM);
+        memset(s->interim_buffer, 0, decoded_buffer_size);
+        s->interim[0] = s->interim_buffer;
+        s->interim[1] = s->interim_buffer + FFALIGN(blockstodecode, 8);
+    } else {
+        av_freep(&s->interim_buffer);
+        s->interim_size = 0;
+        memset(s->interim, 0, sizeof(s->interim));
+    }
+
     s->error=0;
 
     if ((s->channels == 1) || (s->frameflags & APE_FRAMECODE_PSEUDO_STEREO))
-- 
2.17.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 2/2] avcodec/apedec: Implement interim mode detection

[Paul B Mahol]

On Sat, Aug 26, 2023 at 6:54 PM Michael Niedermayer <michael@niedermayer.cc>
wrote:

> Fixes: NoLegacy.ape
> Found-by: Matt Ashland <mail@monkeysaudio.com>
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavcodec/apedec.c | 106 +++++++++++++++++++++++++++++++++++---------
>  1 file changed, 84 insertions(+), 22 deletions(-)
>
> diff --git a/libavcodec/apedec.c b/libavcodec/apedec.c
> index 8bd625ca05..249fc22e24 100644
> --- a/libavcodec/apedec.c
> +++ b/libavcodec/apedec.c
> @@ -171,6 +171,9 @@ typedef struct APEContext {
>      int32_t *decoded_buffer;
>      int decoded_size;
>      int32_t *decoded[MAX_CHANNELS];          ///< decoded data for each
> channel
> +    int32_t *interim_buffer;
> +    int interim_size;
> +    int32_t *interim[MAX_CHANNELS];          ///< decoded data for each
> channel
>      int blocks_per_loop;                     ///< maximum number of
> samples to decode for each call
>
>      int16_t* filterbuf[APE_FILTER_LEVELS];   ///< filter memory
> @@ -187,6 +190,7 @@ typedef struct APEContext {
>      const uint8_t *ptr;                      ///< current position in
> frame data
>
>      int error;
> +    int interim_mode;
>
>      void (*entropy_decode_mono)(struct APEContext *ctx, int
> blockstodecode);
>      void (*entropy_decode_stereo)(struct APEContext *ctx, int
> blockstodecode);
> @@ -223,6 +227,7 @@ static av_cold int ape_decode_close(AVCodecContext
> *avctx)
>          av_freep(&s->filterbuf[i]);
>
>      av_freep(&s->decoded_buffer);
> +    av_freep(&s->interim_buffer);
>      av_freep(&s->data);
>      s->decoded_size = s->data_size = 0;
>
> @@ -248,12 +253,15 @@ static av_cold int ape_decode_init(AVCodecContext
> *avctx)
>      switch (s->bps) {
>      case 8:
>          avctx->sample_fmt = AV_SAMPLE_FMT_U8P;
> +        s->interim_mode = 0;
>          break;
>      case 16:
>          avctx->sample_fmt = AV_SAMPLE_FMT_S16P;
> +        s->interim_mode = 0;
>          break;
>      case 24:
>          avctx->sample_fmt = AV_SAMPLE_FMT_S32P;
> +        s->interim_mode = -1;
>          break;
>      default:
>          avpriv_request_sample(avctx,
> @@ -1181,7 +1189,7 @@ static av_always_inline int
> predictor_update_filter(APEPredictor64 *p,
>                                                      const int decoded,
> const int filter,
>                                                      const int delayA,
> const int delayB,
>                                                      const int adaptA,
> const int adaptB,
> -                                                    int compression_level)
> +                                                    int interim_mode)
>  {
>      int64_t predictionA, predictionB;
>      int32_t sign;
> @@ -1209,7 +1217,7 @@ static av_always_inline int
> predictor_update_filter(APEPredictor64 *p,
>                    p->buf[delayB - 3] * p->coeffsB[filter][3] +
>                    p->buf[delayB - 4] * p->coeffsB[filter][4];
>
> -    if (compression_level < COMPRESSION_LEVEL_INSANE) {
> +    if (interim_mode < 1) {
>          predictionA = (int32_t)predictionA;
>          predictionB = (int32_t)predictionB;
>          p->lastA[filter] = decoded + ((int32_t)(predictionA +
> (predictionB >> 1)) >> 10);
> @@ -1234,33 +1242,74 @@ static av_always_inline int
> predictor_update_filter(APEPredictor64 *p,
>
>  static void predictor_decode_stereo_3950(APEContext *ctx, int count)
>  {
> -    APEPredictor64 *p = &ctx->predictor64;
> -    int32_t *decoded0 = ctx->decoded[0];
> -    int32_t *decoded1 = ctx->decoded[1];
> +    APEPredictor64 *p_default = &ctx->predictor64;
> +    APEPredictor64 p_interim;
> +    int lcount = count;
> +    int num_passes = 1;
>
>      ape_apply_filters(ctx, ctx->decoded[0], ctx->decoded[1], count);
> +    if (ctx->interim_mode == -1) {
> +        p_interim = *p_default;
> +        num_passes ++;
> +        memcpy(ctx->interim[0], ctx->decoded[0],
> sizeof(*ctx->interim[0])*count);
> +        memcpy(ctx->interim[1], ctx->decoded[1],
> sizeof(*ctx->interim[1])*count);
> +    }
>
> -    while (count--) {
> -        /* Predictor Y */
> -        *decoded0 = predictor_update_filter(p, *decoded0, 0, YDELAYA,
> YDELAYB,
> -                                            YADAPTCOEFFSA, YADAPTCOEFFSB,
> -                                            ctx->compression_level);
> -        decoded0++;
> -        *decoded1 = predictor_update_filter(p, *decoded1, 1, XDELAYA,
> XDELAYB,
> -                                            XADAPTCOEFFSA, XADAPTCOEFFSB,
> -                                            ctx->compression_level);
> -        decoded1++;
> +    for(int pass = 0; pass < num_passes; pass++) {
>

Please fix your style in patches.


> +        int32_t *decoded0, *decoded1;
> +        int interim_mode = ctx->interim_mode > 0 || pass;
> +        APEPredictor64 *p;
>
> -        /* Combined */
> -        p->buf++;
> +        if (pass) {
> +            p        = &p_interim;
> +            decoded0 = ctx->interim[0];
> +            decoded1 = ctx->interim[1];
> +        } else {
> +            p        = p_default;
> +            decoded0 = ctx->decoded[0];
> +            decoded1 = ctx->decoded[1];
> +        }
> +        p->buf = p->historybuffer;
> +
> +        count = lcount;
> +        while (count--) {
> +            /* Predictor Y */
> +            int32_t a0 = predictor_update_filter(p, *decoded0, 0,
> YDELAYA, YDELAYB,
> +                                                YADAPTCOEFFSA,
> YADAPTCOEFFSB,
> +                                                interim_mode);
> +            int32_t a1 = predictor_update_filter(p, *decoded1, 1,
> XDELAYA, XDELAYB,
> +                                                XADAPTCOEFFSA,
> XADAPTCOEFFSB,
> +                                                interim_mode);
> +            *decoded0++ = a0;
> +            *decoded1++ = a1;
> +            if (num_passes > 1) {
> +                int32_t left  = a1 - (unsigned)(a0 / 2);
> +                int32_t right = left + a0;
> +
> +                if (FFMAX(FFABS(left), FFABS(right)) > (1<<23)) {
> +                    ctx->interim_mode = !interim_mode;
> +                    av_log(ctx->avctx, AV_LOG_VERBOSE, "Interim mode:
> %d\n", ctx->interim_mode);
> +                    break;
> +                }
> +            }
>
> -        /* Have we filled the history buffer? */
> -        if (p->buf == p->historybuffer + HISTORY_SIZE) {
> -            memmove(p->historybuffer, p->buf,
> -                    PREDICTOR_SIZE * sizeof(*p->historybuffer));
> -            p->buf = p->historybuffer;
> +            /* Combined */
> +            p->buf++;
> +
> +            /* Have we filled the history buffer? */
> +            if (p->buf == p->historybuffer + HISTORY_SIZE) {
> +                memmove(p->historybuffer, p->buf,
> +                        PREDICTOR_SIZE * sizeof(*p->historybuffer));
> +                p->buf = p->historybuffer;
> +            }
>          }
>      }
> +    if (num_passes > 1 && ctx->interim_mode > 0) {
> +        memcpy(ctx->decoded[0], ctx->interim[0],
> sizeof(*ctx->interim[0])*lcount);
> +        memcpy(ctx->decoded[1], ctx->interim[1],
> sizeof(*ctx->interim[1])*lcount);
> +        *p_default = p_interim;
> +        p_default->buf = p_default->historybuffer;
> +    }
>  }
>
>  static void predictor_decode_mono_3950(APEContext *ctx, int count)
> @@ -1590,6 +1639,19 @@ static int ape_decode_frame(AVCodecContext *avctx,
> AVFrame *frame,
>      s->decoded[0] = s->decoded_buffer;
>      s->decoded[1] = s->decoded_buffer + FFALIGN(blockstodecode, 8);
>
> +    if (s->interim_mode < 0) {
> +        av_fast_malloc(&s->interim_buffer, &s->interim_size,
> decoded_buffer_size);
> +        if (!s->interim_buffer)
> +            return AVERROR(ENOMEM);
> +        memset(s->interim_buffer, 0, decoded_buffer_size);
> +        s->interim[0] = s->interim_buffer;
> +        s->interim[1] = s->interim_buffer + FFALIGN(blockstodecode, 8);
> +    } else {
> +        av_freep(&s->interim_buffer);
> +        s->interim_size = 0;
> +        memset(s->interim, 0, sizeof(s->interim));
> +    }
> +
>      s->error=0;
>
>      if ((s->channels == 1) || (s->frameflags &
> APE_FRAMECODE_PSEUDO_STEREO))
> --
> 2.17.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 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 2/2] avcodec/apedec: Implement interim mode detection

[Anton Khirnov]

Quoting Michael Niedermayer (2023-08-26 18:53:50)
> Fixes: NoLegacy.ape

Can you make a FATE test?

-- 
Anton Khirnov
_______________________________________________
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 2/2] avcodec/apedec: Implement interim mode detection

[Michael Niedermayer]

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

On Sat, Aug 26, 2023 at 07:55:10PM +0200, Anton Khirnov wrote:
> Quoting Michael Niedermayer (2023-08-26 18:53:50)
> > Fixes: NoLegacy.ape
> 
> Can you make a FATE test?

The file seems 34mb, ill try to cut it down to something smaller

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

When the tyrant has disposed of foreign enemies by conquest or treaty, and
there is nothing more to fear from them, then he is always stirring up
some war or other, in order that the people may require a leader. -- Plato

[-- 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 2/2] avcodec/apedec: Implement interim mode detection

[Michael Niedermayer]

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

On Sat, Aug 26, 2023 at 07:29:40PM +0200, Paul B Mahol wrote:
> On Sat, Aug 26, 2023 at 6:54 PM Michael Niedermayer <michael@niedermayer.cc>
> wrote:
> 
> > Fixes: NoLegacy.ape
> > Found-by: Matt Ashland <mail@monkeysaudio.com>
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >  libavcodec/apedec.c | 106 +++++++++++++++++++++++++++++++++++---------
> >  1 file changed, 84 insertions(+), 22 deletions(-)
> >
> > diff --git a/libavcodec/apedec.c b/libavcodec/apedec.c
> > index 8bd625ca05..249fc22e24 100644
> > --- a/libavcodec/apedec.c
> > +++ b/libavcodec/apedec.c
> > @@ -171,6 +171,9 @@ typedef struct APEContext {
> >      int32_t *decoded_buffer;
> >      int decoded_size;
> >      int32_t *decoded[MAX_CHANNELS];          ///< decoded data for each
> > channel
> > +    int32_t *interim_buffer;
> > +    int interim_size;
> > +    int32_t *interim[MAX_CHANNELS];          ///< decoded data for each
> > channel
> >      int blocks_per_loop;                     ///< maximum number of
> > samples to decode for each call
> >
> >      int16_t* filterbuf[APE_FILTER_LEVELS];   ///< filter memory
> > @@ -187,6 +190,7 @@ typedef struct APEContext {
> >      const uint8_t *ptr;                      ///< current position in
> > frame data
> >
> >      int error;
> > +    int interim_mode;
> >
> >      void (*entropy_decode_mono)(struct APEContext *ctx, int
> > blockstodecode);
> >      void (*entropy_decode_stereo)(struct APEContext *ctx, int
> > blockstodecode);
> > @@ -223,6 +227,7 @@ static av_cold int ape_decode_close(AVCodecContext
> > *avctx)
> >          av_freep(&s->filterbuf[i]);
> >
> >      av_freep(&s->decoded_buffer);
> > +    av_freep(&s->interim_buffer);
> >      av_freep(&s->data);
> >      s->decoded_size = s->data_size = 0;
> >
> > @@ -248,12 +253,15 @@ static av_cold int ape_decode_init(AVCodecContext
> > *avctx)
> >      switch (s->bps) {
> >      case 8:
> >          avctx->sample_fmt = AV_SAMPLE_FMT_U8P;
> > +        s->interim_mode = 0;
> >          break;
> >      case 16:
> >          avctx->sample_fmt = AV_SAMPLE_FMT_S16P;
> > +        s->interim_mode = 0;
> >          break;
> >      case 24:
> >          avctx->sample_fmt = AV_SAMPLE_FMT_S32P;
> > +        s->interim_mode = -1;
> >          break;
> >      default:
> >          avpriv_request_sample(avctx,
> > @@ -1181,7 +1189,7 @@ static av_always_inline int
> > predictor_update_filter(APEPredictor64 *p,
> >                                                      const int decoded,
> > const int filter,
> >                                                      const int delayA,
> > const int delayB,
> >                                                      const int adaptA,
> > const int adaptB,
> > -                                                    int compression_level)
> > +                                                    int interim_mode)
> >  {
> >      int64_t predictionA, predictionB;
> >      int32_t sign;
> > @@ -1209,7 +1217,7 @@ static av_always_inline int
> > predictor_update_filter(APEPredictor64 *p,
> >                    p->buf[delayB - 3] * p->coeffsB[filter][3] +
> >                    p->buf[delayB - 4] * p->coeffsB[filter][4];
> >
> > -    if (compression_level < COMPRESSION_LEVEL_INSANE) {
> > +    if (interim_mode < 1) {
> >          predictionA = (int32_t)predictionA;
> >          predictionB = (int32_t)predictionB;
> >          p->lastA[filter] = decoded + ((int32_t)(predictionA +
> > (predictionB >> 1)) >> 10);
> > @@ -1234,33 +1242,74 @@ static av_always_inline int
> > predictor_update_filter(APEPredictor64 *p,
> >
> >  static void predictor_decode_stereo_3950(APEContext *ctx, int count)
> >  {
> > -    APEPredictor64 *p = &ctx->predictor64;
> > -    int32_t *decoded0 = ctx->decoded[0];
> > -    int32_t *decoded1 = ctx->decoded[1];
> > +    APEPredictor64 *p_default = &ctx->predictor64;
> > +    APEPredictor64 p_interim;
> > +    int lcount = count;
> > +    int num_passes = 1;
> >
> >      ape_apply_filters(ctx, ctx->decoded[0], ctx->decoded[1], count);
> > +    if (ctx->interim_mode == -1) {
> > +        p_interim = *p_default;
> > +        num_passes ++;
> > +        memcpy(ctx->interim[0], ctx->decoded[0],
> > sizeof(*ctx->interim[0])*count);
> > +        memcpy(ctx->interim[1], ctx->decoded[1],
> > sizeof(*ctx->interim[1])*count);
> > +    }
> >
> > -    while (count--) {
> > -        /* Predictor Y */
> > -        *decoded0 = predictor_update_filter(p, *decoded0, 0, YDELAYA,
> > YDELAYB,
> > -                                            YADAPTCOEFFSA, YADAPTCOEFFSB,
> > -                                            ctx->compression_level);
> > -        decoded0++;
> > -        *decoded1 = predictor_update_filter(p, *decoded1, 1, XDELAYA,
> > XDELAYB,
> > -                                            XADAPTCOEFFSA, XADAPTCOEFFSB,
> > -                                            ctx->compression_level);
> > -        decoded1++;
> > +    for(int pass = 0; pass < num_passes; pass++) {
> >
> 
> Please fix your style in patches.

Noone can know from this what exactly you meant but
ill apply with the whitespace issue i found, which may be what you meant.
No need to be precisse, thats ok with me if you prefer that but a
inprecisse review comment will result in inprecisse results

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Its not that you shouldnt use gotos but rather that you should write
readable code and code with gotos often but not always is less readable

[-- 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 2/2] avcodec/apedec: Implement interim mode detection

[Michael Niedermayer]

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

On Sat, Aug 26, 2023 at 08:07:14PM +0200, Michael Niedermayer wrote:
> On Sat, Aug 26, 2023 at 07:55:10PM +0200, Anton Khirnov wrote:
> > Quoting Michael Niedermayer (2023-08-26 18:53:50)
> > > Fixes: NoLegacy.ape
> > 
> > Can you make a FATE test?
> 
> The file seems 34mb, ill try to cut it down to something smaller

ive been able to cut it down to 100kb
tested on mips/arm/mingw32/64 and linux32/64 x86

test seems trivial, so ill apply it with my next push, test is below
for reference

commit 2c1ade30ad8f523d816a853f90512808b0c039e1 (HEAD -> master)
Author: Michael Niedermayer <michael@niedermayer.cc>
Date:   Sat Aug 26 20:21:32 2023 +0200

    tests/fate: Add NoLegacy-cut.ape test

    Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>

diff --git a/tests/fate/monkeysaudio.mak b/tests/fate/monkeysaudio.mak
index d75937ec02..03c646cd47 100644
--- a/tests/fate/monkeysaudio.mak
+++ b/tests/fate/monkeysaudio.mak
@@ -16,5 +16,8 @@ $(foreach N,$(APE_VERSIONS),$(eval $(call FATE_APE_SUITE,$(N))))
 FATE_APE += fate-lossless-monkeysaudio-399
 fate-lossless-monkeysaudio-399: CMD = md5 -i $(TARGET_SAMPLES)/lossless-audio/luckynight-partial.ape -f s16le -af aresample

+FATE_APE += fate-lossless-monkeysaudio-legacy
+fate-lossless-monkeysaudio-legacy: CMD = md5 -i $(TARGET_SAMPLES)/lossless-audio/NoLegacy-cut.ape -f s32le -af aresample
+
 FATE_SAMPLES_AVCONV-$(call DEMDEC, APE, APE) += $(FATE_APE)
 fate-lossless-monkeysaudio: $(FATE_APE)
diff --git a/tests/ref/fate/lossless-monkeysaudio-legacy b/tests/ref/fate/lossless-monkeysaudio-legacy
new file mode 100644
index 0000000000..33d4b56204
--- /dev/null
+++ b/tests/ref/fate/lossless-monkeysaudio-legacy
@@ -0,0 +1 @@
+2cd8a60478c77382dfed8b8bdf1f70e4


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The day soldiers stop bringing you their problems is the day you have stopped 
leading them. They have either lost confidence that you can help or concluded 
you do not care. Either case is a failure of leadership. - Colin Powell

[-- 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 2/2] avcodec/apedec: Implement interim mode detection

[Michael Niedermayer]

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

On Sat, Aug 26, 2023 at 06:53:50PM +0200, Michael Niedermayer wrote:
> Fixes: NoLegacy.ape
> Found-by: Matt Ashland <mail@monkeysaudio.com>
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavcodec/apedec.c | 106 +++++++++++++++++++++++++++++++++++---------
>  1 file changed, 84 insertions(+), 22 deletions(-)

Also, the same issue should exists in the mono codepath. I have
no testcase for mono, so left this unchanged for now, as i cannot
test it.
But if someone has a testcase, tell me

It also may be needed to wait till one of the CRCs to distinguish
between the 2 decoding forms. (this is how the SDK does it)

Waiting for some CRC is a bit inconvenient as we return data before
currently. And this quicker variant here works for all files i have

We also differ from the SDK by decoding both forms in parallel until
one fails. The advantage here is that if only one is decoded then
a random failure cannot be distinguished from "interim mode" which
would result in a bitstream error causing a switch to the wrong
mode.

Above is how its done ATM in git master. As more issues get submitted
that may change

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I do not agree with what you have to say, but I'll defend to the death your
right to say it. -- Voltaire

[-- 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".