[PCM Offload, DRAFT] PowerPlay Sample App Implementation #2197

madebymozart · 2025-04-08T14:29:13Z

This is the powerplay sample app, which allows a user, through oboe, to choose a new performance mode that allows the CPU to sleep significantly more, thus aiding in it's ability to save power.

* Add offload support. Fixes #2154.

madebymozart · 2025-04-08T14:47:20Z

samples/powerplay/README.md

@@ -0,0 +1,19 @@
+# Minimal Oboe


TODO - Update Readme

flamme

This change should be rebased so that the commit doesn't show the already merged PR.

flamme · 2025-04-09T17:28:44Z

samples/iolib/src/main/cpp/player/SimpleMultiPlayer.cpp

-            mParent->mSampleSources[index]->mixAudio((float*)audioData, mParent->mChannelCount,
-                                                     numFrames);
+        mParent->resetAll();
+        if (mParent->openStream(PerformanceMode::None) && mParent->startStream()) {


Q: Why this is opened with performance none?

flamme · 2025-04-09T17:31:10Z

samples/iolib/src/main/cpp/player/SimpleMultiPlayer.cpp

+
+        builder.setErrorCallback(mErrorCallback);
+        builder.setPresentationCallback(mPresentationCallback);
+        builder.setPerformanceMode(performanceMode);


nit, the set will return a pointer of the builder, it may looks easier as below.

builder.setChannelCount(mChannelCount) ->setDataCallback(mDataCallback) ...

flamme · 2025-04-09T20:32:55Z

samples/iolib/src/main/cpp/player/SimpleMultiPlayer.cpp

+            if (tryCount > 0) {
+                usleep(20 * 1000); // Sleep between tries to give the system time to settle.
+                wasOpenSuccessful = openStream(
+                        PerformanceMode::None); // Try to open the stream again after the first try.


Looks like the original was setting performance mode as low latency, why it is changed to performance mode none?

flamme · 2025-04-09T20:33:35Z

samples/iolib/src/main/cpp/player/SimpleMultiPlayer.cpp

-    mSampleBuffers.clear();
-    mSampleSources.clear();
+    void SimpleMultiPlayer::triggerUp(int32_t index) {
+//        if (index < mNumSampleBuffers) {


Q: why this is commented out?

flamme · 2025-04-09T20:35:05Z

samples/iolib/src/main/cpp/player/SimpleMultiPlayer.h

@@ -33,10 +33,10 @@ class SimpleMultiPlayer  {
 public:
    SimpleMultiPlayer();

-    void setupAudioStream(int32_t channelCount);
+    void setupAudioStream(int32_t channelCount, oboe::PerformanceMode performanceMode);


Just one concern that this may break existing apps using SimpleMultiPlayer. But it is up to you to decide if a default parameter is preferred or not.

If DrumThumper is using SimpleMultiPlayer then switching to None would give it high latency! That would be a problem.

flamme · 2025-04-09T20:36:25Z

samples/powerplay/src/main/cpp/PowerPlayJNI.cpp

+) {
+    __android_log_print(ANDROID_LOG_INFO, TAG, "%s", "setupAudioStreamNative()");
+
+    // TODO - Dynamically Set Performance Mode


I thought the latest PowerPlay you shared already support dynamic setting performance mode, right?

flamme · 2025-04-09T20:37:54Z

samples/powerplay/src/main/cpp/PowerPlayJNI.cpp

+JNIEXPORT void JNICALL
+Java_com_example_powerplay_engine_PowerPlayAudioPlayer_startPlayingNative(JNIEnv *env, jobject,
+                                                                          jint index,
+                                                                          jint offload) {


Looks like this is used to decide what performance mode is. It will be simpler to have the same values as performance mode defined at the kotlin layer and pass it down.

flamme · 2025-04-09T20:38:24Z

samples/powerplay/src/main/kotlin/com/example/powerplay/engine/AudioForegroundService.kt

@@ -0,0 +1,104 @@
+package com.example.powerplay.engine


I think new file should have copy right comment.

flamme · 2025-04-09T20:41:13Z

samples/powerplay/src/main/kotlin/com/example/powerplay/engine/AudioForegroundService.kt

+
+            }
+
+            AudioManager.AUDIOFOCUS_LOSS, AudioManager.AUDIOFOCUS_LOSS_TRANSIENT, AudioManager.AUDIOFOCUS_LOSS_TRANSIENT_CAN_DUCK -> {


Maybe add a todo here when focus loss, it is expected to pause music.

flamme · 2025-04-09T20:41:41Z

samples/powerplay/src/main/kotlin/com/example/powerplay/engine/PowerPlayAudioPlayer.kt

+
+class PowerPlayAudioPlayer() : DefaultLifecycleObserver {
+    /**
+     *


Any comment here? Otherwise, please remove

philburk · 2025-04-11T16:34:17Z

samples/LiveEffect/src/main/cpp/CMakeLists.txt

@@ -42,5 +42,5 @@ target_link_options(liveEffect PRIVATE "-Wl,-z,max-page-size=16384")

 # Enable optimization flags: if having problems with source level debugging,
 # disable -Ofast ( and debug ), re-enable it after done debugging.
-target_compile_options(liveEffect PRIVATE -Wall -Werror "$<$<CONFIG:RELEASE>:-Ofast>")
+target_compile_options(liveEffect PRIVATE -Wall -Werror "$<$<CONFIG:RELEASE>:-O3 -ffast-math>")


This may be a great change. But it seems unrelated to the new PowerPlay app. If you see room for improvement, it would be easier to submit those as separate PRs.

philburk · 2025-04-11T16:43:17Z

samples/powerplay/src/main/cpp/PowerPlayJNI.cpp

+    sampleBuffer->loadSampleData(&reader);
+
+    const auto source = new OneShotSampleSource(sampleBuffer, 0);
+    sDTPlayer.addSampleSource(source, sampleBuffer);


Who manages the deletion of source?

robertwu1 · 2025-04-15T20:22:56Z

samples/powerplay/src/main/cpp/PowerPlayJNI.cpp

+}
+
+/**
+ * Native (JNI) implementation of DrumPlayer.getOutputReset()


change comments

robertwu1 · 2025-04-15T21:03:40Z

samples/powerplay/src/main/kotlin/com/example/powerplay/ui/theme/Theme.kt

+    MaterialTheme(
+        colorScheme = LightColorScheme, typography = Typography, content = content
+    )
+}


add new lines to all files

robertwu1 · 2025-04-15T21:04:01Z

samples/powerplay/src/main/kotlin/com/example/powerplay/engine/AudioForegroundService.kt

+        val result = audioManager.requestAudioFocus(audioFocusRequest)
+        if (result == AudioManager.AUDIOFOCUS_REQUEST_GRANTED) {
+            // Start playback only when audio focus is granted
+            // ...


add todo here

robertwu1 · 2025-04-15T21:05:15Z

samples/iolib/src/main/cpp/player/SimpleMultiPlayer.cpp

+        builder.setErrorCallback(mErrorCallback);
+        builder.setPresentationCallback(mPresentationCallback);
+        builder.setPerformanceMode(performanceMode);
+        builder.setFramesPerDataCallback(128);


Can you verify whether DrumThumper works similarly to before?

DrumThumper may be OK if it passes an explicit LowLatency parameter.
But third-party apps using this as a library class may be impacted if the default behavior changes!

madebymozart added P1 high priority apps related to a test or example app requires device Requires a specific Android phone model or other specific hardware to test labels Apr 8, 2025

madebymozart added this to the future milestone Apr 8, 2025

madebymozart requested review from flamme and robertwu1 April 8, 2025 14:29

madebymozart self-assigned this Apr 8, 2025

madebymozart force-pushed the powerplay-sample branch from dc35492 to 090d355 Compare April 8, 2025 14:41

Add offload support. (#2165)

ca26007

* Add offload support. Fixes #2154.

madebymozart force-pushed the powerplay-sample branch from 090d355 to ca26007 Compare April 8, 2025 14:43

remove NDK specification for unaffected apps

4eee335

madebymozart commented Apr 8, 2025

View reviewed changes

samples/powerplay/README.md

@@ -0,0 +1,19 @@

# Minimal Oboe

Copy link

Collaborator Author

madebymozart Apr 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO - Update Readme

flamme reviewed Apr 9, 2025

View reviewed changes

philburk reviewed Apr 11, 2025

View reviewed changes

robertwu1 reviewed Apr 15, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[PCM Offload, DRAFT] PowerPlay Sample App Implementation #2197

[PCM Offload, DRAFT] PowerPlay Sample App Implementation #2197

madebymozart commented Apr 8, 2025

madebymozart Apr 8, 2025

flamme left a comment

flamme Apr 9, 2025

flamme Apr 9, 2025

flamme Apr 9, 2025

flamme Apr 9, 2025

flamme Apr 9, 2025

philburk Apr 11, 2025

flamme Apr 9, 2025

flamme Apr 9, 2025

flamme Apr 9, 2025

flamme Apr 9, 2025

flamme Apr 9, 2025

philburk Apr 11, 2025

philburk Apr 11, 2025

robertwu1 Apr 15, 2025

robertwu1 Apr 15, 2025

robertwu1 Apr 15, 2025

robertwu1 Apr 15, 2025

philburk Apr 16, 2025


		}

		AudioManager.AUDIOFOCUS_LOSS, AudioManager.AUDIOFOCUS_LOSS_TRANSIENT, AudioManager.AUDIOFOCUS_LOSS_TRANSIENT_CAN_DUCK -> {

[PCM Offload, DRAFT] PowerPlay Sample App Implementation #2197

Are you sure you want to change the base?

[PCM Offload, DRAFT] PowerPlay Sample App Implementation #2197

Conversation

madebymozart commented Apr 8, 2025

Choose a reason for hiding this comment

flamme left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment