subject:"\[FFmpeg\-devel\] \[PATCH 2\/2\] avcodec\/apedec\: Fix 48khz 24bit below insane level"

Re: [FFmpeg-devel] [PATCH 2/2] avcodec/apedec: Fix 48khz 24bit below insane level

2023-08-25 Thread Michael Niedermayer

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 
> wrote:
> 
> > Fixes: Ticket9816
> > Fixes: vlc.ape and APE_48K_24bit_2CH_02_01.ape
> >
> > Regression since: ed0001482a74b60f3d5bc5cd7e304c9d65b2fcd5.
> >
> > Signed-off-by: Michael Niedermayer 
> > ---
> >  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

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

I have often repented speaking, but never of holding my tongue.
-- Xenocrates


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH 2/2] avcodec/apedec: Fix 48khz 24bit below insane level

2023-08-25 Thread Paul B Mahol

On Fri, Aug 25, 2023 at 5:13 PM Michael Niedermayer 
wrote:

> Fixes: Ticket9816
> Fixes: vlc.ape and APE_48K_24bit_2CH_02_01.ape
>
> Regression since: ed0001482a74b60f3d5bc5cd7e304c9d65b2fcd5.
>
> Signed-off-by: Michael Niedermayer 
> ---
>  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-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 2/2] avcodec/apedec: Fix 48khz 24bit below insane level

2023-08-25 Thread Michael Niedermayer

Fixes: Ticket9816
Fixes: vlc.ape and APE_48K_24bit_2CH_02_01.ape

Regression since: ed0001482a74b60f3d5bc5cd7e304c9d65b2fcd5.

Signed-off-by: Michael Niedermayer 
---
 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
+   )
 {
 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 */
-- 
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-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/2] avcodec/apedec: Fix 48khz 24bit below insane level

Re: [FFmpeg-devel] [PATCH 2/2] avcodec/apedec: Fix 48khz 24bit below insane level

[FFmpeg-devel] [PATCH 2/2] avcodec/apedec: Fix 48khz 24bit below insane level

3 matches

Site Navigation

Mail list logo

Footer information