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

Add 16 kb page support to decoder_flac to support Android 15 #1685

Closed
2 tasks
maxoertel opened this issue Sep 2, 2024 · 14 comments
Closed
2 tasks

Add 16 kb page support to decoder_flac to support Android 15 #1685

maxoertel opened this issue Sep 2, 2024 · 14 comments
Assignees

Comments

@maxoertel
Copy link

maxoertel commented Sep 2, 2024

Version

Media3 main branch

More version details

Android 15 adds support for devices with 16 kb page size memory. Any native parts of the app need to be rebuild, otherwise apps will crash at these new devices. Currently, the Flac extension of Exoplayer does not support this. Hence, validating the APK shows UNALIGNED:

Screenshot 2024-09-02 at 11 37 50

TODO:

  • Follow steps here to add 16 kb page size support

Devices that reproduce the issue

Any

Devices that do not reproduce the issue

Reproducible in the demo app?

Yes

Reproduction steps

  1. Build and run the debug app.
  2. Locate the debug APK and unzip it
  3. Use the alignment.sh script provided in the official migration guide linked above and run this command:
    sh alignment.sh my_apk_out | grep "arm64-v8a"

(where my_apk_out is the unzipped apk)

Expected result

libflacJNI.so (the Flac lib) should say ALIGNED

Actual result

It says UNALIGNED

Media

Bug Report

  • You will email the zip file produced by adb bugreport to [email protected] after filing this issue.
@rohitjoins
Copy link
Contributor

Hi @maxoertel,

Thank you for raising this issue. We will add support for 16KB page to all the extension modules and keep you updated.

copybara-service bot pushed a commit that referenced this issue Oct 10, 2024
We need to rebuild any native components of the app to prevent crashes on devices with 16 KB page support.

Tested on a device that supports 16 KB pages and runs Android 15, as well as on older Android devices.

Issue: #1685
PiperOrigin-RevId: 684488244
@rohitjoins
Copy link
Contributor

Hi @maxoertel,

Support for 16 KB page size has been added. Thank you once again for raising this issue.

@maxoertel
Copy link
Author

Thx @rohitjoins for adding 16 KB page size support 💪🏼

@Tolriq
Copy link
Contributor

Tolriq commented Oct 11, 2024

@rohitjoins AFAIK 16k pages requires NDK 27 and all the docs still references NDK 21 as tested solution. Might worth a doc update?

@maxoertel
Copy link
Author

@Tolriq I'm very confident that NDK 27 is not required for 16kb pare support. We updated our own libs already and still use NDK 21 and it works fine.

Google even has a dedicated section for below NDK 27: https://developer.android.com/guide/practices/page-sizes#compile-r26-lower

@Tolriq
Copy link
Contributor

Tolriq commented Oct 11, 2024

Yes but from that part:

