On Thu, Jun 30, 2022 at 10:31 AM Andreas Rheinhardt < andreas.rheinha...@outlook.com> wrote:
> Zhao Zhili: > > From: Zhao Zhili <zhiliz...@tencent.com> > > > > Fixes ticket #9816. > > Regression since ed0001482a74b60f3d5bc5. > > > > Prediction value larger than INT32_MAX should be treated as > > negative. The code already depends on undefined right shift > > behavior before the patch, which doesn't get fixed by the patch. > > > > Signed-off-by: Zhao Zhili <zhiliz...@tencent.com> > > --- > > libavcodec/apedec.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/libavcodec/apedec.c b/libavcodec/apedec.c > > index a7c38bce1b..5f2af2e147 100644 > > --- a/libavcodec/apedec.c > > +++ b/libavcodec/apedec.c > > @@ -1198,7 +1198,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]; > > > > - p->lastA[filter] = decoded + ((int64_t)((uint64_t)predictionA + > (predictionB >> 1)) >> 10); > > + p->lastA[filter] = decoded + ((int32_t)((uint64_t)predictionA + > (predictionB >> 1)) >> 10); > > p->filterA[filter] = p->lastA[filter] + > ((int64_t)(p->filterA[filter] * 31ULL) >> 5); > > > > sign = APESIGN(decoded); > > 1. Right shifts of negative numbers are implementation-defined, not > undefined. And we rely on the implementation to do the sane thing for > two's complement, so it is defined for us. > 2. You are not necessarily treating them as negative, you simply discard > the upper 32 bits and the result could be positive. And you discard the > upper bits before you discard the lowest 10 bits, so the result has > effectively only 22 bits. Is this really intended? If it is, you could > perform the addition predictionA + (predictionB >> 1) in 32 bits. > 3. I see one possibility for UB in this: The outermost addition on the > right side is a signed addition, so it could overflow. > Also make sure that you do not break insane ape files, its on one of trac issues. > > - 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-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".