Skip to content

Conversation

@VelikovPetar
Copy link
Contributor

@VelikovPetar VelikovPetar commented Oct 23, 2025

🎯 Goal

The current implementation of the AudioPlayer is based on android.media.MediaPlayer. While this was working well in most cases, we had occurrences where a voice recoding could not be played (to be more specific: mime-type = "audio/mp4;codecs=opus"). By migrating to ExoPlayer we are able to play such files (+ potentially to support a wider range of codecs)

🛠 Implementation details

  • Adds exoplayer dependency to stream-chat-android-client
  • Replaces the usages of the MediaPlayer with ExoPlayer (inside the NativeMediaPlayerImpl wrapper)

🎨 UI Changes

NA

🧪 Testing

Test ALL features related to voice recording:

  1. Recording a new voice message (XML and Compose)
  2. Listen (with skipping, pausing and resuming) the recording before sending it to the channel
  3. Listen to already sent voice messages (with skipping, pausing and resuming) - both own and others
  4. Switch between playing different voice messages
  5. Send voice messages from iOS/React, and verify that they are playable on Android too

@github-actions
Copy link
Contributor

github-actions bot commented Oct 23, 2025

SDK Size Comparison 📏

SDK Before After Difference Status
stream-chat-android-client 3.23 MB 5.18 MB 1.96 MB 🔴
stream-chat-android-offline 3.45 MB 5.45 MB 2.00 MB 🔴
stream-chat-android-ui-components 10.54 MB 10.54 MB 0.00 MB 🟢
stream-chat-android-compose 12.76 MB 12.77 MB 0.00 MB 🟢

@VelikovPetar VelikovPetar requested a review from Copilot October 23, 2025 15:57
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR migrates the audio playback implementation from Android's MediaPlayer to ExoPlayer to support a wider range of audio codecs, particularly addressing issues with playing voice recordings that have mime-type "audio/mp4;codecs=opus".

Key changes:

  • Replaces MediaPlayer with ExoPlayer in the NativeMediaPlayerImpl wrapper
  • Updates StreamMediaPlayer to StreamAudioPlayer throughout the codebase
  • Deprecates legacy MediaPlayer error constants in favor of PlaybackException error codes

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
stream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/feature/messages/list/AudioPlayerController.kt Updates logger tag and reorders seekTo call after player setup
stream-chat-android-client/src/test/java/io/getstream/chat/android/client/audio/StreamMediaPlayerTest.kt Renames StreamMediaPlayer to StreamAudioPlayer in tests
stream-chat-android-client/src/test/java/io/getstream/chat/android/client/audio/NativeMediaPlayerMock.kt Updates error listener signature to match ExoPlayer's single errorCode parameter
stream-chat-android-client/src/main/java/io/getstream/chat/android/client/audio/StreamAudioPlayer.kt Removes Android version checks and MediaPlayer-specific code
stream-chat-android-client/src/main/java/io/getstream/chat/android/client/audio/NativeMediaPlayer.kt Complete reimplementation using ExoPlayer instead of MediaPlayer
stream-chat-android-client/src/main/java/io/getstream/chat/android/client/audio/AudioPlayer.kt Updates parameter documentation from "hash" to "audioHash"
stream-chat-android-client/src/main/java/io/getstream/chat/android/client/ChatClient.kt Updates audio player initialization to use ExoPlayer with proper audio attributes
stream-chat-android-client/src/test/java/io/getstream/chat/android/client/MockClientBuilder.kt Updates mock to use StreamAudioPlayer type
stream-chat-android-client/build.gradle.kts Adds ExoPlayer dependency

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@VelikovPetar VelikovPetar requested a review from Copilot October 23, 2025 16:05
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@VelikovPetar VelikovPetar requested a review from Copilot October 23, 2025 16:29
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@VelikovPetar VelikovPetar marked this pull request as ready for review October 23, 2025 16:53
@VelikovPetar VelikovPetar requested a review from a team as a code owner October 23, 2025 16:53
}
val audioPlayer: AudioPlayer = StreamAudioPlayer(
mediaPlayer = NativeMediaPlayerImpl(appContext) {
ExoPlayer.Builder(appContext)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm a bit worried about the size bump of the SDK. Some companies pay attention to that.

Can we do it in a way to check if the dependency exists at runtime and decide whether to use ExoPlayer based on that?

I'm thinking, maybe we can add ExoPlayer as compileOnly instead of implementation and then check if the class exists like:

private boolean isExoPlayerAvailable() {
    try {
        Class.forName("androidx.media3.exoplayer.ExoPlayer");
        return true;
    } catch (ClassNotFoundException e) {
        return false;
    }
}

Copy link
Contributor

Choose a reason for hiding this comment

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

and if it does not? Fallback to MediaPlayer?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it adds unnecessary complexity to already complex chat. I think we need to live with the 1.9MBs.

@VelikovPetar why has offline increased for 2MB?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The offline module includes the client as a dependency, so its size has increased transitively.
The ui modules are not affected, because they already include the exoplayer dependency (for video playback).

Another (a bit more complex) solution would be to move the AudioPlayer up to the ui modules, where we already have the exoplayer library. Currently the ChatClient only accesses the AudioPlayer after a logout for cleanup, but with some effort we can ensure proper clean-up in the components where the AudioPlayer is used. The AudioPlayer API is already internal (with the exception of some now unused constants) so there won't be any breaking changes.
But such approach of course would introduce bigger complexity in the wiring of the player, because now can just access it as chatClient.audioPlayer, if we move it upwards, we would need to handle its instantiation better.

Copy link
Contributor

Choose a reason for hiding this comment

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

The correct way to avoid the 2MB increase is I think to have the contract live in chat and default MediaPlayer implementation.
Then as a separate module include the ExoPlayer backed implementation of said contract. You can then supply an external StreamMediaPlayer implementation of whichever player you choose.

However this goes against our goal of reducing integration complexity and dependencies. I think including exo player is not an optional dependency when MediaPlayer is buggy. We've had several issues with this already. And in the end we are just going to tell people to include the dependency anyway and they will still suffer the 2MB size increase.

@sonarqubecloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
3.5% Coverage on New Code (required ≥ 80%)
B Maintainability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

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.

3 participants