search

sw55555 / mp4v2

Automatically exported from code.google.com/p/mp4v2
Other
1 stars 0 forks source link

[patch] Workiaround for issue #138 (custom FileProvider not working) #147

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago 
Patch is attached.
Basically this patch does the following:

1) Modified libplatform::io::File not to call FileSystem::getFileSize() when a 
custom provider is passed to the constructor. Instead, member variable _size is 
set to SIZE_UNKNOWN (maximum value of 64bit signed integer).
2) With the above modification, libmp4v2 now tries to read next atom after 
hitting EOF and it fails. This happens inside of ReadChildAtoms() loop for the 
root atom. Therefore, I modified ReadChildAtoms() not to throw exception in 
this case, and to gracefully break the loop (and modify m_end variable to the 
correct value) instead.

I didn't tested much, but as far as I can see it works, and is not platform 
dependent.

Original issue reported on code.google.com by honeycom...@gmail.com on 5 Nov 2012 at 5:07

Attachments:

GoogleCodeExporter commented 9 years ago 
I was leaning more towards a solution that actually updated FileProvider to 
include  MP4FileProvider.fileSize or something along those lines.  Likely a 
breaking change, but to be honest a custom file provider interface that doesn't 
allow such a thing is broken to begin with.

thoughts?

Original comment by kid...@gmail.com on 18 Nov 2012 at 6:04

GoogleCodeExporter commented 9 years ago 
Personally I can accept breaking changes on API and ABI, and I agree with you 
that fixing MP4FileProvider interface is more proper solution.

If you are making breaking changes..
1) Please re-consider the interface of functions like MP4ReadProvider(), 
MP4File::Read() and MP4File::Open(). They only accept MP4FileProvider 
structure, and doesn't accept magic cookie (handle) argument passed to user 
defined callbacks. Therefore they are quite useless for many use cases.
*If* name argument was just passed to user defined open() function without any 
modification, there was at least a chance of hackish workaround such as 
encoding user   defined structure or pointer *after* null terminator of name 
string, but since std::string is used, there's a no chance.

2) Of course fileSize() will work. Alternatively, you could extend seek() to 
accept seek type argument like fseek(), and do either of the following:
2-1) Change seek() to return offset from beginning, like lseek()
2-2) Add tell()

Original comment by honeycom...@gmail.com on 21 Nov 2012 at 1:21

GoogleCodeExporter commented 9 years ago 
To make it clear... I mean, open() in MP4FileProvider is useless, and more 
generic and most usual solution is adding handle argument to functions like 
MP4File::Open() and  just passing given handle to user defined read(), seek() 
or something.

Original comment by honeycom...@gmail.com on 21 Nov 2012 at 1:38

GoogleCodeExporter commented 9 years ago 
Sorry for going off into a black hole, really busy through the holidays.  I 
agree with a lot of your comments; let me look into stuff more closely when I 
get back.  Of course, if you have something worked up that I can look at 
already, I'd glady accept it.  I'm amenable to some amount of breaking change 
for this, since I think the file provider interface could really use some love 
(I do not think many people use it, likely because of a lot of the issues 
outlined in this defect).

Original comment by kid...@gmail.com on 28 Dec 2012 at 4:50