Skip to content

android: Move AudioTrackBridge array allocation to Java to avoid direct env->New* calls#10780

Open
kjyoun wants to merge 1 commit into
youtube:mainfrom
kjyoun:avoid-env-new
Open

android: Move AudioTrackBridge array allocation to Java to avoid direct env->New* calls#10780
kjyoun wants to merge 1 commit into
youtube:mainfrom
kjyoun:avoid-env-new

Conversation

@kjyoun
Copy link
Copy Markdown
Contributor

@kjyoun kjyoun commented Jun 5, 2026

Move the allocation of audio data buffers from C++ JNI calls to the
Java AudioTrackBridge constructor.

This change follows JNI best practices by avoiding direct calls to
memory allocation methods in the JNI environment, which reduces the
risk of memory leaks from un-wrapped local references. The Java side
now manages the buffer lifecycle, and the C++ layer retrieves the
reference during initialization.

Issue: 520007952

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 5, 2026

🤖 Gemini Suggested Commit Message


android: Move AudioTrack data allocation to Java

Move the allocation of audio data buffers from C++ JNI calls to the
Java AudioTrackBridge constructor.

This change follows JNI best practices by avoiding direct calls to
memory allocation methods in the JNI environment, which reduces the
risk of memory leaks from un-wrapped local references. The Java side
now manages the buffer lifecycle, and the C++ layer retrieves the
reference during initialization.

Bug: 520007952

💡 Pro Tips for a Better Commit Message:

  1. Influence the Result: Want to change the output? You can write custom prompts or instructions directly in the Pull Request description. The model uses that text to generate the message.
  2. Re-run the Generator: Post a comment with: /generate-commit-message

@kjyoun
Copy link
Copy Markdown
Contributor Author

kjyoun commented Jun 5, 2026

/gemini review

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request refactors audio buffer allocation by moving the pre-allocation of the audio data array from C++ to Java within AudioTrackBridge. The pre-allocated array is then retrieved in C++ via JNI. Feedback on this change highlights a potential issue where throwing a RuntimeException from a JNI-invoked constructor for an unsupported sample type can lead to undefined behavior or crashes. It is recommended to log an error and let the C++ side handle the resulting null reference gracefully.

Comment thread cobalt/android/apk/app/src/main/java/dev/cobalt/media/AudioTrackBridge.java Outdated
…ew* calls

To follow best practices and reduce the risk of memory leaks from un-wrapped local references, this change moves the allocation of the audio data array from C++ to Java.

- AudioTrackBridge in Java now allocates the mAudioData array in its constructor.
- The C++ side calculates the required max_samples_per_write and passes it to Java during creation.
- C++ retrieves the allocated array reference using getAudioData() getter.
- Reordered parameters in CreateAudioTrackBridge to group max_samples_per_write with format-related parameters.

BUG=b/520007952
TAG=agy
CONV=5d7aa66e-ca8b-4eab-bf9f-38fb21320b8c
@kjyoun kjyoun marked this pull request as ready for review June 5, 2026 17:01
@kjyoun kjyoun requested a review from a team as a code owner June 5, 2026 17:01
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the audio data buffer allocation in AudioTrackBridge. The allocation of the JNI audio data array is moved from the C++ side to the Java side during construction, using a new maxSamplesPerWrite parameter. The C++ side then retrieves this pre-allocated array via a new JNI method getPreAllocatedAudioData(). I have no feedback to provide as there are no review comments.

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

Successfully merging this pull request may close these issues.

1 participant