stephengold
commented
1 year ago Perhaps not relevant, but I wonder why Android doesn't use jme3-jogg, as desktops do.
Also, assuming this is a regression, I wonder which release it first occurred in.
Closed Ali-RS closed 1 year ago
stephengold
commented
1 year ago Perhaps not relevant, but I wonder why Android doesn't use jme3-jogg, as desktops do.
Also, assuming this is a regression, I wonder which release it first occurred in.
Ali-RS
commented
1 year ago but I wonder why Android doesn't use jme3-jogg, as desktops do.
Hmm, I have no idea, is jme3-jogg a pure java implementation or does it contain natives?
Also, assuming this is a regression, I wonder which release it first occurred in.
No regression on our side afaik, but it is more like a hidden bug that showed up after the below change on the android side.
Also the reason why only higher android api triggers this error is because android changed fdsan warning level to error Quoting official docs- “Android 11. fdsan now aborts upon detecting an error; the previous behavior was to log a warning and continue.” (Source: Behavior changes: all apps | Android Developers 2)
pavly-gerges
commented
1 year ago Hmm, good point, is jme3-jogg a pure java implementation or does it contain native binaries? Maybe @Scrappers-glitch has an idea.
When hardware is involved, nothing is pure java, correct me if i am wrong, afaik the jogg library relies on the javax.media from Java Media Framework API, i don't think it provides a build for android....
No regression on our side afaik, but it is more like a change on the android side.
In fact, this is an actual bug, the double resource closure is a bug, and fortunately the new android fdsan (file descriptor sanitizer) has discovered this bug and we should provide the fix for this...
Notice, on the GNU C side, the double closure doesn't crash the program with a not owned state as android do, instead, it just throws [-1] as a return for the int close(int) function, however if you tried to use any previously allocated copies from a previously closed files, the system will throw a memory Segmentation fault with a program crash; because the close function from GNU libc doesnot only close the file stream, but it also deallocates the associated file descriptor and any unread data or queued read/write operations will be discarded....
Consider this case:
#include <stdio.h>
#include <unistd.h>
int main() {
int res0 = fclose(stdin);
int res1 = fclose(stdin);
/* returns [0] for success */
printf("Return of the first call: %d\n", res0);
/* returns [-1] for failure */
printf("Return of the second call: %d\n", res1);
return 0;
}And in this case (which is similar to the jme's, but protected by the android fdsan):
#include <stdio.h>
#include <unistd.h>
static inline FILE* copy() {
return dup(stdin);
}
int main() {
FILE* file = copy();
int res0 = fclose(stdin);
/* returns [0] for success */
printf("Return of the first call: %d\n", res0);
int res1 = fclose(file);
/* returns a Segmentation fault associated with a crash */
printf("Return of the second call: %d\n", res1);
return 0;
}The double closure has also other problems like the premature closure of resources, interruption of the buffer read/write operations and etcetera, though those errors will never manifest in this case, because we aren't exposing this API to the user, however, logical code errors should be fixed properly...
stephengold
commented
1 year ago the jogg library relies on the javax.media from Java Media Framework API
Thanks for the API link.
I unpacked the javax.media "jmf" JAR. It consists entirely of Java classes and GIF images. There's no native code. So if it's unavailable on Android, there must be a different explanation.
In fact, this is an actual bug
True, but not all bugs are regressions.
pavly-gerges
commented
1 year ago I unpacked the javax.media "jmf" JAR. It consists entirely of Java classes and GIF images. There's no native code. So if it's unavailable on Android, there must be a different explanation.
I found the native dependencies here, the performance packs should provide the native sound APIs: https://www.oracle.com/java/technologies/javase/jmf-setup-211.html
EDIT: Idk, if the performance pack is an essential or is just an addition.
EDIT2: I downloaded the jar file now too, it seems they rewrote the codecs completely in java, well we need to discuss this later and test it on android...
EDIT3: Btw, the jmf was originally built as a plugin to the java SE, they use classes from java.awt, which is not supported by android, for example the java.awt.Image....
stephengold
commented
1 year ago Idk, if the performance pack is an essential or is just an addition.
JMonkeyEngine doesn't use any performance packs, just the "jmf" library itself. So I'd say not essential.
they use classes from java.awt, which is not supported by android
Perhaps that's the reason Android doesn't use jme3-jogg. Thanks!
pavly-gerges
commented
1 year ago In fact, this is an actual bug
True, but not all bugs are regressions.
Btw, if you care, this was added in those 2 commits (since 2014), probably was jme-3.1: https://github.com/jMonkeyEngine/jmonkeyengine/commit/891ffa175d91d0326705d010e0622dfb3a721e21 and https://github.com/jMonkeyEngine/jmonkeyengine/commit/ba91da8db4a4cb964d0229c6a5f951b3f3afc525.
stephengold
commented
1 year ago 2014 ... wow!
Ali-RS
commented
1 year ago Perhaps not relevant, but I wonder why Android doesn't use jme3-jogg, as desktops do.
I just tested jme3-jogg and it works fine on Android.
pavly-gerges
commented
1 year ago Are you sure it was j-ogg and the run is not delegated to android-native ?
To be fully assured that this is actually j-ogg, you need to remove jme3-android-native from your dependencies and test a plain jme3-jogg with no game application.
Ali-RS
commented
1 year ago Are you sure it was j-ogg and the run is not delegated to android-native ?
Yes
I replaced this
with
LOADER com.jme3.audio.plugins.OGGLoader : ogg Also noticed when I removed the dependency to jme3-jogg it throws a class not found exception for com.jme3.audio.plugins.OGGLoader at runtime.
pavly-gerges
commented
1 year ago Well, this may be explained by the original source code of jmf that decode vorbis isn't calling a java awt package classes....and so the NoClassDefFoundException is not thrown on the android dalvik...thanks for testing that.
pavly-gerges
commented
1 year ago It is your choice then, to accept my PR to fix this issue, or create another PR to drop the whole native decode library and replace it with the jogg...i think we should do both 😁, replacing with jogg will not fix the current issue, it just a workaround for a better maintenance.
Ali-RS
commented
1 year ago I do not see how awt package may be useful in loading audio but for video loading yes it could be.
Ali-RS
commented
1 year ago I will merge your PR, I think, we should keep both.
Ali-RS
commented
1 year ago By the way, I excluded the jmf library coming with j-ogg-all and I can still play the ogg sound. Looks like jmf is not used for decoding ogg files so we may safely exclude it in jme3-jogg. (the jmf jar size is 1.8 MB!)
Edit: see #1958
Apparently, JME Vorbis (Ogg) audio plugin is broken on the Android API 31+.
Here is the crash log:
Forum thread: https://hub.jmonkeyengine.org/t/jmesurfaceview-for-android-api-31-not-working-properly/46465