diff mbox series

[FFmpeg-devel,1/2] Revert "avcodec/apedec: fix decoding 24bit insane files with recent versions"

Message ID 20230824232135.26854-1-michael@niedermayer.cc
State New
Headers show
Series [FFmpeg-devel,1/2] Revert "avcodec/apedec: fix decoding 24bit insane files with recent versions" | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Michael Niedermayer Aug. 24, 2023, 11:21 p.m. UTC
Fixes: Ticket9816

This reverts commit ed0001482a74b60f3d5bc5cd7e304c9d65b2fcd5.
---
 libavcodec/apedec.c | 59 ++++++++++-----------------------------------
 1 file changed, 13 insertions(+), 46 deletions(-)

Comments

Paul B Mahol Aug. 24, 2023, 11:37 p.m. UTC | #1
NAK

Breaks decoding of another sample.
Michael Niedermayer Aug. 25, 2023, 12:01 a.m. UTC | #2
On Fri, Aug 25, 2023 at 01:37:27AM +0200, Paul B Mahol wrote:
> NAK
> 
> Breaks decoding of another sample.

please provide the sample

if noone provides a sample, i will disregard this objection.
Also as a sidenote, iam in contact with Matt Ashland and he also
doesnt know what your commit was trying to fix

thx

[...]
Paul B Mahol Aug. 25, 2023, 9:08 a.m. UTC | #3
On Fri, Aug 25, 2023 at 2:01 AM Michael Niedermayer <michael@niedermayer.cc>
wrote:

> On Fri, Aug 25, 2023 at 01:37:27AM +0200, Paul B Mahol wrote:
> > NAK
> >
> > Breaks decoding of another sample.
>
> please provide the sample
>
> if noone provides a sample, i will disregard this objection.
> Also as a sidenote, iam in contact with Matt Ashland and he also
> doesnt know what your commit was trying to fix
>

What to say, monkey see - monkey do.

Just wasted too much time on running latest MAC converter software in
virtualbox to confirm
you break this ticket bellow with your patch set.

https://trac.ffmpeg.org/ticket/8918

My patience is lowering by each new day and each new patch(es) from you.


>
> thx
>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> If the United States is serious about tackling the national security
> threats
> related to an insecure 5G network, it needs to rethink the extent to which
> it
> values corporate profits and government espionage over security.-Bruce
> Schneier
> _______________________________________________
> 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".
>
Michael Niedermayer Aug. 25, 2023, 2:20 p.m. UTC | #4
On Fri, Aug 25, 2023 at 11:08:55AM +0200, Paul B Mahol wrote:
> On Fri, Aug 25, 2023 at 2:01 AM Michael Niedermayer <michael@niedermayer.cc>
> wrote:
> 
> > On Fri, Aug 25, 2023 at 01:37:27AM +0200, Paul B Mahol wrote:
> > > NAK
> > >
> > > Breaks decoding of another sample.
> >
> > please provide the sample
> >
> > if noone provides a sample, i will disregard this objection.
> > Also as a sidenote, iam in contact with Matt Ashland and he also
> > doesnt know what your commit was trying to fix
> >
> 
> What to say, monkey see - monkey do.
> 

> Just wasted too much time on running latest MAC converter software in
> virtualbox to confirm
> you break this ticket bellow with your patch set.

you dont need virtualbox to run ffmpeg to test the samples
if you used virtualbox to generate more samples, please share them so
i can make sure there are no regressions on changes


> 
> https://trac.ffmpeg.org/ticket/8918

thanks for the testcase, i will make sure my next pachset will not break that


[...]
Paul B Mahol Aug. 25, 2023, 2:42 p.m. UTC | #5
On Fri, Aug 25, 2023 at 4:21 PM Michael Niedermayer <michael@niedermayer.cc>
wrote:

> On Fri, Aug 25, 2023 at 11:08:55AM +0200, Paul B Mahol wrote:
> > On Fri, Aug 25, 2023 at 2:01 AM Michael Niedermayer <
> michael@niedermayer.cc>
> > wrote:
> >
> > > On Fri, Aug 25, 2023 at 01:37:27AM +0200, Paul B Mahol wrote:
> > > > NAK
> > > >
> > > > Breaks decoding of another sample.
> > >
> > > please provide the sample
> > >
> > > if noone provides a sample, i will disregard this objection.
> > > Also as a sidenote, iam in contact with Matt Ashland and he also
> > > doesnt know what your commit was trying to fix
> > >
> >
> > What to say, monkey see - monkey do.
> >
>
> > Just wasted too much time on running latest MAC converter software in
> > virtualbox to confirm
> > you break this ticket bellow with your patch set.
>
> you dont need virtualbox to run ffmpeg to test the samples
> if you used virtualbox to generate more samples, please share them so
> i can make sure there are no regressions on changes
>
>
MAC is reference Monkey's APEs encoder/decoder Windows only utility.
Just make sure that its not also MAC regression somehow...


