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

[Stefano Sabatini]

On date Saturday 2010-03-27 00:51:13 +0100, Stefano Sabatini encoded:
> 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

See attached patches.

Regards.
-- 
FFmpeg = Faithful Fierce Multimedia Picky Erotic Gnome