[FFmpeg-devel] [PATCH] Use AVERROR(ENOSYS) when it makes sense

[Howard Chu]

Michael Niedermayer wrote:
> On Mon, Apr 12, 2010 at 12:42:05AM +0200, Stefano Sabatini wrote:
>> On date Sunday 2010-04-11 23:33:15 +0200, Michael Niedermayer encoded:
>>> On Sun, Apr 11, 2010 at 10:13:30PM +0200, Stefano Sabatini wrote:
>>>> On date Saturday 2010-04-03 13:46:24 +0200, Stefano Sabatini encoded:
>>>>> Hi, as in subject.
>>>>> --
>>>>> FFmpeg = Fucking and Fabulous Moronic Peaceful Elitarian Genius
>>>>
>>>>>  From e66cec2193ee289f742120f92db17f33e7651fba Mon Sep 17 00:00:00 2001
>>>>> From: Stefano Sabatini<stefano.sabatini-lala at poste.it>
>>>>> Date: Tue, 16 Mar 2010 22:48:37 +0100
>>>>> Subject: [PATCH 2/5] Make url_seek() return AVERROR(ENOSYS) rather than AVERROR(EPIPE) in
>>>>>   the case where the seek operation is not defined in the protocol
>>>>>   handler.
>>>>>
>>>>> ---
>>>>>   libavformat/avio.c |    2 +-
>>>>>   1 files changed, 1 insertions(+), 1 deletions(-)
>>>>>
>>>>> diff --git a/libavformat/avio.c b/libavformat/avio.c
>>>>> index af9e049..3fb64d1 100644
>>>>> --- a/libavformat/avio.c
>>>>> +++ b/libavformat/avio.c
>>>>> @@ -201,7 +201,7 @@ int64_t url_seek(URLContext *h, int64_t pos, int whence)
>>>>>       int64_t ret;
>>>>>
>>>>>       if (!h->prot->url_seek)
>>>>> -        return AVERROR(EPIPE);
>>>>> +        return AVERROR(ENOSYS);
>>>>>       ret = h->prot->url_seek(h, pos, whence&  ~AVSEEK_FORCE);
>>>>>       return ret;
>>>>>   }
>>>>> --
>>>>> 1.7.0
>>>>>
>>>>
>>>>>  From f11131bc3a2e08f46b291e4fa849e7e8c642c757 Mon Sep 17 00:00:00 2001
>>>>> From: Stefano Sabatini<stefano.sabatini-lala at poste.it>
>>>>> Date: Tue, 16 Mar 2010 23:08:32 +0100
>>>>> Subject: [PATCH 3/5] Make url_fseek() return AVERROR(ENOSYS) rather than AVERROR(EPIPE) if
>>>>>   the seek operation is not defined in the ByteIOContext.
>>>>>
>>>>> ---
>>>>>   libavformat/aviobuf.c |    2 +-
>>>>>   1 files changed, 1 insertions(+), 1 deletions(-)
>>>>>
>>>>> diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
>>>>> index 37a6f6d..039b6d3 100644
>>>>> --- a/libavformat/aviobuf.c
>>>>> +++ b/libavformat/aviobuf.c
>>>>> @@ -160,7 +160,7 @@ int64_t url_fseek(ByteIOContext *s, int64_t offset, int whence)
>>>>>               return AVERROR_EOF;
>>>>>           s->buf_ptr = s->buf_end + offset - s->pos;
>>>>>       } else {
>>>>> -        int64_t res = AVERROR(EPIPE);
>>>>> +        int64_t res = AVERROR(ENOSYS);
>>>>>
>>>>>   #if CONFIG_MUXERS || CONFIG_NETWORK
>>>>>           if (s->write_flag) {
>>>>> --
>>>>> 1.7.0
>>>>>
>>>>
>>>>>  From 492f5063961b0d63fd6dda4404a107aa99484d40 Mon Sep 17 00:00:00 2001
>>>>> From: Stefano Sabatini<stefano.sabatini-lala at poste.it>
>>>>> Date: Tue, 16 Mar 2010 23:10:21 +0100
>>>>> Subject: [PATCH 4/5] Make url_fsize() return AVERROR(ENOSYS) rather than AVERROR(EPIPE) if
>>>>>   the seek operation is not defined in the ByteIOContext.
>>>>>
>>>>> ---
>>>>>   libavformat/aviobuf.c |    2 +-
>>>>>   1 files changed, 1 insertions(+), 1 deletions(-)
>>>>>
>>>>> diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
>>>>> index 039b6d3..0c46327 100644
>>>>> --- a/libavformat/aviobuf.c
>>>>> +++ b/libavformat/aviobuf.c
>>>>> @@ -197,7 +197,7 @@ int64_t url_fsize(ByteIOContext *s)
>>>>>           return AVERROR(EINVAL);
>>>>>
>>>>>       if (!s->seek)
>>>>> -        return AVERROR(EPIPE);
>>>>> +        return AVERROR(ENOSYS);
>>>>>       size = s->seek(s->opaque, 0, AVSEEK_SIZE);
>>>>>       if(size<0){
>>>>>           if ((size = s->seek(s->opaque, -1, SEEK_END))<  0)
>>>>
>>>> Ping.
>>>
>>> you are misusing ENOSYS
>>
>> This is possible, anyway how is EPIPE better than ENOSYS in this case?
>>
>> Please give some advice if you have any.
>
> ive none atm, just that i prefer to leave things as they are if they are
> wrong before and afte the patch. And i didnt really think much about it
> maybe EPIPE does make some sense dunno

Leaving it EPIPE makes for some other headaches. Most of the code uses ENOSYS 
for unimplemented functions, so using EPIPE here makes an awkward 
inconsistency. Callers expect ENOSYS.

aviobuf.c:737
int64_t av_url_read_fseek(ByteIOContext *s, int stream_index,
                           int64_t timestamp, int flags)
{
     URLContext *h = s->opaque;
     int64_t ret;
     if (!s->read_seek)
         return AVERROR(ENOSYS);
     ret = s->read_seek(h, stream_index, timestamp, flags);
     if(ret >= 0) {
         int64_t pos;
         s->buf_ptr = s->buf_end; // Flush buffer
         pos = s->seek(h, 0, SEEK_CUR);
         if (pos >= 0)
             s->pos = pos;
         else if (pos != AVERROR(ENOSYS) /* && pos != AVERROR(EPIPE) */)
             ret = pos;
     }
     return ret;
}

Callers expect ENOSYS:

asfdec.c:1129
static int asf_read_seek(AVFormatContext *s, int stream_index, int64_t pts, 
int flags)
{
     ASFContext *asf = s->priv_data;
     AVStream *st = s->streams[stream_index];
     int64_t pos;
     int index;

     if (s->packet_size <= 0)
         return -1;

     /* Try using the protocol's read_seek if available */
     if(s->pb) {
         int ret = av_url_read_fseek(s->pb, stream_index, pts, flags);
         if(ret >= 0)
             asf_reset_header(s);
         if (ret != AVERROR(ENOSYS))
             return ret;
     }

...








-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/