[FFmpeg-devel] [PATCH] avcodec/wavpack: check for overflow

[Paul B Mahol]

On 7/3/13, Michael Niedermayer <michaelni at gmx.at> wrote:
> On Tue, Jul 02, 2013 at 11:05:09PM +0000, Paul B Mahol wrote:
>> On 7/2/13, Michael Niedermayer <michaelni at gmx.at> wrote:
>> > On Mon, Jun 17, 2013 at 05:49:47PM +0200, Michael Niedermayer wrote:
>> >> On Mon, Jun 17, 2013 at 10:54:28AM +0000, Paul B Mahol wrote:
>> >> > On 6/15/13, Michael Niedermayer <michaelni at gmx.at> wrote:
>> >> > > Fix integer overflow in fate
>> >> > >
>> >> > > Signed-off-by: Michael Niedermayer <michaelni at gmx.at>
>> >> > > ---
>> >> > >  libavcodec/wavpack.c |   10 ++++++++--
>> >> > >  1 file changed, 8 insertions(+), 2 deletions(-)
>> >> > >
>> >> > > diff --git a/libavcodec/wavpack.c b/libavcodec/wavpack.c
>> >> > > index 47f598a..dd273f7 100644
>> >> > > --- a/libavcodec/wavpack.c
>> >> > > +++ b/libavcodec/wavpack.c
>> >> > > @@ -581,8 +581,14 @@ static inline int
>> >> > > wv_unpack_stereo(WavpackFrameContext
>> >> > > *s, GetBitContext *gb,
>> >> > >                      L2 = L + ((s->decorr[i].weightA * (int64_t)A
>> >> > > +
>> >> > > 512) >>
>> >> > > 10);
>> >> > >                      R2 = R + ((s->decorr[i].weightB * (int64_t)B
>> >> > > +
>> >> > > 512) >>
>> >> > > 10);
>> >> > >                  } else {
>> >> > > -                    L2 = L + ((s->decorr[i].weightA * A + 512) >>
>> >> > > 10);
>> >> > > -                    R2 = R + ((s->decorr[i].weightB * B + 512) >>
>> >> > > 10);
>> >> > > +                    int64_t Lt = s->decorr[i].weightA * (int64_t)A
>> >> > > +
>> >> > > 512;
>> >> > > +                    int64_t Rt = s->decorr[i].weightB * (int64_t)B
>> >> > > +
>> >> > > 512;
>> >> > > +                    if ((int32_t)Lt != Lt || (int32_t)Rt != Rt) {
>> >> > > +                        av_log(s->avctx, AV_LOG_ERROR, "sample
>> >> > > overflow %d
>> >> >
>> >> > This looks extremly ugly.
>> >>
>> >> Iam quite aware of that, which is part of the reason why i posted this
>> >> (aka do you know a less ugly solution?)
>> >
>> > ping
>> > anyone has a better solution ?
>> > do any objections to the original patch remain ?
>>
>> Yes, i see no point to apply this patch as is.
>
> what alternative solution do you suggest?

There is no alternative solution yet, as you did not provided any real
solution to this problem.

Why only this overflow in decoder need checking?

Why check is needed at all?