[webkit-changes] [WebKit/WebKit] e77ffd: [MSE] video's currentTime can be further than the ...

[Jean-Yves Avenard]

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: e77ffd167967c24232e8dcbac584ac1809524c90
      https://github.com/WebKit/WebKit/commit/e77ffd167967c24232e8dcbac584ac1809524c90
  Author: Jean-Yves Avenard <jya at apple.com>
  Date:   2024-03-27 (Wed, 27 Mar 2024)

  Changed paths:
    M LayoutTests/media/media-source/media-managedmse-noresumeafterpause-expected.txt
    M LayoutTests/media/media-source/media-managedmse-noresumeafterpause.html
    A LayoutTests/media/media-source/media-managedmse-resume-after-remove-expected.txt
    A LayoutTests/media/media-source/media-managedmse-resume-after-remove.html
    M LayoutTests/media/media-source/media-managedmse-resume-after-stall-expected.txt
    M LayoutTests/media/media-source/media-managedmse-resume-after-stall.html
    A LayoutTests/media/media-source/media-managedmse-stall-endofstream-expected.txt
    A LayoutTests/media/media-source/media-managedmse-stall-endofstream.html
    M LayoutTests/media/media-source/media-source-fudge-factor-expected.txt
    M LayoutTests/media/media-source/media-source-fudge-factor.html
    M LayoutTests/platform/glib/TestExpectations
    M LayoutTests/platform/mac-wk1/TestExpectations
    M LayoutTests/platform/mac/TestExpectations
    M Source/WebCore/Modules/mediasource/ManagedMediaSource.cpp
    M Source/WebCore/Modules/mediasource/MediaSource.cpp
    M Source/WebCore/Modules/mediasource/MediaSource.h
    M Source/WebCore/platform/graphics/MediaSourcePrivate.cpp
    M Source/WebCore/platform/graphics/MediaSourcePrivate.h
    M Source/WebCore/platform/graphics/SourceBufferPrivate.cpp
    M Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.h
    M Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm
    M Source/WebCore/platform/graphics/avfoundation/objc/MediaSourcePrivateAVFObjC.h
    M Source/WebCore/platform/graphics/avfoundation/objc/MediaSourcePrivateAVFObjC.mm
    M Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.cpp
    M Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.h
    M Source/WebKit/WebProcess/GPU/media/MediaSourcePrivateRemote.cpp

  Log Message:
  -----------
  [MSE] video's currentTime can be further than the gap's start time
https://bugs.webkit.org/show_bug.cgi?id=270618
rdar://124186726

Reviewed by Jer Noble.

The AVFObjC MSE player relies on using AVSampleBufferRenderSynchronizer
which provides the base routines to handle A/V sync, seeking and determining
the playback current time.
The code used notification listeners such as AVSampleBufferRenderSynchronizerRateDidChangeNotification
to determine when the actual playback rate changed to 0.
But this notification will be received only if playback was paused through
external means such as on iOS "by a phone call or another non-mixable app starting playback".
But actually AVSampleBufferRenderSynchronizer never stops otherwise, even
when it has run out of data to decode and play such as when there's a gap
in the buffered range or even if the currentTime has gone past the media's duration.
The MediaSource's monitorSourceBuffer would monitor the current time and
the buffered range and would pause the media element if it detected that
we no longer had anything to play. However this occurs in the content process
while decoding and playback occurs in the GPU process. The inherent latency
between the two processes means that the CP would set the readyState to
be HaveCurrentData which would make the MediaPlayerPrivateMediaSourceAVFObjC
pause playback, but by the time this was received the AVSampleBufferRenderSynchronizer's
timebase/clock would have already progressed further than it should have.
Worse, in combination with the time clock used by the MediaPlayerPrivateRemote
you would end up with nonsensical currentTime, positioned in the middle
of a media data gap, causing the readyState to move back to HaveMetadataData.
Similarly, you could end up with currentTime being significantly past
the media's duration. (When using a debugger, and blocking the GPU process
I saw currentTime being further than 90s past where it should have been).

