Better FileProvider API

[GoogleCodeExporter]

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.

Original issue reported on code.google.com by Andree.e...@gmail.com on 4 Apr 2014 at 12:37

[GoogleCodeExporter]

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

Original comment by Nicholas...@gmail.com on 18 Aug 2014 at 6:47

Attachments:

• MP4CreateProvider.patch

[GoogleCodeExporter]

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).

Original comment by kid...@gmail.com on 14 Oct 2014 at 3:26

[GoogleCodeExporter]

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.

Original comment by Nicholas...@gmail.com on 17 Oct 2014 at 10:47