> >
> > https://trac.ffmpeg.org/ticket/8918
>
> thanks for the testcase, i will make sure my next pachset will not break
> that
>
>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> What does censorship reveal? It reveals fear. -- Julian Assange
> _______________________________________________
> 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".
>
diff mbox series

Patch

diff --git a/libavcodec/apedec.c b/libavcodec/apedec.c
index 613c76df0b..cc0d7e2749 100644
--- a/libavcodec/apedec.c
+++ b/libavcodec/apedec.c
@@ -134,21 +134,6 @@  typedef struct APEPredictor {
     unsigned int sample_pos;
 } APEPredictor;
 
-typedef struct APEPredictor64 {
-    int64_t *buf;
-
-    int64_t lastA[2];
-
-    int64_t filterA[2];
-    int64_t filterB[2];
-
-    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 */
 typedef struct APEContext {
     AVClass *class;                          ///< class for AVOptions
@@ -168,7 +153,6 @@  typedef struct APEContext {
     uint32_t CRC_state;                      ///< accumulated CRC
     int frameflags;                          ///< frame flags
     APEPredictor predictor;                  ///< predictor used for final reconstruction
-    APEPredictor64 predictor64;              ///< 64bit predictor used for final reconstruction
 
     int32_t *decoded_buffer;
     int decoded_size;
@@ -808,20 +792,13 @@  static const int32_t initial_coeffs_3930[4] = {
     360, 317, -109, 98
 };
 
-static const int64_t initial_coeffs_3930_64bit[4] = {
-    360, 317, -109, 98
-};
-
 static void init_predictor_decoder(APEContext *ctx)
 {
     APEPredictor *p = &ctx->predictor;
-    APEPredictor64 *p64 = &ctx->predictor64;
 
     /* Zero the history buffers */
     memset(p->historybuffer, 0, PREDICTOR_SIZE * sizeof(*p->historybuffer));
-    memset(p64->historybuffer, 0, PREDICTOR_SIZE * sizeof(*p64->historybuffer));
     p->buf = p->historybuffer;
-    p64->buf = p64->historybuffer;
 
     /* Initialize and zero the coefficients */
     if (ctx->fileversion < 3930) {
@@ -839,11 +816,8 @@  static void init_predictor_decoder(APEContext *ctx)
     } else {
         memcpy(p->coeffsA[0], initial_coeffs_3930, sizeof(initial_coeffs_3930));
         memcpy(p->coeffsA[1], initial_coeffs_3930, sizeof(initial_coeffs_3930));
-        memcpy(p64->coeffsA[0], initial_coeffs_3930_64bit, sizeof(initial_coeffs_3930_64bit));
-        memcpy(p64->coeffsA[1], initial_coeffs_3930_64bit, sizeof(initial_coeffs_3930_64bit));
     }
     memset(p->coeffsB, 0, sizeof(p->coeffsB));
-    memset(p64->coeffsB, 0, sizeof(p64->coeffsB));
     if (ctx->fileversion < 3930) {
         memcpy(p->coeffsB[0], initial_coeffs_b_3800,
                sizeof(initial_coeffs_b_3800));
@@ -855,13 +829,7 @@  static void init_predictor_decoder(APEContext *ctx)
     p->filterB[0] = p->filterB[1] = 0;
     p->lastA[0]   = p->lastA[1]   = 0;
 
-    p64->filterA[0] = p64->filterA[1] = 0;
-    p64->filterB[0] = p64->filterB[1] = 0;
-    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) */
@@ -1181,17 +1149,16 @@  static void predictor_decode_mono_3930(APEContext *ctx, int count)
     }
 }
 
-static av_always_inline int predictor_update_filter(APEPredictor64 *p,
+static av_always_inline int predictor_update_filter(APEPredictor *p,
                                                     const int decoded, const int filter,
                                                     const int delayA,  const int delayB,
                                                     const int adaptA,  const int adaptB)
 {
-    int64_t predictionA, predictionB;
-    int32_t sign;
+    int32_t predictionA, predictionB, sign;
 
     p->buf[delayA]     = p->lastA[filter];
     p->buf[adaptA]     = APESIGN(p->buf[delayA]);
-    p->buf[delayA - 1] = p->buf[delayA] - (uint64_t)p->buf[delayA - 1];
+    p->buf[delayA - 1] = p->buf[delayA] - (unsigned)p->buf[delayA - 1];
     p->buf[adaptA - 1] = APESIGN(p->buf[delayA - 1]);
 
     predictionA = p->buf[delayA    ] * p->coeffsA[filter][0] +
@@ -1200,9 +1167,9 @@  static av_always_inline int predictor_update_filter(APEPredictor64 *p,
                   p->buf[delayA - 3] * p->coeffsA[filter][3];
 
     /*  Apply a scaled first-order filter compression */
-    p->buf[delayB]     = p->filterA[filter ^ 1] - ((int64_t)(p->filterB[filter] * 31ULL) >> 5);
+    p->buf[delayB]     = p->filterA[filter ^ 1] - ((int)(p->filterB[filter] * 31U) >> 5);
     p->buf[adaptB]     = APESIGN(p->buf[delayB]);
-    p->buf[delayB - 1] = p->buf[delayB] - (uint64_t)p->buf[delayB - 1];
+    p->buf[delayB - 1] = p->buf[delayB] - (unsigned)p->buf[delayB - 1];
     p->buf[adaptB - 1] = APESIGN(p->buf[delayB - 1]);
     p->filterB[filter] = p->filterA[filter ^ 1];
 
@@ -1212,8 +1179,8 @@  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];
 
-    p->lastA[filter] = decoded + ((int64_t)((uint64_t)predictionA + (predictionB >> 1)) >> 10);
-    p->filterA[filter] = p->lastA[filter] + ((int64_t)(p->filterA[filter] * 31ULL) >> 5);
+    p->lastA[filter] = decoded + ((int)((unsigned)predictionA + (predictionB >> 1)) >> 10);
+    p->filterA[filter] = p->lastA[filter] + ((int)(p->filterA[filter] * 31U) >> 5);
 
     sign = APESIGN(decoded);
     p->coeffsA[filter][0] += p->buf[adaptA    ] * sign;
@@ -1231,7 +1198,7 @@  static av_always_inline int predictor_update_filter(APEPredictor64 *p,
 
 static void predictor_decode_stereo_3950(APEContext *ctx, int count)
 {
-    APEPredictor64 *p = &ctx->predictor64;
+    APEPredictor *p = &ctx->predictor;
     int32_t *decoded0 = ctx->decoded[0];
     int32_t *decoded1 = ctx->decoded[1];
 
@@ -1260,7 +1227,7 @@  static void predictor_decode_stereo_3950(APEContext *ctx, int count)
 
 static void predictor_decode_mono_3950(APEContext *ctx, int count)
 {
-    APEPredictor64 *p = &ctx->predictor64;
+    APEPredictor *p = &ctx->predictor;
     int32_t *decoded0 = ctx->decoded[0];
     int32_t predictionA, currentA, A, sign;
 
@@ -1272,14 +1239,14 @@  static void predictor_decode_mono_3950(APEContext *ctx, int count)
         A = *decoded0;
 
         p->buf[YDELAYA] = currentA;
-        p->buf[YDELAYA - 1] = p->buf[YDELAYA] - (uint64_t)p->buf[YDELAYA - 1];
+        p->buf[YDELAYA - 1] = p->buf[YDELAYA] - (unsigned)p->buf[YDELAYA - 1];
 
         predictionA = p->buf[YDELAYA    ] * p->coeffsA[0][0] +
                       p->buf[YDELAYA - 1] * p->coeffsA[0][1] +
                       p->buf[YDELAYA - 2] * p->coeffsA[0][2] +
                       p->buf[YDELAYA - 3] * p->coeffsA[0][3];
 
-        currentA = A + (uint64_t)(predictionA >> 10);
+        currentA = A + (unsigned)(predictionA >> 10);
 
         p->buf[YADAPTCOEFFSA]     = APESIGN(p->buf[YDELAYA    ]);
         p->buf[YADAPTCOEFFSA - 1] = APESIGN(p->buf[YDELAYA - 1]);
@@ -1299,7 +1266,7 @@  static void predictor_decode_mono_3950(APEContext *ctx, int count)
             p->buf = p->historybuffer;
         }
 
-        p->filterA[0] = currentA + (uint64_t)((int64_t)(p->filterA[0] * 31U) >> 5);
+        p->filterA[0] = currentA + (unsigned)((int)(p->filterA[0] * 31U) >> 5);
         *(decoded0++) = p->filterA[0];
     }
 
@@ -1336,7 +1303,7 @@  static void do_apply_filter(APEContext *ctx, int version, APEFilter *f,
                                                      f->delay - order,
                                                      f->adaptcoeffs - order,
                                                      order, APESIGN(*data));
-        res = (int64_t)(res + (1LL << (fracbits - 1))) >> fracbits;
+        res = (int)(res + (1U << (fracbits - 1))) >> fracbits;
         res += (unsigned)*data;
         *data++ = res;