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

[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.
-- 
FFmpeg = Free and Fee Mean Proud Elected Goblin