To fix this issue, we adds two steps:
1- In the MediaPlayerPrivateRemote, when using MSE we ensure that the currentTime never
goes past the start of a gap greater than 2/24th of a second (the SourceBuffer's timeFuzzingFactor),
nor past the media's duration.
2- Add stall detections in the MediaPlayerPrivateMediaSourceAVFObjC by
using time observers with AVSampleBufferRenderSynchronizer that will
immediately pause playback once we reached the start of a gap.

To reduce the latency between data being added to the source buffer and the
GPU's MediaPlayer being notified that new data got added, we make the SourceBufferPrivate
immediately notifies its MediaSourcePrivate's parent that the buffered data
has changed.

Add tests to ensure that we always stall and fire the waiting event where
a gap starts.

* LayoutTests/media/media-source/media-managedmse-noresumeafterpause-expected.txt:
* LayoutTests/media/media-source/media-managedmse-noresumeafterpause.html:
* LayoutTests/media/media-source/media-managedmse-resume-after-remove-expected.txt: Added.
* LayoutTests/media/media-source/media-managedmse-resume-after-remove.html: Copied from LayoutTests/media/media-source/media-managedmse-resume-after-stall.html.
* LayoutTests/media/media-source/media-managedmse-resume-after-stall-expected.txt:
* LayoutTests/media/media-source/media-managedmse-resume-after-stall.html:
* LayoutTests/media/media-source/media-managedmse-stall-endofstream-expected.txt: Added.
* LayoutTests/media/media-source/media-managedmse-stall-endofstream.html: Copied from LayoutTests/media/media-source/media-managedmse-resume-after-stall.html.
* LayoutTests/media/media-source/media-source-fudge-factor-expected.txt:
* LayoutTests/media/media-source/media-source-fudge-factor.html: The test relied on timers to check
value which would cause intermittent failures. Test had been marked as expected to fail
on mac. Re-write the test using events instead of timers.
* LayoutTests/platform/glib/TestExpectations:
* LayoutTests/platform/mac-wk1/TestExpectations:
* LayoutTests/platform/mac/TestExpectations:
* Source/WebCore/Modules/mediasource/ManagedMediaSource.cpp:
(WebCore::ManagedMediaSource::monitorSourceBuffers):
* Source/WebCore/Modules/mediasource/MediaSource.cpp:
(WebCore::MediaSource::hasFutureTime):
* Source/WebCore/Modules/mediasource/MediaSource.h:
* Source/WebCore/platform/graphics/MediaSourcePrivate.cpp:
(WebCore::MediaSourcePrivate::hasFutureTime const):
(WebCore::MediaSourcePrivate::trackBufferedChanged):
(WebCore::MediaSourcePrivate::hasBufferedData const):
(WebCore::MediaSourcePrivate::timeIsProgressing const):
* Source/WebCore/platform/graphics/MediaSourcePrivate.h:
* Source/WebCore/platform/graphics/SourceBufferPrivate.cpp:
(WebCore::SourceBufferPrivate::updateBuffered):
* Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.h:
* Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm:
(WebCore::MediaPlayerPrivateMediaSourceAVFObjC::MediaPlayerPrivateMediaSourceAVFObjC):
(WebCore::MediaPlayerPrivateMediaSourceAVFObjC::~MediaPlayerPrivateMediaSourceAVFObjC):
(WebCore::MediaPlayerPrivateMediaSourceAVFObjC::currentTime const):
(WebCore::MediaPlayerPrivateMediaSourceAVFObjC::clampTimeToSensicalValue const):
(WebCore::MediaPlayerPrivateMediaSourceAVFObjC::setCurrentTimeDidChangeCallback):
(WebCore::MediaPlayerPrivateMediaSourceAVFObjC::bufferedChanged):
(WebCore::MediaPlayerPrivateMediaSourceAVFObjC::durationChanged): we no longer needs to
specifically add an observer for when currentTime reaches the end of the media. This is taken
care by the more generic gap detection.
(WebCore::MediaPlayerPrivateMediaSourceAVFObjC::setReadyState):
(WebCore::MediaPlayerPrivateMediaSourceAVFObjC::clampTimeToLastSeekTime const): Deleted.
* Source/WebCore/platform/graphics/avfoundation/objc/MediaSourcePrivateAVFObjC.h:
* Source/WebCore/platform/graphics/avfoundation/objc/MediaSourcePrivateAVFObjC.mm:
(WebCore::MediaSourcePrivateAVFObjC::removeSourceBuffer):
(WebCore::MediaSourcePrivateAVFObjC::player const):
(WebCore::MediaSourcePrivateAVFObjC::bufferedChanged):
(WebCore::MediaSourcePrivateAVFObjC::trackBufferedChanged):
* Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.cpp:
(WebKit::MediaPlayerPrivateRemote::TimeProgressEstimator::cachedTimeWithLockHeld const):
(WebKit::MediaPlayerPrivateRemote::TimeProgressEstimator::timeIsProgressing const):
(WebKit::MediaPlayerPrivateRemote::currentTime const):
(WebKit::MediaPlayerPrivateRemote::currentTimeWithLockHeld const):
(WebKit::MediaPlayerPrivateRemote::currentOrPendingSeekTime const):
(WebKit::MediaPlayerPrivateRemote::setReadyState):
* Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.h:
* Source/WebKit/WebProcess/GPU/media/MediaSourcePrivateRemote.cpp:
(WebKit::MediaSourcePrivateRemote::setMediaPlayerReadyState):

Canonical link: https://commits.webkit.org/276761@main



To unsubscribe from these emails, change your notification settings at https://github.com/WebKit/WebKit/settings/notifications