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

feat(android): ExoPlayer for Android #1691

Open
wants to merge 59 commits into
base: main
Choose a base branch
from
Open

feat(android): ExoPlayer for Android #1691

wants to merge 59 commits into from

Conversation

Gustl22
Copy link
Collaborator

@Gustl22 Gustl22 commented Nov 5, 2023

Description

Closes #1526

Checklist

  • The title of my PR starts with a Conventional Commit prefix (fix:, feat:, refactor:,
    docs:, chore:, test:, ci: etc).
  • I have read the Contributor Guide and followed the process outlined for submitting PRs.
  • I have updated/added tests for ALL new/updated/fixed functionality.
  • I have updated/added relevant documentation and added dartdoc comments with ///, where necessary.
  • I have updated/added relevant examples in example.

Breaking Change

  • Yes, this is a breaking change.
  • No, this is not a breaking change.

Related Issues

Inspiration from androidx/media#310
Blocked by flutter/flutter#137040

@crimesp
Copy link

crimesp commented Feb 1, 2025

hi there i was able to get this branch running locally against an old alcatel pixi4 that was having issues playing mp3s with the mediaplayer library. Is this likely to get merged soon or does it need more work / is it safe for me to use in production please

thank you!

@Gustl22
Copy link
Collaborator Author

Gustl22 commented Feb 2, 2025

I have to work on a solution to support channel volumes to not break the library experience. I propose to fork the branch and use that in the meantime. This branch and also this functionality is not guaranteed to work and is not ready for the stable environment yet.

# Conflicts:
#	packages/audioplayers/example/.metadata
#	packages/audioplayers/example/android/app/build.gradle
#	packages/audioplayers/example/android/build.gradle
#	packages/audioplayers/example/android/gradle.properties
#	packages/audioplayers/example/android/gradle/wrapper/gradle-wrapper.properties
#	packages/audioplayers/example/android/settings.gradle
#	packages/audioplayers/example/pubspec.yaml
#	packages/audioplayers/pubspec.yaml
@Gustl22 Gustl22 marked this pull request as ready for review March 1, 2025 17:25
@Gustl22
Copy link
Collaborator Author

Gustl22 commented Mar 2, 2025

@spydon I finally had some time to wrap up the ExoPlayer implementation. But like with every new things we have some quirks coming along:

  • I have not yet explored the possibility for Low latency mode in ExoPlayer, and also am not quite sure if that exists. But I also have never done any tests how much time this actually is saving in our current implementation.
  • As long as we are unclear with Low latency mode in exoplayer I propose to keep the classic implementation as endorsed plugin and the audioplayers_android_exo implementation as non-endorsed. So one could easily use the exo package as described in the feature_parity_table docs, if wanted.
  • As long as we haven't published audioplayers_android_exo, we cannot dynamically add it to the pubspec via the dart command (unless I have some own written script). So my plan is to currently test the example with overriding the endorsed plugin with audioplayers_android_exo, and as soon as it's released, test both implementation, by also overriding in a separate test, when the package is available.

@Gustl22 Gustl22 requested review from luanpotter and spydon March 2, 2025 12:35
@spydon
Copy link
Member

spydon commented Mar 2, 2025

@spydon I finally had some time to wrap up the ExoPlayer implementation. But like with every new things we have some quirks coming along:

  • I have not yet explored the possibility for Low latency mode in ExoPlayer, and also am not quite sure if that exists. But I also have never done any tests how much time this actually is saving in our current implementation.
  • As long as we are unclear with Low latency mode in exoplayer I propose to keep the classic implementation as endorsed plugin and the audioplayers_android_exo implementation as non-endorsed. So one could easily use the exo package as described in the feature_parity_table docs, if wanted.
  • As long as we haven't published audioplayers_android_exo, we cannot dynamically add it to the pubspec via the dart command (unless I have some own written script). So my plan is to currently test the example with overriding the endorsed plugin with audioplayers_android_exo, and as soon as it's released, test both implementation, by also overriding in a separate test, when the package is available.

Exciting! Sounds like a great plan

Copy link
Member

@spydon spydon left a comment

Choose a reason for hiding this comment

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

Good job!

import androidx.annotation.RequiresApi
import java.util.*

data class AudioContextAndroid(
Copy link
Member

Choose a reason for hiding this comment

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

Is this duplicated from the promoted package? Is this the only one that is duplicated, otherwise maybe the promoted package could be imported and the files reused?

If it's only this one I think duplication is fine though.

Copy link
Collaborator Author

@Gustl22 Gustl22 Mar 2, 2025

Choose a reason for hiding this comment

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

Not 100% sure how this can be achieved. There are more files, which are the same, so it would be nice to have them unique.

Don't know how the files from the other package can be reused by importing them (that kind of defeats the purpose of a separate package to not be dependent on the other one). Or creating a separate kotlin package? That involes compiling and archiving and uploading it to some maven package repo(?). Pretty much effort also.

Best alternative I see: I don't know the exact outcome of this: dart-lang/pub#3298, but if symlinked files are also copied (and not referenced) during packaging, we could easily write a symlink to ensure both files are the same.

Copy link
Member

Choose a reason for hiding this comment

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

Oh nice, I didn't know the symlink one was finally merged!

But I guess we can do that later? Something similar needs to be done for macOS / iOS right?

@Gustl22 Gustl22 changed the title feat(android): ExoPlayer for Android feat(android): ExoPlayer for Android Mar 3, 2025
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.

feat: ExoPlayer for Android
3 participants