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

[Stefano Sabatini]

On date Saturday 2010-12-18 02:40:59 +0100, Michael Niedermayer encoded:
> 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

And I agree but note that:
av_file_map(filename, &buf, &size, log_offset, NULL);

will disable logging if log_offset is set to AVLOG_MAX-AVLOG_MIN+1,
that was the reason for which you wanted the log_offset thing, so it
doesn't require the user to do anything else with the FFmpeg log
system, and we could define a symbol for avoiding the use of the
clumsy AVLOG_MAX-AVLOG_MIN+1 expression.

Anyway since I lost enough time on this, if you don't want this I
propose to use it only internally (ff_file_map()) or I'll just
duplicate the code where I need it (vf_libopencv.c).
-- 
FFmpeg = Fantastic and Fundamentalist Mind-dumbing Portable Eretic Gadget