[FFmpeg-devel] [PATCH] Add av_file_get_size() and av_file_read(), replace cmdutils.h:read_file().

[Michael Niedermayer]

On Fri, Dec 17, 2010 at 12:52:19PM +0100, Stefano Sabatini wrote:
> On date Wednesday 2010-12-15 04:44:56 +0100, Michael Niedermayer encoded:
> > On Tue, Dec 14, 2010 at 11:52:31PM +0100, Stefano Sabatini wrote:
> > > On date Tuesday 2010-12-14 23:18:52 +0100, Michael Niedermayer encoded:
> > > > On Tue, Dec 14, 2010 at 01:48:52AM +0100, Stefano Sabatini wrote:
> > > [...]
> > > > > New try:
> > > > > int av_file_map(const char *filename, uint8_t **bufptr, size_t *size, int log_offset, void *log_ctx);
> > > > > void av_file_unmap(uint8_t *bufptr, size_t size);
> > > > > 
> > > > > this will simply achieve to create a writable buffer from the content
> > > > > of the file, buffer which will be completely decoupled from the file
> > > > > itself (that is: mmapped with MAP_PRIVATE), and which is closed before
> > > > > returning from av_file_map(), so there is no need to keep the filedes
> > > > > around anymore.
> > > > > 
> > > > > And if you don't like this design please *give more detailed
> > > > > indications* so we'll avoid to go around in circles and waste precious
> > > > > time and energy.
> > > > 
> > > > this is ok for private (ff_) API
> > > > for public API use of av_log() should be droped otherwise it is too
> > > > inconvenient to use in applications that dont use av_log() already.
> > > 
> > > what's the problem with:
> > > ret = av_file_map(filename, &buf, &size, log_offset, NULL);
> > > ?
> > 
> > bloated unneeded APItis
> > 
> > mmap and open use errno, a non libav based application that wants to replace
> > these 2 calls will not use a function that does funny callbacks to a funny
> > loging system that calls printf() by default
> > everyone will throw this in the trash bin where the other bloated intermingled
> > APIs are and they would be quite correct in doing so. 2 calls to open+mmap are
> > alot cleaner than this
> 
> Bah, every application using FFmpeg is supposed to map its own logging
> system to that of FFmpeg, simply returning an error doesn't give any
> hint about where the error occurred (opening the file?, file too big?,
> calling mmap()?).
> 
> Also since when av_log() is bloated? Are you suggesting to drop it
> from FFmpeg?

We dont disagree i think, we misunderstand each other.
Yes av_log() use in ffmpeg is all fine. But this is a file+mmap function this
is not ffmpeg specific nor multimedia specific
and there are plenty of applications that might want to
use libavutil and or a fopen+mmap simplification very few of them will switch
to av_log() for that.
libavutil and actually not only that should stay clean and free from making
all its innards interdepend on everything else. Keeping code selfcontained makes
it much easier to reuse parts in other applications.
think of libmpcodecs if you like, its interdependancies hit you quite hard.
If you write public API you should think of someone wanting to use it
and if you now say they are all ffmpeg users. You wanted to use libmpcodecs and
not in a mplayer fork or an application using mplayer ...
That said ive used code from libavutil in many little projects, it is usefull
code outside multimedia


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

I have often repented speaking, but never of holding my tongue.
-- Xenocrates
-------------- 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/20101218/c50ac5d3/attachment.pgp>