Issue 168 in mp4v2: Better FileProvider API

[mp...@googlecode.com]

Status: New
Owner: ----
Labels: Type-Defect Priority-Medium

New issue 168 by Andree.e...@gmail.com: Better FileProvider API
http://code.google.com/p/mp4v2/issues/detail?id=168

The current FileProvider API is bad. I recently tried to implement
this library in another software, but with the current API it's not
possible to read from other sources than files (buffers, network etc).

There was a workaround before, which involved passing the filename as
the context, but this doesn't work since the mp4v2 tries to read the
file size.

There has been some talk about it here
https://groups.google.com/forum/#!searchin/mp4v2/readprovider/mp4v2/ZNbXTgb6dkI/NhhCAnIN_yAJ
but as far as I can see nothing has happened.

I wouldn't mind looking into it, but it'd be good to know if you have
any plans for it already.

--
You received this message because this project is configured to send all
issue notifications to this address.
You may adjust your notification preferences at:
https://code.google.com/hosting/settings

[mp...@googlecode.com]

Comment #1 on issue 168 by Nicholas...@gmail.com: Better FileProvider API
http://code.google.com/p/mp4v2/issues/detail?id=168

At my employer we too have to use the file provider API and have made
changes to that code. While we do have actual files to read and write we
also need to build an MP4 in memory. Specifically, we build MP4 files of
short coded video sequences that are guaranteed to begin with an I frame.

The issue with the file provider is that it does not provide an abstraction
for getting the file size. It assumes the construct is a real file and
calls a function that expects a genuine file descriptor. This may be OK in
*NIX since most things are file descriptors even if it is not a file on
disk. However, in Windows the API call to get file information fails
because the file provider is not a real file (that is, the file provider
cannot return a handle that is compatible with the Win32 API call for
getting file size).

As stated, I have made changes to accommodate my needs. I don't want to
maintain a personal vendor branch and would like to contribute my changes
back to the community. I have provided a patch file of my changes. Allow
me to describe those changes. I'll be as brief as possible.

I strived for changes that are non-breaking to existing compiled projects.
However, this was not 100% possible. However, reading the discussion from
the OP's link shows that a breaking change isn't entirely taboo.

The first change was to provide additional MP4Create functions. I followed
the style of the MP4Read functions. Thus, I have added two new functions
named MP4CreatProvider and MP4CreateProviderEx. This is a non-breaking
change. The implementation of MP4Create and MP4CreateEx have been modified
to simply call into MP4CreateProviderEx. Default arguments have been
preserved. The original MP4Create and MP4CreateEx functions retain their
behavior in that a NULL provider is used under the hood, which results in
the StandardFileProvider instance being used.

The second change was to add an additional function pointer to the
MP4FileProvider interface. This is a breaking change. I had a choice
between adding a "tell" operation or a "getSize" operation. With "tell"
you would seek to the end and then call "tell" to get the current
position. A problem of that strategy is it could potentially be expensive,
say you were dealing with a large MP4 movie from a SAN drive (maybe a
unrealistic example but you get my point). Thus, I decided on "getSize"
because that implies a cheap operation (reading the file meta data as
opposed to seeking through the whole file).

Lastly, for all intents and purposes I made a very conscience effort to
keep the style and design of the existing code. All spacing, alignments,
naming style, parameter placement, comments, etc., have been written to
faithfully match its neighboring code. I hope this helps with the adoption
of the patch. Please also note that I develop for the Windows platform so
my changes are tested on that platform. I do not have as thorough of a
test for *NIX platforms.

Any feedback is appreciated. Thank you.

Nicholas


Attachments:
MP4CreateProvider.patch 13.9 KB

[mp...@googlecode.com]

Comment #2 on issue 168 by kid...@gmail.com: Better FileProvider API
https://code.google.com/p/mp4v2/issues/detail?id=168

I think it's fine to make breaking changes to this API, because I agree the
design is more or less broken (primarily due to the file size issue you
mention).

[mp...@googlecode.com]

Comment #3 on issue 168 by Nicholas...@gmail.com: Better FileProvider API
https://code.google.com/p/mp4v2/issues/detail?id=168

I have committed my patch. My patch does not represent a major redesign of
the feature. It only addresses a single point of failure, namely the need
of the provider API to allow querying the size of the file.

As a Windows developer the code changes have been through much use on the
Windows platform. For *NIX platforms I fired up an openSUSE Linux VM. I
have built the library with my patch applied. There was a compiler error
in my patch and has been fixed. I created a test harness app that reads a
raw H.264 file (from the encoder cards that I work with at my job) and
generates an MP4 file using the custom file provider that adapts an
in-memory buffer as a file (note: this is my use case that is the
motivation for this patch, this is the work flow that I have to deal with
at my job). The test harness dumped the memory buffer to file when it was
finished. I was able to play the file in VLC. My H.264 analysis programs
were also able to consume the file.