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

[Michael Niedermayer]

On Tue, Dec 14, 2010 at 01:48:52AM +0100, Stefano Sabatini wrote:
> On date Monday 2010-12-13 14:23:06 +0100, Michael Niedermayer encoded:
> > On Mon, Dec 13, 2010 at 12:48:25AM +0100, Stefano Sabatini wrote:
> [...]
> > > > > > > +int av_file_map(const char *filename, uint8_t **bufptr, size_t *size, int protect,
> > > > > > > +                void *log_ctx);
> > > > > > > +
> > > > > > > +/**
> > > > > > > + * Unmap the file with filedescriptor fd, and free the allocated or
> > > > > > > + * mmapped buffer in *bufptr with size size.
> > > > > > > + */
> > > > > > > +void av_file_unmap(int fd, uint8_t *bufptr, size_t size);
> > > > > 
> > > > > I'm not sure it's a good idea to keep fd as we could simply close the
> > > > > file at the end of av_file_map().
> > > > 
> > > > if you support writeable buffers, of course you have to write things back
> > > > to the file at the end and that needs the fd
> > > > otherwise what is AV_FILE_PROT_WRITE doing?
> > > 
> > > Uhm, well actually I was using MAP_PRIVATE rather than MAP_SHARED.
> > 
> > uhm, then why do you need access rights at all ?!
> > r+w and no exec always should be fine
> 
> I suppose having write permission may be related to performance issues
> (but I don't think it is a good idea to keep it and complicate the
> interface).
> 
> 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.

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

Those who are too smart to engage in politics are punished by being
governed by those who are dumber. -- Plato 
-------------- 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/20101214/190c06eb/attachment.pgp>