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

fix(android): playback doesn't work with 0 startPositionMs #3784

Merged

Conversation

YangJonghun
Copy link
Contributor

Summary

Some device(maybe low performance device) doesn't work with zero startPositionMs and DRM
(playback doesn't work before user trigger seek)
So I use resetPosition = true when startPositionMs is 0

Motivation

Changes

Test plan

player.setMediaSource(mediaSource, startPositionMs);
} else {
player.setMediaSource(mediaSource, !haveResumePosition);
player.setMediaSource(mediaSource, false);
Copy link
Collaborator

Choose a reason for hiding this comment

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

correct me if I am wrong, but the last else case should be:
player.setMediaSource(mediaSource, true );
?

Suggested change
player.setMediaSource(mediaSource, false);
player.setMediaSource(mediaSource, true);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the part that has changed.
If resetPosition is false, it use getCurrentPosition() internally.
I felt this behavior was more appropriate, so I fixed it.

Copy link
Contributor Author

@YangJonghun YangJonghun May 16, 2024

Choose a reason for hiding this comment

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

If you think it's better to guarantee the same behavior as before, I'll fix it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I was thinking the aim of this fix is to add the case with if (startPositionMs == 0) { ?
because here it is the normal use case ?!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I'm wrong and it would be better to just fix the bug case. Thanks :)

@YangJonghun YangJonghun force-pushed the fix/zero-start-position branch from f954358 to 3c57114 Compare May 16, 2024 11:33
@freeboub freeboub requested a review from KrzysztofMoch May 16, 2024 13:08
@KrzysztofMoch KrzysztofMoch merged commit 66e0ba5 into TheWidlarzGroup:master May 17, 2024
3 checks passed
@YangJonghun YangJonghun deleted the fix/zero-start-position branch May 18, 2024 15:47
@ZakirBangash
Copy link
Contributor

ZakirBangash commented Jun 6, 2024

@freeboub @YangJonghun I have tiktok like app , where we scroll the videos, on android 9 S8 Samsung, some videos get stuck, now in my client phone two videos constantly stuck. while others are working, I just want to know whether this PR resolved this issue?

@freeboub
Copy link
Collaborator

freeboub commented Jun 6, 2024

@ZakirBangash let's try on 6.2 , will be released soon

@ZakirBangash
Copy link
Contributor

@freeboub Yes Please 🙏, We are in production App Nooberly, client reported this behaviour, Would be great help if we have this issue fixed.

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.

4 participants