Re: [FFmpeg-devel] [PATCH V1] lavc/mpeg4audio: add chan_config check to avoid indeterminate channels

2019-09-27 Thread James Almer
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

2019-09-21 Thread James Almer
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

2019-09-21 Thread James Almer
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

2019-09-21 Thread Moritz Barsnick
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

2019-09-21 Thread Jun Zhao
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".