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

[Michael Niedermayer]

On Fri, Nov 23, 2007 at 02:19:03PM +0000, Bj?rn Axelsson wrote:
> 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? 

ok


> 
> 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.

patch looks ok


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

Frequently ignored awnser#1 FFmpeg bugs should be sent to our bugtracker, user
questions for the command line tools ffmpeg, ffplay, ... as well as questions
about how to use libav* should be sent to the ffmpeg-user mailinglist.
-------------- 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/20071123/c843c2b6/attachment.pgp>