[FFmpeg-devel] [PATCH][1/4] URLProtocol extensions

[Björn Axelsson]

On Thu, 2007-11-22 at 19:12 +0100, Michael Niedermayer wrote:
> On Thu, Nov 22, 2007 at 02:49:01PM +0000, Bj?rn Axelsson wrote:
> > Extend URLProtocol with new function pointers and api functions for
> > url_read_play(), url_read_pause() and url_read_seek().
> 
> please give the functions some prefix like av_
> doing something stupid because the code is then consistently stupid
> does not seem like a very good idea IMHO ;)

Argh. Inconsistency vs stupidity is not a fun choice. How about
prefixing all the avio functions with "av_" once and for all while we
are still close to a major version increase? 

Anyhow, changed to names like av_url_read_play().

> [...]
> > +int url_read_seek(URLContext *h,
> > +        int stream_index, int64_t timestamp, int flags)
> 
> nitpick:
> 
> int url_read_seek(URLContext *h,
>                   int stream_index, int64_t timestamp, int flags)

Addressed in patch.

> [...]
> > +/**
> > + * Seek to a given timestamp relative to some component stream.
> > + * Only meaningful if using a network streaming protocol (e.g. MMS).
> 
> > + * @param stream_index The stream index that the timestamp is relative to.
> > + * May be -1, in which case the timestamp is interpreted as time from the
> > + * beginning of the presentation. This parameter is not guaranteed to be
> > + * supported by the protocol, in which case it will be ignored.
> 
> what happens with the units in which timestamp is specified ?
> with stream_index== -1 its AV_TIME_BASE with != -1 its (or should be) the
> timebase of the stream

Didn't think about that. Changed the spec so that the protocol can
return ENOTSUP if a stream index is given (or if -1 is used). That way
the caller can try the other way before giving up.

> > + * @param flags selects which direction should be preferred if no exact
> > + * match is available. This parameter is not guaranteed to be supported by the
> > + * protocol, in which case it will be ignored.
> > + * @return >= 0 on success (but not necessarily the new offset)
> > + */
> > +int url_read_seek(URLContext *h,
> > +        int stream_index, int64_t timestamp, int flags);
> > +
> 
> if the return should optionally be the new offset then it should be int64_t

Returning the new offset is not optional. Comment (and return type) was
copied from AVInputFormat's read_seek documentation. Perhaps removing
the comment makes it less ambigous.

Updated patch attached. I'll update the others when this passes.

-- 
Bj?rn Axelsson                    Phone: +46-(0)90-18 98 97
Intinor AB                          Fax: +46-(0)920-757 10
www.intinor.se
Interactive Television & Digital Media Distribution
-------------- next part --------------
A non-text attachment was scrubbed...
Name: urlprotocol_api_extension.diff
Type: text/x-patch
Size: 3159 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20071123/37e2a223/attachment.bin>