Bug 227047 – [iOS] Fullscreen video playback gets stuck after interacting with the playback controls

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Peng Liu]

[iOS] Fullscreen video playback gets stuck after interacting with the playback controls

[Peng Liu]

<rdar://79103461>

[Peng Liu]

Created attachment 431486 [details]
Patch

[Peng Liu]

Created attachment 431487 [details]
Patch

[Devin Rousso]

Comment on attachment 431486 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=431486&action=review

> Source/WebCore/platform/cocoa/PlaybackSessionModel.h:105
> +    virtual void rateChanged(bool /* isPlaying */, bool /* isStalled */, double /* playbackRate */, double /* defaultPlaybackRate */) { }

Could we maybe have an `OptionSet` of enum flags?

> Source/WebCore/platform/cocoa/PlaybackSessionModelMediaElement.mm:-55
> -static const float StalledPlaybackRate = 0.00000001f;

Do we still need to report this value (or something like it) to AVKit?  Or will AVKit be fine with a playback rate of 1 (or 2 or 0.5 or whatever) when we're still loading?

> Source/WebCore/platform/ios/PlaybackSessionInterfaceAVKit.mm:130
> +    if (!isStalled)

I wonder if instead of not modifying `m_playerController` we should instead have a `stalled` property on `WebAVPlayerController` and have `setRate:` ignore/defer modifications if `stalled`.  This way, if AVKit calls `setRate:` then we will wait to update the rate until we're no longer stalled (e.g. inside `setStalled:` you could send the `rate` back to the `model`).  Though I think all of this assumes my comment on PlaybackSessionModelMediaElement.mm:55 is accurate.

[Peng Liu]

Created attachment 431494 [details]
Fix build failures on macOS

[Peng Liu]

Comment on attachment 431486 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=431486&action=review

>> Source/WebCore/platform/cocoa/PlaybackSessionModel.h:105
>> +    virtual void rateChanged(bool /* isPlaying */, bool /* isStalled */, double /* playbackRate */, double /* defaultPlaybackRate */) { }
> 
> Could we maybe have an `OptionSet` of enum flags?

Yes. Like an `OptionSet` of
```
enum class PlaybackState {
    Playing = 1 << 0,
    Stalled = 1 << 1,
};
```

>> Source/WebCore/platform/cocoa/PlaybackSessionModelMediaElement.mm:-55
>> -static const float StalledPlaybackRate = 0.00000001f;
> 
> Do we still need to report this value (or something like it) to AVKit?  Or will AVKit be fine with a playback rate of 1 (or 2 or 0.5 or whatever) when we're still loading?

With this patch, I cannot reproduce bug 211797. So looks like AVKit does not need this value anymore.

>> Source/WebCore/platform/ios/PlaybackSessionInterfaceAVKit.mm:130
>> +    if (!isStalled)
> 
> I wonder if instead of not modifying `m_playerController` we should instead have a `stalled` property on `WebAVPlayerController` and have `setRate:` ignore/defer modifications if `stalled`.  This way, if AVKit calls `setRate:` then we will wait to update the rate until we're no longer stalled (e.g. inside `setStalled:` you could send the `rate` back to the `model`).  Though I think all of this assumes my comment on PlaybackSessionModelMediaElement.mm:55 is accurate.

Hmm, this approach sounds safer. However, if `m_playerController` does not report the rate back to the WebContent process when its `stalled` property is YES, its rate might be overwritten by a value comes from the WebContent process. That could happen when a user changes playback rate in fullscreen immediately after scrubbing the video.

[Eric Carlson]

Comment on attachment 431494 [details]
Fix build failures on macOS

View in context: https://bugs.webkit.org/attachment.cgi?id=431494&action=review

> Source/WebCore/platform/cocoa/PlaybackSessionModel.h:105
> +    virtual void rateChanged(bool /* isPlaying */, bool /* isStalled */, double /* playbackRate */, double /* defaultPlaybackRate */) { }

I agree that an enums and an OptionSet would be nicer than bools

[Peng Liu]

Created attachment 431727 [details]
Revise the patch based on Devin and Eric's comments

[EWS]

Committed r279043 (238963@main): <https://commits.webkit.org/238963@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 431727 [details].

[Peng Liu]

*** Bug 227106 has been marked as a duplicate of this bug. ***