[FFmpeg-devel] [PATCH] Make wmaprodec.c:decode_init() return AVERROR_INVALIDDATA in case of illegal number of channels

[Sascha Sommer]

Hi,

Am Sonntag 14 M?rz 2010 12:42:11 schrieb Stefano Sabatini:
> On date Saturday 2010-03-13 23:00:16 +0100, Michael Niedermayer encoded:
> > On Sat, Mar 13, 2010 at 09:53:11PM +0100, Stefano Sabatini wrote:
> > > Hi, as in subject.
> > >
> > > This is consistent with what is done in most of cases, and more
> >
> > consistency is the bane of correctness
> >
> > invalid and not supported are 2 quite differnt things, nothing in your
> > mail explains why what we have is incorrect and if its correct then the
> > change is wrong.
> 
> The code:
>     if (s->num_channels < 0 || s->num_channels > WMAPRO_MAX_CHANNELS) {
>         av_log_ask_for_sample(avctx, "invalid number of channels\n");
>         return AVERROR_NOTSUPP;
>     }
> 
> In other parts of the file, whenever av_log_ask_for_sample() is
> provided usually AVERROR_INVALIDDATA is returned.
> 
> This specific error may mean:
> 
> 1) the value for the channel is illegal, so the data is to be
>    considered invalid and AVERROR_INVALIDDATA should be returned, *or*
>    we don't know how to deal with such a value so we consider this
>    invalid data.
> 
> 2) the value for the channel is valid, but we currently do not support
>    decoding when that value is set, in this case AVERROR_PATCHWELCOME
>    should be returned.
> 
> 3) the value for the channel *may* be valid but we don't know as the
>    codec is experimental and it's been implemented by RE, so there is
>    chance that that value is currently legal but we don't know how to
>    interpret it. In this case we should return AVERROR_NOTSUPP, and
>    ask for a sample for inspecting more.
> 
>    After more time we may discover out how to deal with that value and
>    remove the ask-for-sample, *or* we may realize that that value is
>    illegal, and return AVERROR_INVALIDDATA.
> 
> Interpretation 1) seemed the more correct to me, but I'm aware that
> the interpretation of error code is rather subtle and documentation
> lacks.
> 
> (BTW as for what regards POSIX error codes I'm using these references:
> http://www.opengroup.org/onlinepubs/9699919799/basedefs/errno.h.html
> http://www.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html#tag
> _15_03 )
> 
> Regards.
> 

According to http://en.wikipedia.org/wiki/Windows_Media_Audio, wmapro files 
can have more than 8 channels but I never saw such a file in the wild.
I therefore think that AVERROR_INVALIDDATA is wrong.
It could be replaced with AVERROR_PATCHWELCOME, though.

Regards

Sascha