| Message ID | 20210824134343.190946-2-olivier.crete@collabora.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/2] avcodec/wma: Return specific error code | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | fail | Make fate failed |
| andriy/make_ppc | success | Make finished |
| andriy/make_fate_ppc | fail | Make fate failed |
lgtm
On Tue, Aug 24, 2021 at 09:43:43AM -0400, Olivier Crête wrote: > From: Stéphane Cerveau <scerveau@collabora.com> > > 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 | 11 +++++++---- > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/libavcodec/wmadec.c b/libavcodec/wmadec.c > index d627bbe50e..d43fe239ce 100644 > --- a/libavcodec/wmadec.c > +++ b/libavcodec/wmadec.c > @@ -601,15 +601,18 @@ static int wma_decode_block(WMACodecContext *s) > if (s->channel_coded[ch]) { > int tindex; > WMACoef *ptr = &s->coefs1[ch][0]; > + int ret; > > /* special VLC tables are used for ms stereo because > * 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], > - s->level_table[tindex], s->run_table[tindex], > - 0, ptr, 0, nb_coefs[ch], > - s->block_len, s->frame_len_bits, coef_nb_bits); > + ret = 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); > + if (ret < 0) > + return ret; > } > if (s->version == 1 && s->avctx->channels >= 2) > align_get_bits(&s->gb); Does this just discard the packet or replace by silence ? if it discards it, it can cause problems with av-sync [...]
On Thu, 2021-08-26 at 12:33 +0200, Michael Niedermayer wrote: > On Tue, Aug 24, 2021 at 09:43:43AM -0400, Olivier Crête wrote: > > From: Stéphane Cerveau <scerveau@collabora.com> > > > > 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 | 11 +++++++---- > > 1 file changed, 7 insertions(+), 4 deletions(-) > > > > diff --git a/libavcodec/wmadec.c b/libavcodec/wmadec.c > > index d627bbe50e..d43fe239ce 100644 > > --- a/libavcodec/wmadec.c > > +++ b/libavcodec/wmadec.c > > @@ -601,15 +601,18 @@ static int wma_decode_block(WMACodecContext *s) > > if (s->channel_coded[ch]) { > > int tindex; > > WMACoef *ptr = &s->coefs1[ch][0]; > > + int ret; > > > > /* special VLC tables are used for ms stereo because > > * 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], > > - s->level_table[tindex], s->run_table[tindex], > > - 0, ptr, 0, nb_coefs[ch], > > - s->block_len, s->frame_len_bits, coef_nb_bits); > > + ret = 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); > > + if (ret < 0) > > + return ret; > > } > > if (s->version == 1 && s->avctx->channels >= 2) > > align_get_bits(&s->gb); > > Does this just discard the packet or replace by silence ? > if it discards it, it can cause problems with av-sync It just discards it, but it seems that the Microsoft implementation has the same behaviour, so I think it's better to be bug compatible with them.
On Thu, Aug 26, 2021 at 10:11:40AM -0400, Olivier Crête wrote: > On Thu, 2021-08-26 at 12:33 +0200, Michael Niedermayer wrote: > > On Tue, Aug 24, 2021 at 09:43:43AM -0400, Olivier Crête wrote: > > > From: Stéphane Cerveau <scerveau@collabora.com> > > > > > > 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 | 11 +++++++---- > > > 1 file changed, 7 insertions(+), 4 deletions(-) > > > > > > diff --git a/libavcodec/wmadec.c b/libavcodec/wmadec.c > > > index d627bbe50e..d43fe239ce 100644 > > > --- a/libavcodec/wmadec.c > > > +++ b/libavcodec/wmadec.c > > > @@ -601,15 +601,18 @@ static int wma_decode_block(WMACodecContext *s) > > > if (s->channel_coded[ch]) { > > > int tindex; > > > WMACoef *ptr = &s->coefs1[ch][0]; > > > + int ret; > > > > > > /* special VLC tables are used for ms stereo because > > > * 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], > > > - s->level_table[tindex], s->run_table[tindex], > > > - 0, ptr, 0, nb_coefs[ch], > > > - s->block_len, s->frame_len_bits, coef_nb_bits); > > > + ret = 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); > > > + if (ret < 0) > > > + return ret; > > > } > > > if (s->version == 1 && s->avctx->channels >= 2) > > > align_get_bits(&s->gb); > > > > Does this just discard the packet or replace by silence ? > > if it discards it, it can cause problems with av-sync > > It just discards it, but > it seems that the Microsoft implementation has > the same behaviour, so I think it's better to be bug compatible with > them. thats a good argument, indeed thx [...]
diff --git a/libavcodec/wmadec.c b/libavcodec/wmadec.c index d627bbe50e..d43fe239ce 100644 --- a/libavcodec/wmadec.c +++ b/libavcodec/wmadec.c @@ -601,15 +601,18 @@ static int wma_decode_block(WMACodecContext *s) if (s->channel_coded[ch]) { int tindex; WMACoef *ptr = &s->coefs1[ch][0]; + int ret; /* special VLC tables are used for ms stereo because * 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], - s->level_table[tindex], s->run_table[tindex], - 0, ptr, 0, nb_coefs[ch], - s->block_len, s->frame_len_bits, coef_nb_bits); + ret = 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); + if (ret < 0) + return ret; } if (s->version == 1 && s->avctx->channels >= 2) align_get_bits(&s->gb);
From: Stéphane Cerveau <scerveau@collabora.com> 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 | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)