Re: [FFmpeg-devel] [PATCH V1] lavc/mpeg4audio: add chan_config check to avoid indeterminate channels
On 9/21/2019 8:29 AM, Jun Zhao wrote: > From: Jun Zhao > > add chan_config check to avoid indeterminate channels. > > Signed-off-by: Jun Zhao > --- > libavcodec/mpeg4audio.c |4 > 1 files changed, 4 insertions(+), 0 deletions(-) > > diff --git a/libavcodec/mpeg4audio.c b/libavcodec/mpeg4audio.c > index 2197147..0ada239 100644 > --- a/libavcodec/mpeg4audio.c > +++ b/libavcodec/mpeg4audio.c > @@ -93,6 +93,10 @@ int ff_mpeg4audio_get_config_gb(MPEG4AudioConfig *c, > GetBitContext *gb, > c->chan_config = get_bits(gb, 4); > if (c->chan_config < FF_ARRAY_ELEMS(ff_mpeg4audio_channels)) > c->channels = ff_mpeg4audio_channels[c->chan_config]; > +else { > +av_log(NULL, AV_LOG_ERROR, "Invalid chan_config %d\n", > c->chan_config); > +return -1; > +} > c->sbr = -1; > c->ps = -1; > if (c->object_type == AOT_SBR || (c->object_type == AOT_PS && Amended with a log context and applied, thanks. ___ 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".
Re: [FFmpeg-devel] [PATCH V1] lavc/mpeg4audio: add chan_config check to avoid indeterminate channels
On 9/21/2019 8:29 AM, Jun Zhao wrote: > From: Jun Zhao > > add chan_config check to avoid indeterminate channels. > > Signed-off-by: Jun Zhao > --- > libavcodec/mpeg4audio.c |4 > 1 files changed, 4 insertions(+), 0 deletions(-) > > diff --git a/libavcodec/mpeg4audio.c b/libavcodec/mpeg4audio.c > index 2197147..0ada239 100644 > --- a/libavcodec/mpeg4audio.c > +++ b/libavcodec/mpeg4audio.c > @@ -93,6 +93,10 @@ int ff_mpeg4audio_get_config_gb(MPEG4AudioConfig *c, > GetBitContext *gb, > c->chan_config = get_bits(gb, 4); > if (c->chan_config < FF_ARRAY_ELEMS(ff_mpeg4audio_channels)) > c->channels = ff_mpeg4audio_channels[c->chan_config]; > +else { > +av_log(NULL, AV_LOG_ERROR, "Invalid chan_config %d\n", > c->chan_config); Is chan_config > 8 invalid, or currently unsupported instead? > +return -1; > +} > c->sbr = -1; > c->ps = -1; > if (c->object_type == AOT_SBR || (c->object_type == AOT_PS && > ___ 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".
Re: [FFmpeg-devel] [PATCH V1] lavc/mpeg4audio: add chan_config check to avoid indeterminate channels
On 9/21/2019 10:07 AM, Moritz Barsnick wrote: > On Sat, Sep 21, 2019 at 19:29:47 +0800, Jun Zhao wrote: >> +else { >> +av_log(NULL, AV_LOG_ERROR, "Invalid chan_config %d\n", >> c->chan_config); >> +return -1; >> +} > > I know the function definition says it returns -1 on error, but that's > already not the case: It can return AVERROR_INVALIDDATA by means of the > call to parse_config_ALS(). I believe the doc should be changed, and > this code change should also return AVERROR_INVALIDDATA. > > Furthermore, can you pass and find a useful context for this av_log() > (and for the one in parse_config_ALS()? av_log() with NULL context is > very unfortunate. avpriv_mpeg4audio_get_config() is tied to the ABI, so such changes can only happen after a major bump. It could be done to ff_mpeg4audio_get_config_gb() in the meantime, using a new logctx paramtere, and schedule the addition to avpriv_mpeg4audio_get_config() with a preprocessor check. I'll look into doing that in a bit. > > Cheers, > Moritz > ___ > 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".
Re: [FFmpeg-devel] [PATCH V1] lavc/mpeg4audio: add chan_config check to avoid indeterminate channels
On Sat, Sep 21, 2019 at 19:29:47 +0800, Jun Zhao wrote: > +else { > +av_log(NULL, AV_LOG_ERROR, "Invalid chan_config %d\n", > c->chan_config); > +return -1; > +} I know the function definition says it returns -1 on error, but that's already not the case: It can return AVERROR_INVALIDDATA by means of the call to parse_config_ALS(). I believe the doc should be changed, and this code change should also return AVERROR_INVALIDDATA. Furthermore, can you pass and find a useful context for this av_log() (and for the one in parse_config_ALS()? av_log() with NULL context is very unfortunate. Cheers, Moritz ___ 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] [PATCH V1] lavc/mpeg4audio: add chan_config check to avoid indeterminate channels
From: Jun Zhao add chan_config check to avoid indeterminate channels. Signed-off-by: Jun Zhao --- libavcodec/mpeg4audio.c |4 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/libavcodec/mpeg4audio.c b/libavcodec/mpeg4audio.c index 2197147..0ada239 100644 --- a/libavcodec/mpeg4audio.c +++ b/libavcodec/mpeg4audio.c @@ -93,6 +93,10 @@ int ff_mpeg4audio_get_config_gb(MPEG4AudioConfig *c, GetBitContext *gb, c->chan_config = get_bits(gb, 4); if (c->chan_config < FF_ARRAY_ELEMS(ff_mpeg4audio_channels)) c->channels = ff_mpeg4audio_channels[c->chan_config]; +else { +av_log(NULL, AV_LOG_ERROR, "Invalid chan_config %d\n", c->chan_config); +return -1; +} c->sbr = -1; c->ps = -1; if (c->object_type == AOT_SBR || (c->object_type == AOT_PS && -- 1.7.1 ___ 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".