| Message ID | 1481894385-64104-2-git-send-email-rsbultje@gmail.com |
|---|---|
| State | New |
| Headers | show |
On 16.12.2016 14:19, Ronald S. Bultje wrote: > This reverts 2a4700a4f03280fa8ba4fc0f8a9987bb550f0d1e and implements it > correctly so streams actually decode the way the encoder intended them > to. Why is it correct? Is this behavior defined in a specification or reference decoder? > --- > libavcodec/wmavoice.c | 20 ++++---------------- > 1 file changed, 4 insertions(+), 16 deletions(-) > > diff --git a/libavcodec/wmavoice.c b/libavcodec/wmavoice.c > index 0f29bdd..f1b5369 100644 > --- a/libavcodec/wmavoice.c > +++ b/libavcodec/wmavoice.c > @@ -1900,16 +1900,10 @@ static int wmavoice_decode_packet(AVCodecContext *ctx, void *data, > cnt += s->spillover_nbits; > s->skip_bits_next = cnt & 7; > res = cnt >> 3; > - if (res > avpkt->size) { > - av_log(ctx, AV_LOG_ERROR, > - "Trying to skip %d bytes in packet of size %d\n", > - res, avpkt->size); > - return AVERROR_INVALIDDATA; > - } > - return res; > + return FFMIN(avpkt->size, res); > } else > - skip_bits_long (gb, s->spillover_nbits - cnt + > - get_bits_count(gb)); // resync > + skip_bits_long(gb, s->spillover_nbits - cnt + > + get_bits_count(gb)); // resync Please do cosmetic changes in a separate commit. Best regards, Andreas
2016-12-19 23:45 GMT+01:00 Andreas Cadhalpun <andreas.cadhalpun@googlemail.com>: > On 16.12.2016 14:19, Ronald S. Bultje wrote: >> This reverts 2a4700a4f03280fa8ba4fc0f8a9987bb550f0d1e and implements it >> correctly so streams actually decode the way the encoder intended them >> to. > > Why is it correct? Because the patch introduced an (imo horrible) regression, files that before reported why they couldn't be played suddenly gave no explanation, just a useless error message. Carl Eugen
On 20.12.2016 00:59, Carl Eugen Hoyos wrote: > 2016-12-19 23:45 GMT+01:00 Andreas Cadhalpun <andreas.cadhalpun@googlemail.com>: >> On 16.12.2016 14:19, Ronald S. Bultje wrote: >>> This reverts 2a4700a4f03280fa8ba4fc0f8a9987bb550f0d1e and implements it >>> correctly so streams actually decode the way the encoder intended them >>> to. >> >> Why is it correct? > > Because the patch introduced an (imo horrible) regression, I don't think that not triggering an av_assert0 is a regression. > files that before reported why they couldn't be played suddenly > gave no explanation, just a useless error message. An error message is not no explanation. Best regards, Andreas
2016-12-20 1:10 GMT+01:00 Andreas Cadhalpun <andreas.cadhalpun@googlemail.com>: > On 20.12.2016 00:59, Carl Eugen Hoyos wrote: >> 2016-12-19 23:45 GMT+01:00 Andreas Cadhalpun <andreas.cadhalpun@googlemail.com>: >>> On 16.12.2016 14:19, Ronald S. Bultje wrote: >>>> This reverts 2a4700a4f03280fa8ba4fc0f8a9987bb550f0d1e and implements it >>>> correctly so streams actually decode the way the encoder intended them >>>> to. >>> >>> Why is it correct? >> >> Because the patch introduced an (imo horrible) regression, > > I don't think that not triggering an av_assert0 is a regression. > >> files that before reported why they couldn't be played suddenly >> gave no explanation, just a useless error message. > > An error message is not no explanation. Sorry! I was seriously expecting Ronald to fix 04e9853a - an untested broken change - but I was wrong. Carl Eugen
diff --git a/libavcodec/wmavoice.c b/libavcodec/wmavoice.c index 0f29bdd..f1b5369 100644 --- a/libavcodec/wmavoice.c +++ b/libavcodec/wmavoice.c @@ -1900,16 +1900,10 @@ static int wmavoice_decode_packet(AVCodecContext *ctx, void *data, cnt += s->spillover_nbits; s->skip_bits_next = cnt & 7; res = cnt >> 3; - if (res > avpkt->size) { - av_log(ctx, AV_LOG_ERROR, - "Trying to skip %d bytes in packet of size %d\n", - res, avpkt->size); - return AVERROR_INVALIDDATA; - } - return res; + return FFMIN(avpkt->size, res); } else - skip_bits_long (gb, s->spillover_nbits - cnt + - get_bits_count(gb)); // resync + skip_bits_long(gb, s->spillover_nbits - cnt + + get_bits_count(gb)); // resync } else skip_bits_long(gb, s->spillover_nbits); // resync } @@ -1926,13 +1920,7 @@ static int wmavoice_decode_packet(AVCodecContext *ctx, void *data, int cnt = get_bits_count(gb); s->skip_bits_next = cnt & 7; res = cnt >> 3; - if (res > avpkt->size) { - av_log(ctx, AV_LOG_ERROR, - "Trying to skip %d bytes in packet of size %d\n", - res, avpkt->size); - return AVERROR_INVALIDDATA; - } - return res; + return FFMIN(res, avpkt->size); } else if ((s->sframe_cache_size = pos) > 0) { /* rewind bit reader to start of last (incomplete) superframe... */ init_get_bits(gb, avpkt->data, size << 3);