[FFmpeg-devel] [PATCH] Windows support for av_file_map()
Michael Niedermayer
michaelni
Wed Jan 5 17:58:48 CET 2011
On Wed, Jan 05, 2011 at 12:16:44AM -0700, Daniel Verkamp wrote:
> On Tue, Jan 4, 2011 at 10:47 PM, Daniel Verkamp <daniel at drv.nu> wrote:
> > On Sat, Jan 1, 2011 at 11:39 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
> >> On Fri, Dec 31, 2010 at 02:35:50AM -0700, Daniel Verkamp wrote:
> >>> Hi,
> >>>
> >>> Here's a simple implementation of av_file_map() using
> >>> CreateFileMapping() and MapViewOfFile(), which are the rough Win32
> >>> equivalent of mmap().
> >>>
> >>> This currently maps a read-only view of the file; this could be
> >>> changed to R/W to match the mmap() path if desired, but any code
> >>> expecting to actually use it for writing will fail with the non-mmap()
> >>> path anyway since the file is never written back in that case.
> >>>
> >>> Thanks,
> >>> -- Daniel Verkamp
> >>
> >> lgtm if tested
> >>
> >
> > Applied with a small modification recommended by Ramiro on IRC.
> >
> > An (ugly hack of an) idea occurred to me: for the malloc() (non-mmap)
> > case, the buffer could be allocated with some extra space at the
> > beginning to store a fd or other information to be used to flush the
> > file at close time; this follows the same API with working write
> > access. ?A first try at implementing this is attached.
> >
> > On the other hand, this API is not very useful on 32-bit machines,
> > since it only maps full files, and large files will obviously not fit
> > into a 32-bit address space no matter how hard they are squeezed. ?It
> > might not be a bad idea to have some kind of windowing/view support
> > like mmap() et. al. if this is really intended to be used for anything
> > other than the trivial small file case (and if that's all it's for, it
> > seems like overkill).
> >
>
> Slightly changed version that doesn't depend on undefined integer
> overflow semantics attached.
> Index: file.c
> ===================================================================
> --- file.c (revision 26221)
> +++ file.c (working copy)
> @@ -33,16 +33,21 @@
> void *log_ctx;
> } FileLogContext;
>
> +/// Stored immediately preceding file data in memory in non-memory-mapped implementation
> +typedef struct {
> + int fd;
> +} FileMapContext;
> +
fd could be put in FileLogContext and FileLogContext be put in the array
> static const AVClass file_log_ctx_class = {
> "FILE", av_default_item_name, NULL, LIBAVUTIL_VERSION_INT,
> - offsetof(FileLogContext, log_offset), offsetof(FileLogContext, log_ctx)
> +///// offsetof(FileLogContext, log_offset), offsetof(FileLogContext, log_ctx)
> };
hmm?
>
> int av_file_map(const char *filename, uint8_t **bufptr, size_t *size,
> int log_offset, void *log_ctx)
> {
> FileLogContext file_log_ctx = { &file_log_ctx_class, log_offset, log_ctx };
> - int err, fd = open(filename, O_RDONLY);
> + int err, fd = open(filename, O_RDWR);
> struct stat st;
> av_unused void *ptr;
> off_t off_size;
> @@ -84,6 +89,7 @@
> return err;
> }
> *bufptr = ptr;
> + close(fd);
> #elif HAVE_MAPVIEWOFFILE
> {
> HANDLE mh, fh = (HANDLE)_get_osfhandle(fd);
> @@ -104,18 +110,31 @@
> }
>
> *bufptr = ptr;
> + close(fd);
> }
> #else
> - *bufptr = av_malloc(*size);
> - if (!*bufptr) {
> + if (*size > max_size - sizeof(FileMapContext))
> + {
> + av_log(&file_log_ctx, AV_LOG_ERROR,
> + "File size for file '%s' is too big\n", filename);
> + close(fd);
> + return AVERROR(EINVAL);
> + }
something like
p=NULL;
if(small enough)
p=malloc()
if(!p)
...
seems simpler
> +
> + {
> + FileMapContext* buf = av_malloc(*size + sizeof(FileMapContext));
this block in the middle is a bit ugly
> + if (!buf) {
> av_log(&file_log_ctx, AV_LOG_ERROR, "Memory allocation error occurred\n");
> close(fd);
> return AVERROR(ENOMEM);
> }
> +
> + buf->fd = fd;
> + *bufptr = (uint8_t*)(buf + 1);
> read(fd, *bufptr, *size);
> + }
> #endif
>
> - close(fd);
> return 0;
> }
>
> @@ -126,7 +145,13 @@
> #elif HAVE_MAPVIEWOFFILE
> UnmapViewOfFile(bufptr);
> #else
> - av_free(bufptr);
> + {
> + FileMapContext* buf = (FileMapContext*)bufptr - 1;
> + if (!lseek(buf->fd, 0, SEEK_SET))
> + write(buf->fd, bufptr, size);
some error should be printed if lseek/write fails
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
The bravest are surely those who have the clearest vision
of what is before them, glory and danger alike, and yet
notwithstanding go out to meet it. -- Thucydides
-------------- 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/20110105/ef1e4a42/attachment.pgp>
More information about the ffmpeg-devel
mailing list