In order to test on those lower versions of the NDK on 16 KB devices, canary releases of LTS NDK versions [r21](https://ci.android.com/builds/branches/aosp-ndk-release-r21/grid) (for Windows and Linux only), [r23](https://ci.android.com/builds/branches/aosp-ndk-release-r23/grid), and [r25](https://ci.android.com/builds/branches/aosp-ndk-r25-release/grid) are available on Android CI with 16 KB aligned libc++_shared.so libraries. Regardless, it is recommended to move to Android NDK r27 or higher to update to the 16 KB ELF aligned version of the C++ standard library. Otherwise, your app will fail to install on 16 KB devices today.

Says it requires canary versions of those NDK

@rohitjoins
Copy link
Contributor

Hi @Tolriq,

We recently streamlined the build process for most decoder extensions by switching from ndk-build to CMake. Users no longer need to manually check out the NDK or rely on it directly. Once we upgrade to AGP versions that support NDK version 27, we might need to update the flags for 16 KB page sizes.

Eg: Simplified Flac extension build process

OxygenCobalt pushed a commit to OxygenCobalt/media that referenced this issue Oct 18, 2024
We need to rebuild any native components of the app to prevent crashes on devices with 16 KB page support.

Tested on a device that supports 16 KB pages and runs Android 15, as well as on older Android devices.

Issue: androidx#1685
PiperOrigin-RevId: 684488244
@maxoertel
Copy link
Author

@rohitjoins We tested the 1.5.0-beta01 release which mentions it includes the 16 kb page size support:

Decoder Extensions (FFmpeg, VP9, AV1, etc.):
Add 16 KB page support for decoder extensions on Android 15 ([https://github.com/androidx/media/issues/1685](https://github.com/a...

When running the alignment.sh script I'm still getting the UNALIGNED output. Could you please check on your side, if you can reproduce it (specifically for the Flac extension)?

Screenshot 2024-11-08 at 09 53 49

@rohitjoins
Copy link
Contributor

Hi @maxoertel,

Did you test it on a device with 16KB page size enabled?

I have tested this on both an emulator and a device with 16KB page enabled and the script does show aligned to me.

Screenshot 2024-11-08 at 11 42 37

@maxoertel
Copy link
Author

Hey @rohitjoins,

I didn't test it on a 16KB emulator yet, since the script showed UNALIGNED so I stopped there. Here are the exact steps I did and also tried again today:

  1. Bump the following dependencies to 1.5.0-beta01
androidx.media3:media3-exoplayer:1.5.0-beta01
androidx.media3:media3-exoplayer-hls:1.5.0-beta01
androidx.media3:media3-datasource-okhttp:1.5.0-beta01
  1. Target Android 15 / SDK 35 (since the new version required that).
  2. Build and run our app.
  3. Take the generated debug APK file, unzip it
  4. Use the unzipped folder of the APK as input for the alignment.sh script

The same steps worked for other SDKs, they showed after updating their versions in the last step ALIGNED. Only the libflachJNI.so shows still UNALIGNED for us:

Screenshot 2024-11-11 at 09 08 54

Am I doing something wrong?

@rohitjoins
Copy link
Contributor

Hey @maxoertel,

Thanks for the details! Testing specifically on a 16KB emulator isn’t necessary. What I meant was that testing should be done on either a physical or virtual device running Android 15 with 16KB page alignment enabled.

Could you check if 16KB page alignment was enabled on the device? This setting is essential for verifying alignment.

We use the same alignment parameters in our CMakeLists file for flacJNI as we do for other extensions:

# Enable 16 KB ELF alignment.
target_link_options(flacJNI
                    PRIVATE "-Wl,-z,max-page-size=16384")

@rohitjoins rohitjoins reopened this Nov 11, 2024
@maxoertel
Copy link
Author

maxoertel commented Nov 12, 2024

@rohitjoins I tested using the alignment.sh script from Google (link) and gave as input for the script the generated APK.

I did however use an APK that was build for a non-16KB device - I will retry in the next days with a 16KB one. Our other SDKs we use and our own C-libraries did show ALIGNED already for the usual debug APK after doing the 16KB page support steps. Is there a reason it would be different for Exoplayer?

Will test and let you know! Thx for your help

@maxoertel
Copy link
Author

Hey @rohitjoins,
We tested it on a 16KB emulator and it seems to work fine. Verified with the add shell getconf PAGE_SIZE command that the emulator is indeed 16KB (output: 16384). Running the app with the new Exoplayer version leads to no issues and also the Flac lib works fine.

Still I'm getting UNALIGNED when running the alignment script:
Screenshot 2024-11-15 at 11 35 10

Compared to ALIGNED for our own libs:
Screenshot 2024-11-15 at 11 35 16

TL;DR:
testing worked fine, no issues, just the script provided by Google doesn't print ALIGNED. Any idea why?

@rohitjoins
Copy link
Contributor

Nice to hear that.

just the script provided by Google doesn't print ALIGNED. Any idea why?

Not really, it shows aligned to me when I use the script as attached in #1685 (comment).

Closing the issue as the extension works!

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

No branches or pull requests

3 participants