Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Android port doesn't support reading from within package file [patch attached] #454

Closed
SDLBugzilla opened this issue Feb 10, 2021 · 0 comments
Closed

Comments

@SDLBugzilla
Copy link
Collaborator

@SDLBugzilla SDLBugzilla commented Feb 10, 2021

This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

Reported in version: HG 2.0
Reported for operating system, platform: Android (All), ARM

Comments on the original bug report:

On 2011-07-29 07:57:17 +0000, Tim Angus wrote:

Created attachment 662
Patch that adds read only SDL_RWops support for Android

The Android SDL port currently lacks the ability to read files from within the .apk files which Android applications are distributed and installed with. This means that to access any data, an SDL based android application must read from a shared SD card. This of course means that the data must be manually copied to a directory where the application is expecting it; it won't be available immediately post-install.

It is clearly unacceptable to expect users to manually unpack an application's data files so this must be fixed.

Attached is a patch that implements SDL_rwops for Android using various JNI gymnastics to read from the "assets" directory of an .apk file.

Note that since version 5 of the Android NDK, there has been a native API for accessing the AssetManager. Unfortunately however, this entails raising the minimum supported version of Android to 2.3, which is still relatively new. Since the existing SDL port in theory works all the way back to 1.6, it would be a shame to require such a new version simply for file access. I therefore chose to implement the support in terms of the Java interface in order to maintain backwards compatibility. This sounds pretty nasty but it should still be relatively efficient as no unnecessary data copies are happening (at least outwith the JVM). Currently there is no write support; this is a different API and it's not all that clear how it will fit in terms of the SDL_rwops interface.

Note that reading and writing files to the /sdcard directory should still be possible by using the stdio interface directly, optionally in conjunction with SDL_RWFromFP.

Final random thought; it would be nice to get Android added to the Bugzilla OS list.

On 2011-07-29 10:18:15 +0000, Ryan C. Gordon wrote:

(I added an Android platform to Bugzilla.)

The patch looks good; I'll give it another look when I wake up and get it into revision control.

Thanks!

--ryan.

On 2011-07-29 13:51:49 +0000, Ryan C. Gordon wrote:

This is now hg changeset 1281a3f1f0a6, thanks!

--ryan.

On 2011-07-29 15:12:01 +0000, Tim Angus wrote:

Cool, cheers for the prompt commit :).

On 2011-08-26 03:12:25 +0000, Tim Angus wrote:

Created attachment 682
Fix to Android FS memory leaks

My original testing of this only included relatively small files. Unfortunately, there are a number of unobvious memory leaks that become quite apparent quite quickly with larger files. The TL;DR way of describing of the problem is that the way the JVM is involved in the SDL Android port means that garbage collection only happens on things allocated in the JNI code when SDL_main returns. Therefore for all intents and purposes it's all leaking.

So essentially everything allocated in JNI land has to be manually deallocated using DeleteLocalRef (or PushLocalFrame/PopLocalFrame). This is the bulk of the attached patch. I've also added code to pass through the details of Java exceptions to SDL_SetError when they occur.

On 2011-08-26 03:12:50 +0000, Tim Angus wrote:

Reopening.

On 2011-08-26 03:15:34 +0000, Tim Angus wrote:

*** Bug 1288 has been marked as a duplicate of this bug. ***

On 2011-08-26 05:13:17 +0000, Tim Angus wrote:

Created attachment 684
Fix to Android FS memory leaks

On 2011-08-26 05:35:35 +0000, Gabriel Jacobo wrote:

Thanks for the fix, I've since noticed that the push/restore routine needs to be done in the other file (read, etc) functions as well (as there's some object reference grabbing in some of them), I don't easily see from the patch you uploaded if this is the case.

On 2011-08-26 05:47:05 +0000, Tim Angus wrote:

In the other places I've manually added appropriate DeleteLocalRef calls so this shouldn't be necessary.

On 2011-08-26 07:46:53 +0000, Ryan C. Gordon wrote:

This is in revision control now, thanks Tim!

--ryan.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant