[FFmpeg-devel] [PATCH] fix av_url_read_fseek

[Michael Niedermayer]

On Sun, Apr 11, 2010 at 04:17:41PM -0700, Howard Chu wrote:
> Michael Niedermayer wrote:
>> 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:
>>>>> 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
>
> -- 
>   -- Howard Chu
>   CTO, Symas Corp.           http://www.symas.com
>   Director, Highland Sun     http://highlandsun.com/hyc/
>   Chief Architect, OpenLDAP  http://www.openldap.org/project/

> Index: libavformat/aviobuf.c
> ===================================================================
> --- libavformat/aviobuf.c	(revision 22813)
> +++ libavformat/aviobuf.c	(working copy)
> @@ -698,8 +698,13 @@
>          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 >= 0)
> +            s->pos = pos;
> +        else if (pos != AVERROR(ENOSYS))
> +            ret = pos;
>      }

looks ok if it works & doesnt break any of the reg/fate tests

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

Many things microsoft did are stupid, but not doing something just because
microsoft did it is even more stupid. If everything ms did were stupid they
would be bankrupt already.
-------------- 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/20100412/2cb8e726/attachment.pgp>