| Message ID | 20230825151328.25567-2-michael@niedermayer.cc |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/2] avcodec/apedec: Fix CRC for 24bps and bigendian | expand |
| 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 |
On Fri, Aug 25, 2023 at 5:13 PM Michael Niedermayer <michael@niedermayer.cc> wrote: > Fixes: Ticket9816 > Fixes: vlc.ape and APE_48K_24bit_2CH_02_01.ape > > Regression since: ed0001482a74b60f3d5bc5cd7e304c9d65b2fcd5. > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavcodec/apedec.c | 15 +++++++++++---- > 1 file changed, 11 insertions(+), 4 deletions(-) > > diff --git a/libavcodec/apedec.c b/libavcodec/apedec.c > index 7bad8500e1..b1cceb9395 100644 > --- a/libavcodec/apedec.c > +++ b/libavcodec/apedec.c > @@ -1184,7 +1184,9 @@ static void predictor_decode_mono_3930(APEContext > *ctx, int count) > 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) > + const int adaptA, > const int adaptB, > + int compression_level > + ) > Why on ')' separate line? > { > int64_t predictionA, predictionB; > int32_t sign; > @@ -1212,7 +1214,12 @@ 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); > + if (compression_level < COMPRESSION_LEVEL_INSANE) { > + predictionA = (int32_t)predictionA; > + predictionB = (int32_t)predictionB; > + p->lastA[filter] = decoded + ((int32_t)(predictionA + > (predictionB >> 1)) >> 10); > + } else > + p->lastA[filter] = decoded + ((int64_t)((uint64_t)predictionA + > (predictionB >> 1)) >> 10); > Please make {} block. > p->filterA[filter] = p->lastA[filter] + ((int64_t)(p->filterA[filter] > * 31ULL) >> 5); > > sign = APESIGN(decoded); > @@ -1240,10 +1247,10 @@ static void > predictor_decode_stereo_3950(APEContext *ctx, int count) > while (count--) { > /* Predictor Y */ > *decoded0 = predictor_update_filter(p, *decoded0, 0, YDELAYA, > YDELAYB, > - YADAPTCOEFFSA, YADAPTCOEFFSB); > + YADAPTCOEFFSA, YADAPTCOEFFSB, > ctx->compression_level); > decoded0++; > *decoded1 = predictor_update_filter(p, *decoded1, 1, XDELAYA, > XDELAYB, > - XADAPTCOEFFSA, XADAPTCOEFFSB); > + XADAPTCOEFFSA, XADAPTCOEFFSB, > ctx->compression_level); > decoded1++; > > /* Combined */ > -- > 2.17.1 > > OK with that style fixed, assuming it fixes that ticket and is still bitexact with both files. > _______________________________________________ > 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". >
On Fri, Aug 25, 2023 at 05:27:14PM +0200, Paul B Mahol wrote: > On Fri, Aug 25, 2023 at 5:13 PM Michael Niedermayer <michael@niedermayer.cc> > wrote: > > > Fixes: Ticket9816 > > Fixes: vlc.ape and APE_48K_24bit_2CH_02_01.ape > > > > Regression since: ed0001482a74b60f3d5bc5cd7e304c9d65b2fcd5. > > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > --- > > libavcodec/apedec.c | 15 +++++++++++---- > > 1 file changed, 11 insertions(+), 4 deletions(-) > > > > diff --git a/libavcodec/apedec.c b/libavcodec/apedec.c > > index 7bad8500e1..b1cceb9395 100644 > > --- a/libavcodec/apedec.c > > +++ b/libavcodec/apedec.c > > @@ -1184,7 +1184,9 @@ static void predictor_decode_mono_3930(APEContext > > *ctx, int count) > > 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) > > + const int adaptA, > > const int adaptB, > > + int compression_level > > + ) > > > > Why on ')' separate line? I asked the ')' but it doesnt seem to remember fixed > > > > { > > int64_t predictionA, predictionB; > > int32_t sign; > > @@ -1212,7 +1214,12 @@ 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); > > + if (compression_level < COMPRESSION_LEVEL_INSANE) { > > + predictionA = (int32_t)predictionA; > > + predictionB = (int32_t)predictionB; > > + p->lastA[filter] = decoded + ((int32_t)(predictionA + > > (predictionB >> 1)) >> 10); > > + } else > > + p->lastA[filter] = decoded + ((int64_t)((uint64_t)predictionA + > > (predictionB >> 1)) >> 10); > > > > Please make {} block. ok > > > > p->filterA[filter] = p->lastA[filter] + ((int64_t)(p->filterA[filter] > > * 31ULL) >> 5); > > > > sign = APESIGN(decoded); > > @@ -1240,10 +1247,10 @@ static void > > predictor_decode_stereo_3950(APEContext *ctx, int count) > > while (count--) { > > /* Predictor Y */ > > *decoded0 = predictor_update_filter(p, *decoded0, 0, YDELAYA, > > YDELAYB, > > - YADAPTCOEFFSA, YADAPTCOEFFSB); > > + YADAPTCOEFFSA, YADAPTCOEFFSB, > > ctx->compression_level); > > decoded0++; > > *decoded1 = predictor_update_filter(p, *decoded1, 1, XDELAYA, > > XDELAYB, > > - XADAPTCOEFFSA, XADAPTCOEFFSB); > > + XADAPTCOEFFSA, XADAPTCOEFFSB, > > ctx->compression_level); > > decoded1++; > > > > /* Combined */ > > -- > > 2.17.1 > > > > > OK with that style fixed, assuming it fixes that ticket and is still > bitexact with both files. ive fixed the style issues i found, i hope i found all CRC tests pass on the 3 files i have will apply with my next git push thx [...]
diff --git a/libavcodec/apedec.c b/libavcodec/apedec.c index 7bad8500e1..b1cceb9395 100644 --- a/libavcodec/apedec.c +++ b/libavcodec/apedec.c @@ -1184,7 +1184,9 @@ static void predictor_decode_mono_3930(APEContext *ctx, int count) 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) + const int adaptA, const int adaptB, + int compression_level + ) { int64_t predictionA, predictionB; int32_t sign; @@ -1212,7 +1214,12 @@ 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); + if (compression_level < COMPRESSION_LEVEL_INSANE) { + predictionA = (int32_t)predictionA; + predictionB = (int32_t)predictionB; + p->lastA[filter] = decoded + ((int32_t)(predictionA + (predictionB >> 1)) >> 10); + } else + 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); sign = APESIGN(decoded); @@ -1240,10 +1247,10 @@ static void predictor_decode_stereo_3950(APEContext *ctx, int count) while (count--) { /* Predictor Y */ *decoded0 = predictor_update_filter(p, *decoded0, 0, YDELAYA, YDELAYB, - YADAPTCOEFFSA, YADAPTCOEFFSB); + YADAPTCOEFFSA, YADAPTCOEFFSB, ctx->compression_level); decoded0++; *decoded1 = predictor_update_filter(p, *decoded1, 1, XDELAYA, XDELAYB, - XADAPTCOEFFSA, XADAPTCOEFFSB); + XADAPTCOEFFSA, XADAPTCOEFFSB, ctx->compression_level); decoded1++; /* Combined */
Fixes: Ticket9816 Fixes: vlc.ape and APE_48K_24bit_2CH_02_01.ape Regression since: ed0001482a74b60f3d5bc5cd7e304c9d65b2fcd5. Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavcodec/apedec.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-)