[FFmpeg-devel] [PATCH] fix av_url_read_fseek

[Michael Niedermayer]

On Sat, Apr 10, 2010 at 06:57:09PM -0700, Howard Chu wrote:
> Michael Niedermayer wrote:
>> On Sat, Apr 10, 2010 at 04:37:36PM -0700, Howard Chu wrote:
>>> Michael Niedermayer wrote:
>>>> On Sat, Apr 10, 2010 at 09:26:31AM -0700, Howard Chu wrote:
>>>>> Index: libavformat/aviobuf.c
>>>>> ===================================================================
>>>>> --- libavformat/aviobuf.c	(revision 22813)
>>>>> +++ libavformat/aviobuf.c	(working copy)
>>>>> @@ -698,8 +698,11 @@
>>>>>            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
>>>>> -        s->pos = s->seek(h, 0, SEEK_CUR);
>>>>> +        pos = s->seek(h, 0, SEEK_CUR);
>>>>> +        if (pos != AVERROR(ENOSYS))
>>>>> +            s->pos = pos;
>>>>>        }
>>>>>        return ret;
>>>>>    }
>>>>
>>>> this should be a seperate patch
>>>
>>> Protocols that implement read_seek are unlikely to also implement seek, 
>>> so
>>> this patch is needed to prevent the pos from getting set to a negative
>>> value. Possibly it should just check for pos<= 0 instead of checking for
>>> any specific error code. It's not clear that there's any real point to
>>> calling seek here anyway, any stream that knows the value of SEEK_CUR 
>>> would
>>> have already updated itself while executing the read_seek.
>
>> should that check not be<0 ?
>
> Probably. As I already noted above, I have a lot of doubts about this chunk 
> of code. Maybe better would be something like:
>
>   if (pos >= 0)
>      s->pos = pos
>   else if (pos != AVERROR(ENOSYS))
>      ret = pos;
>
> I.e., if seek is actually implemented and fails, that probably should be 
> reported to the caller.

what you say sounds reasonable at first glance

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If a bugfix only changes things apparently unrelated to the bug with no
further explanation, that is a good sign that the bugfix is wrong.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100411/20b55fd9/attachment.pgp>