Message ID | 20210811145412.271251-1-scerveau@collabora.com |
---|---|
State | New |
Headers | show |
Series | [FFmpeg-devel] avcodec/wma: handle run_level_decode error | expand |
Context | Check | Description |
---|---|---|
andriy/x86_make | success | Make finished |
andriy/x86_make_fate | success | Make fate finished |
andriy/PPC64_make | success | Make finished |
andriy/PPC64_make_fate | success | Make fate finished |
Stéphane Cerveau: > Consider data as invalid if ff_wma_run_level_decode > gets out with an error. > > It avoids an unpleasant sound distorsion. > > See http://trac.ffmpeg.org/ticket/9358 > --- > libavcodec/wmadec.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/libavcodec/wmadec.c b/libavcodec/wmadec.c > index d627bbe50e..74ea8cea77 100644 > --- a/libavcodec/wmadec.c > +++ b/libavcodec/wmadec.c > @@ -606,10 +606,11 @@ static int wma_decode_block(WMACodecContext *s) > * there is potentially less energy there */ > tindex = (ch == 1 && s->ms_stereo); > memset(ptr, 0, s->block_len * sizeof(WMACoef)); > - ff_wma_run_level_decode(s->avctx, &s->gb, &s->coef_vlc[tindex], > + if (ff_wma_run_level_decode(s->avctx, &s->gb, &s->coef_vlc[tindex], > s->level_table[tindex], s->run_table[tindex], > 0, ptr, 0, nb_coefs[ch], > - s->block_len, s->frame_len_bits, coef_nb_bits); > + s->block_len, s->frame_len_bits, coef_nb_bits)) > + return AVERROR_INVALIDDATA; > } > if (s->version == 1 && s->avctx->channels >= 2) > align_get_bits(&s->gb); > Generally, one should forward error codes and not make up some on the fly; in this case, the callee does not return proper error codes, so this should be fixed, too (but not in the same commit and not necessarily by you). Even more importantly, the callee emits an error message, stating that an error will be ignored, which will no longer true with this patch. - Andreas
Hi Andreas, > Generally, one should forward error codes and not make up some on the > fly; in this case, the callee does not return proper error codes, so > this should be fixed, too (but not in the same commit and not > necessarily by you). Even more importantly, the callee emits an error > message, stating that an error will be ignored, which will no longer > true with this patch. If I understand correctly, the "ignoring" refers to the block being ignored because of errors, not to the error being ignored. Is there any chance to get this into 4.4.1 ? Olivier On Wed, 2021-08-11 at 16:54 +0200, Stéphane Cerveau wrote: > Consider data as invalid if ff_wma_run_level_decode > gets out with an error. > > It avoids an unpleasant sound distorsion. > > See http://trac.ffmpeg.org/ticket/9358 > --- > libavcodec/wmadec.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/libavcodec/wmadec.c b/libavcodec/wmadec.c > index d627bbe50e..74ea8cea77 100644 > --- a/libavcodec/wmadec.c > +++ b/libavcodec/wmadec.c > @@ -606,10 +606,11 @@ static int wma_decode_block(WMACodecContext *s) > * there is potentially less energy there */ > tindex = (ch == 1 && s->ms_stereo); > memset(ptr, 0, s->block_len * sizeof(WMACoef)); > - ff_wma_run_level_decode(s->avctx, &s->gb, &s->coef_vlc[tindex], > + if (ff_wma_run_level_decode(s->avctx, &s->gb, &s->coef_vlc[tindex], > s->level_table[tindex], s->run_table[tindex], > 0, ptr, 0, nb_coefs[ch], > - s->block_len, s->frame_len_bits, coef_nb_bits); > + s->block_len, s->frame_len_bits, coef_nb_bits)) > + return AVERROR_INVALIDDATA; > } > if (s->version == 1 && s->avctx->channels >= 2) > align_get_bits(&s->gb);
Hi, On Tue, Aug 24, 2021 at 9:07 AM Olivier Crête <olivier.crete@collabora.com> wrote: > Hi Andreas, > > > Generally, one should forward error codes and not make up some on the > > fly; in this case, the callee does not return proper error codes, so > > this should be fixed, too (but not in the same commit and not > > necessarily by you). Even more importantly, the callee emits an error > > message, stating that an error will be ignored, which will no longer > > true with this patch. > > If I understand correctly, the "ignoring" refers to the block being > ignored because of errors, not to the error being ignored. > No, the opposite. He suggests to try: ret = try_decode_wma(); if (ret < 0) return ret; instead of: if (ret < 0) return AVERROR_INVALIDDATA; Ronald
diff --git a/libavcodec/wmadec.c b/libavcodec/wmadec.c index d627bbe50e..74ea8cea77 100644 --- a/libavcodec/wmadec.c +++ b/libavcodec/wmadec.c @@ -606,10 +606,11 @@ static int wma_decode_block(WMACodecContext *s) * there is potentially less energy there */ tindex = (ch == 1 && s->ms_stereo); memset(ptr, 0, s->block_len * sizeof(WMACoef)); - ff_wma_run_level_decode(s->avctx, &s->gb, &s->coef_vlc[tindex], + if (ff_wma_run_level_decode(s->avctx, &s->gb, &s->coef_vlc[tindex], s->level_table[tindex], s->run_table[tindex], 0, ptr, 0, nb_coefs[ch], - s->block_len, s->frame_len_bits, coef_nb_bits); + s->block_len, s->frame_len_bits, coef_nb_bits)) + return AVERROR_INVALIDDATA; } if (s->version == 1 && s->avctx->channels >= 2) align_get_bits(&s->gb);