-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Improve Media Player #13467
Improve Media Player #13467
Conversation
e8ffd33
to
27c0eee
Compare
879769e
to
bc4ddf4
Compare
63870e7
to
d97a864
Compare
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! (If you believe you should not receive this message, you can add yourself to the blocklist.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your contribution! I've created a Git patch to eliminate the use of the "!!" operator.
Could you apply this patch?
Additionally, the use of runBlocking
blocks the current thread until it completes. BackgroundPlayerService
runs on the main thread. I tested, general flow looks okay. It might be acceptable in this case, but for initNextcloudExoPlayer()
, would it be possible to notify the service after this function finishes?
Sure, @alperozturk96
Ah, I was aware of this forgot to add the comment. I know it would block the main thread it's just that the Also, we're not the one starting the service, it is started when session token is created. So no way we can tell it to start after the client init on background. since controlling when service starts not in our control. Can we have some other option to inject Exoplayer without having to create client instance? tell me if you have any ideas I will see. thanks 👍 |
d97a864
to
6412e42
Compare
@parneet-guraya Could you please address the Codacy Static Code Analysis issues and apply a git patch to eliminate the use of the |
Signed-off-by: parneet-guraya <[email protected]>
Signed-off-by: parneet-guraya <[email protected]>
Signed-off-by: parneet-guraya <[email protected]>
Signed-off-by: parneet-guraya <[email protected]>
Signed-off-by: parneet-guraya <[email protected]>
Signed-off-by: parneet-guraya <[email protected]>
Signed-off-by: parneet-guraya <[email protected]>
Signed-off-by: parneet-guraya <[email protected]>
Signed-off-by: parneet-guraya <[email protected]>
Signed-off-by: parneet-guraya <[email protected]>
Signed-off-by: parneet-guraya <[email protected]>
Signed-off-by: parneet-guraya <[email protected]>
Signed-off-by: parneet-guraya <[email protected]>
Signed-off-by: parneet-guraya <[email protected]>
Signed-off-by: parneet-guraya <[email protected]>
Signed-off-by: parneet-guraya <[email protected]>
Signed-off-by: parneet-guraya <[email protected]>
Signed-off-by: parneet-guraya <[email protected]>
6412e42
to
9f176b4
Compare
@alperozturk96 hi, applied the patch, also what would you say about use of |
APK file: https://www.kaminsky.me/nc-dev/android-artifacts/13467.apk |
I agree, no need to block this improvement, thank you 👍 |
Fixes:
#925
#5037
#11810
#7462 [
PlayerService
that used legacy media library no longer used so it makes it obsolete I guess?]Changes Introduced:
Record_2024-09-04-04-53-22.mp4
Notification Styles
Oneplus Android 14
Stock Android 12
Stock Android 13 & 14