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

[Michael Niedermayer]

On Sat, Dec 18, 2010 at 11:58:38AM +0100, Stefano Sabatini wrote:
> 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,

Why do you use av_log for this at all?
Its not the right tool for the job here
the error is in errno and perror() and strerror(_r)()/av_log(strerr...())
are the correct functions to present that to the user
calling av_log() in there could even change errno if for example it tries to
write to a file and that fails. But thats not the point the point is
public API is intended to be used and this av_log use makes it really annoying
for most cases where one might want to use it.

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

it is not once nor twice but times without number that the same ideas make
their appearance in the world. -- Aristotle
-------------- 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/20101219/46a7553b/attachment.pgp>