[FFmpeg-devel] [RFC] The meaning of AVERROR_NOTSUPP

[Stefano Sabatini]

On date Tuesday 2010-03-23 00:57:17 +0100, Stefano Sabatini encoded:
> On date Monday 2010-03-22 10:27:03 +0100, Michael Niedermayer encoded:
> > On Sun, Mar 21, 2010 at 10:51:12PM +0100, Stefano Sabatini wrote:
> > > On date Sunday 2010-03-21 22:38:28 +0100, Michael Niedermayer encoded:
[...]
> > AVERROR_NOTSUPP feels more toward a feature that can be supported but is not
> > we also might want to have a 
> > AVERROR_DISABLED for a feature suported but not compiled in
> > but then there is AVERROR_PATCHWELCOME and iam not sure what AVERROR_NOTSUPP is
> > supposed to mean then
> > anyway, error docs are poor and you are changing error code without their
> > meaning being clearly documented.
> 
> Description for AVERROR_NOTSUPP:
> #define AVERROR_NOTSUPP     AVERROR(ENOSYS)  /**< Operation not supported. */
> 
> Description for AVERROR_PATCHWELCOME:
> #define AVERROR_PATCHWELCOME    (-MKTAG('P','A','W','E')) /**< Not yet implemented in FFmpeg. Patches welcome. */
> 
> My interpretation of AVERROR_NOTSUPP:
> The *operation* requested is not legal/valid, as it is not supported
> by the element being requested.
> 
> So AVERROR_NOTSUPP should be returned just in case a certain operation
> is requested to an object for which that operation doesn't make sense
> / cannot be implemented, and in this sense is semantically near to the
> meaning of ENOSYS.
> 
> Think for example of a seek operation in a pipe, for which
> AVERROR_PATCHWELCOME wouldn't make sense.
> 
> I recognize though that the term "supported" in the "not supported"
> expression may lead to confusion, as it suggests what you said, so I'd
> suggest to use instead AVERROR_ILLEGALOPERATION or
> AVERROR_INVALIDOPERATION.
> 
> As for the distinction between AVERROR_DISABLED and
> AVERROR_PATCHWELCOME, I don't think that would be a good idea
> implementation-wise, as most of the time a program can't tell the
> distinction amongst the two.
> 
> For example av_error_input_format() doesn't know if it can't recognize
> a format because it has not been compiled in, because the format is
> not implemented, or because we inflicted to him bogus random data.
> 
> If you agree with the semantics I proposed, I'll post a patch for
> clarifying the description of AVERROR_NOTSUPP, also tell me what do
> you think about the AVERROR_INVALIDOPERATION rename.

Currently we have just one use of AVERROR_NOTSUPP:

in libavformat/file.c:
static int64_t file_seek(URLContext *h, int64_t pos, int whence)
{
    int fd = (intptr_t) h->priv_data;
    if (whence != SEEK_SET && whence != SEEK_CUR && whence != SEEK_END)
        return AVERROR_NOTSUPP;
    return lseek(fd, pos, whence);
}

This suggests the interpretation of AVERROR_NOTSUPP as "Operation
non-valid or illegal".

Currently AVERROR_NOTSUPP is defined as AVERROR(ENOSYS), definition from [1]:
|[ENOSYS]
| Function not implemented. An attempt was made to use a function that
| is not available in this implementation.

Then we have the description of ENOTSUP, lexically close to AVERROR_NOTSUPP:
|[ENOTSUP]
| Not supported. The implementation does not support this feature of
| the Realtime Option Group.

Both these descriptions have some resemblance with the interpretation
of AVERROR_NOTSUPP -> feature not implemented.

Since the semantics of AVERROR_NOTSUPP -> feature not
implemented is already covered by AVERROR_PATCHWELCOME, I suggest to
prefer the semantics of AVERROR_NOTSUPP -> operation non-valid or
illegal.

So I suggest to *rename* the symbol:
AVERROR_NOTSUPP -> AVERROR_INVALIDOPERATION

and drop the old definition at the next major bump, and adopt the
following description:
"Invalid or illegal operation requested. An operation has been
requested which does not make sense in the given context or cannot be
implemented in the requested object."

Comments are welcome, regards.

[1] IEEE 1003.1 error code descriptions: http://www.opengroup.org/onlinepubs/000095399/functions/xsh_chap02_03.html
-- 
FFmpeg = Faithless Fast Multimedia Philosophical Elected Geek