[FFmpeg-devel] [FFmpeg-commits] lavf: replace all uses of url_fskip with avio_seek

[Anton Khirnov]

On Thu, Mar 03, 2011 at 02:08:19AM +1100, Peter Ross wrote:
> On Tue, Mar 01, 2011 at 06:32:10PM +0100, Anton Khirnov  wrote:
> > Module: ffmpeg
> > Branch: master
> > Commit: e356fc57a2e9887370caec58d8aafeafd1f336dc
> > 
> > Author:    Anton Khirnov <anton at khirnov.net>
> > Committer: Ronald S. Bultje <rsbultje at gmail.com>
> > Date:      Mon Feb 28 14:57:55 2011 +0100
> > 
> > lavf: replace all uses of url_fskip with avio_seek
> > 
> > Signed-off-by: Ronald S. Bultje <rsbultje at gmail.com>
> > 
> > ---
> 
> [..]
> 
> >  libavformat/vocdec.c         |    8 +++---
> >  libavformat/vqf.c            |    6 ++--
> >  libavformat/wav.c            |    6 ++--
> >  libavformat/wtv.c            |   58 +++++++++++++++++++++---------------------
> 
> Guys, the patch subject indicates what was done, but not why.
> Can somebody explain WHY the fskip convention has been replaced
> with seek. Im ignoring the original patch thread because that
> has gone political.
> 
> This function has existed since I started using FFmpeg a decade ago.
> In all my demuxers url_fskip has been used to skip a block of unknown
> or irrelevant data. I don't 'seek' to another location in the file,
> I 'skip' over the data. The intent of the code becomes less obvious
> when fseek is used in both instances.
> 
> Fskip has never been a problem. What possible benefit does this
> change give us? Objectively please!
> 
> By logical extension, the next patch in this series will be to
> universally replace get_bits1(gb) with get_bits(gb, 1). Seriously,
> why not?
> 
> I am not one normally to be critical of API and broader framework
> changes, and dont have the time to read all mails, but this change,
> and the get/put_tag renaming insanity really pinches a nerve. This
> is a backward step on the master public API.
> 
> Its really late here -- expect an avio_skip() patch from me tomorrow.
> 

The old code was very inconsistent in this regard -- about half of it
used skip(), while the other half used seek(SEEK_CUR). It seemed easier
and better to just eliminate the skip().
If you care so much, I don't mind if reintroduce it, but then please use it
consistently everywhere.

-- 
Anton Khirnov
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20110302/2a0f230f/attachment.pgp>