GNOME Bugzilla – Bug 758980
filesrc/others: Add support for files larger than 2GB on Android and other platforms without _FILE_OFFSET_BITS=64 support
Last modified: 2015-12-28 10:37:40 UTC
Created attachment 316683 [details] Debug log file zip I am reaching out to you because I have some questions regarding your GStreamer Android SDK. I am trying to implement your GStreamer Android SDK through JNI. However, I am unable to play any large files. I have tried multiple ways and it seems that any file larger than 2gb results in an error in the playback. (Files smaller than 2gb work through plugin Playbin) I have tried all versions: 1.2, 1.3, 1.4, 1.5, and 1.6. The same error also occurs in ARM and ARM 7. Can you please let me know if your Android SDK is able to support media files larger than 2gb? If your Android SDK is able to support files larger than 2gb, please let me know what I need to do. please check the attached android logcat log.
Can you provide the information I asked for on the mailing list? http://lists.freedesktop.org/archives/gstreamer-android/2015-December/001043.html
In general GStreamer supports files up to 2^64 - 1 bytes, my guess is just that we don't use the correct API on Android for accessing larger files because Google decided to provide different API behaviour than every other platform out there. After the information I asked for on the mailing list, we will know.
See https://code.google.com/p/android/issues/detail?id=64613 btw, which confirms my guess. This means that a lot of code has to be changed to a) check for the existence of the 64 bit variants of these functions and b) to actually use them. Not only in GStreamer but also in e.g. glib.
As a plan here, I would suggest to first fix filesrc/filesink and then worry about other parts. Those two are the most important parts here where handling of large files happens usually.
Created attachment 316918 [details] [review] filesink: enable largefile support on Android
Created attachment 316919 [details] [review] filesrc: enable largefile support on Android
Comment on attachment 316918 [details] [review] filesink: enable largefile support on Android Might make sense to move this up a few lines and make it a #elif defined(__BIONIC__) case after the G_OS_WIN32 one Otherwise, ftruncate() also needs to be changed to a 64 bit variant. Android does not provide any API for that directly, so need to do a syscall yourself. See e.g. http://www.scs.stanford.edu/histar/src/pkg/uclibc/libc/sysdeps/linux/common/ftruncate64.c
Comment on attachment 316919 [details] [review] filesrc: enable largefile support on Android Same here, same problem with fstat() vs fstat64().
E.g. see http://www.scs.stanford.edu/histar/src/pkg/uclibc/libc/sysdeps/linux/common/fstat64.c
(In reply to Sebastian Dröge (slomo) from comment #8) > Comment on attachment 316919 [details] [review] [review] > filesrc: enable largefile support on Android > > Same here, same problem with fstat() vs fstat64(). Bionic (checked android-9) has fstat64 as an alias to fstat and the implementation seems to be 64bits-aware as far as I saw. ftruncate we will have to implement though. Thanks for taking a look
(In reply to Reynaldo H. Verdejo Pinochet from comment #10) > (In reply to Sebastian Dröge (slomo) from comment #8) > > Comment on attachment 316919 [details] [review] [review] [review] > > filesrc: enable largefile support on Android > > > > Same here, same problem with fstat() vs fstat64(). > > Bionic (checked android-9) has fstat64 as an alias to fstat > and the implementation seems to be 64bits-aware as far as I > saw. ftruncate we will have to implement though. Thanks > for taking a look And, thinking abut this twice, the only call to ftruncate() we have on the code does ftruncate(,0) which should work with BIONIC's ftruncate regardless or is there something I'm missing?
(In reply to Reynaldo H. Verdejo Pinochet from comment #10) > (In reply to Sebastian Dröge (slomo) from comment #8) > > Comment on attachment 316919 [details] [review] [review] [review] > > filesrc: enable largefile support on Android > > > > Same here, same problem with fstat() vs fstat64(). > > Bionic (checked android-9) has fstat64 as an alias to fstat > and the implementation seems to be 64bits-aware as far as I > saw. ftruncate we will have to implement though. Thanks > for taking a look Indeed, but let's better be explicit about that and use fstat64() on bionic in case they ever change that :) (In reply to Reynaldo H. Verdejo Pinochet from comment #11) > (In reply to Reynaldo H. Verdejo Pinochet from comment #10) > > (In reply to Sebastian Dröge (slomo) from comment #8) > > > Comment on attachment 316919 [details] [review] [review] [review] [review] > > > filesrc: enable largefile support on Android > > > > > > Same here, same problem with fstat() vs fstat64(). > > > > Bionic (checked android-9) has fstat64 as an alias to fstat > > and the implementation seems to be 64bits-aware as far as I > > saw. ftruncate we will have to implement though. Thanks > > for taking a look > > And, thinking abut this twice, the only call to ftruncate() > we have on the code does ftruncate(,0) which should work > with BIONIC's ftruncate regardless or is there something I'm > missing? Most likely you're right, unless there is something silly internally. Let's keep it like that for now I guess
Review of attachment 316918 [details] [review]: commit add4526314af627be2d2485950bf56ca02ddd8e1 Author: Reynaldo H. Verdejo Pinochet <reynaldo@osg.samsung.com> Date: Thu Dec 3 15:04:32 2015 -0800 filesink: enable large file support on Android
Review of attachment 316919 [details] [review]: Pushed with the explicit fstat64 fix commit a7f8ad9c9a1a8842b55db4aec639a5f579cab345 Author: Reynaldo H. Verdejo Pinochet <reynaldo@osg.samsung.com> Date: Mon Dec 7 20:27:45 2015 -0800 filesrc: enable large file support in Android [..]
Review of attachment 316919 [details] [review]: Pushed
I backported the two patches to 1.6, so will be part of 1.6.2. I'm not sure why the #undef for lseek, etc is needed (it does not seem to be #defined anywhere in the NDK) but it also doesn't cause compiler warnings (which it would if it really wasn't #defined but then #undef'd).
(In reply to Sebastian Dröge (slomo) from comment #16) > I backported the two patches to 1.6, so will be part of 1.6.2. > [..] Thanks for taking care of that Seb. Now I'm not sure how to get this bug to a "fixed" state. Things now work for Android but the agreed scope is broader. If we want to move forward I think we should get filesink to stop using the FILE API and move it to lower level primitives (open and friends) like its src counterpart (as you suggested on IRC). Other than that, what about isolating this in our own generic API gst_open/seek/truncate/etc so we contain this multi-platform ifdef nightmare on a single header?
There's other places where we use the wrong API and could have files >2GB, e.g. in gstsparsefile/queue2/downloadbuffer. Then there's the related bug for GLib.
Pushed in this same context: 2a17bad7e834b04b downloadbuffer: drop unneeded macros for G_OS_WIN32 806b48f6ba099698 fdsink: enable large file support in Android e71344010add73a9 fdsrc: enable large file support in Android ea8bbd58dfe4a204 queue2: enable large file support on Android 63727152e041acc1 downloadbuffer: enable large file support on Android 6e563cf353ebc6bd sparsefile: enable large file support on Android Related: These likely were originated by ppl c&p-ing the G_OS_WIN32 stub: b3a704de6aff3784 fdsrc: drop unneeded macros for G_OS_WIN32 2a17bad7e834b04b downloadbuffer: drop unneeded macros for G_OS_WIN32 I'm still working on others but this should be all for the "gstreamer" module itself. Will keep updating here so we can keep a reference.
And merged all the functional changes to 1.6 too (not the cleanup)