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

Improve same media playback completion handler #226

Open
defagos opened this issue Jul 17, 2020 · 4 comments
Open

Improve same media playback completion handler #226

defagos opened this issue Jul 17, 2020 · 4 comments

Comments

@defagos
Copy link
Member

defagos commented Jul 17, 2020

See https://github.com/SRGSSR/playsrg-ios/issues/154.

@defagos
Copy link
Member Author

defagos commented Jul 17, 2020

As discussed with @oiledCode, and in addition to the changes proposed in the Play SRG issue above, it is likely we should also call the prepare to play completion handler even if in some cases we do nothing. This would provide for a more consistent behavior.

This change has been made on a feature/completion-consistency for testing purposes. A corresponding unit test should also be written.

@pyby pyby added done and removed done labels Jul 27, 2020
@pyby pyby changed the title Improve same media playback behavior Improve same media playback completion handler Jul 27, 2020
@pyby
Copy link
Member

pyby commented Jul 27, 2020

A unit test has been added on feature/completion-consistency.

@pyby pyby added this to the 5.0.1 milestone Jul 28, 2020
@pyby pyby added the done label Jul 28, 2020
@defagos defagos closed this as completed Sep 2, 2020
@defagos defagos reopened this Jun 28, 2021
@defagos
Copy link
Member Author

defagos commented Jun 28, 2021

Not sure this fix was a good idea, see https://appcenter.ms/orgs/RTS-Organization/apps/Play-RTS-1/crashes/errors/4277850553u/overview.

If we exit early and call the completion handler, this means the -play which might be called from it might not find any content URL, prepare playback again, thus ending in an infinite recursion. Relevant code:

Some ideas:

  • In the play with parameters implementation ) call -play on the media player controller directly. Problem is that the Letterbox controller play must be able to handle the same cases as today, and that clients of our Letterbox controller API could still do what we currently do. So we would not fix the issue.
  • Only call the completion handler early if there is a content URL. This seems hacky and is inconsistent.
  • Remove early completion handler call, i.e. revert this fix, but we must check with @oiledCode first whether this behavior is still being used in their code.

A proper fix would require #227 to be implemented anyway.

@defagos
Copy link
Member Author

defagos commented Jul 27, 2021

We had a rare crash in production which seems to confirm this intuition.

This crash is not related to an other infinite loop which was fixed at the SRG Media Player level.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants