SRGSSR / srgletterbox-apple

The official SRG SSR media playback experience
https://srgssr.github.io/marketing/letterbox/
MIT License
14 stars 7 forks source link

Playback incorrectly restarts in background without user interaction #231

Closed defagos closed 4 years ago

defagos commented 4 years ago

We recently received several (nice) complaints from users about radio restarting in the background in the middle of a meeting or when receiving calls. This seems to occur more often as of iOS 14.

defagos commented 4 years ago

This issue is related to automatic restart after the network becomes reachable again. This issue is not new, but according to my tests with an iPad running iOS 10 the new iOS 14 might be more tolerant in background, letting paused playback restart, while this was not the case in the past (to be confirmed for other OS versions, though).

Nevertheless, I made a fix proposal which replaced the stopped boolean we introduced in the past with a more obvious approach: Only recover from unreachable network errors when the network becomes reachable. This will avoid all issues we had in the past with this feature and improve our code.

We will lose automatic recovery from other failures (e.g. player errors due to network being unreachable), but the user can always tap to resume playback, so I think this change is safe and therefore for the best.

Available for review on feature/unexpected-resume-fix.

defagos commented 4 years ago

Issue confirmed by @pyby on iOS 13 as well.

defagos commented 4 years ago

As discussed with @pyby we have further improved error management:

defagos commented 4 years ago

As discussed with @pyby, we started with an unrestrictive condition on NSURLErrors to see whether the error message could appear in conditions where it does not make sense.

I found such a case with recently published content with bad streams (either 404 or 416), two episodes from the same TV series The 404 leads to an error containing a NSURLErrorFileDoesNotExist within the underlying error stack (and thus is incorrectly caught by our current greedy condition as a network error), while the 416 isn't.

For this reason, starting restrictive is probably a better idea. The fix is made on a feature/limited-network-errors branch.