Skip to content

Commit

Permalink
Add TimeProgressEstimator class
Browse files Browse the repository at this point in the history 
https://bugs.webkit.org/show_bug.cgi?id=268911
rdar://122463053

Reviewed by Eric Carlson and Youenn Fablet.

Simplify and move the emulated time progress management to its own class.
Fly-by: the time would have continued to progress when pause() was called
(until we got a new time update from the GPU process), reading the time following
a call to pause() would have temporarily returned the wrong value.

When seeking, the time should have stopped until the GPU process signalled we
had completed the seek.

The class is thread-safe when used with MediaSource, when used with CRABS
it will only be called from the main thread.

Covered by existing tests.

* Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.cpp:
(WebKit::MediaPlayerPrivateRemote::TimeProgressEstimator::TimeProgressEstimator):
(WebKit::MediaPlayerPrivateRemote::TimeProgressEstimator::currentTime const):
(WebKit::MediaPlayerPrivateRemote::TimeProgressEstimator::cachedTime const):
(WebKit::MediaPlayerPrivateRemote::TimeProgressEstimator::timeIsProgressing const):
(WebKit::MediaPlayerPrivateRemote::TimeProgressEstimator::pause):
(WebKit::MediaPlayerPrivateRemote::TimeProgressEstimator::setTime):
(WebKit::MediaPlayerPrivateRemote::TimeProgressEstimator::setRate):
(WebKit::MediaPlayerPrivateRemote::MediaPlayerPrivateRemote):
(WebKit::MediaPlayerPrivateRemote::pause):
(WebKit::MediaPlayerPrivateRemote::durationMediaTime const):
(WebKit::MediaPlayerPrivateRemote::currentMediaTime const):
(WebKit::MediaPlayerPrivateRemote::seekToTarget):
(WebKit::MediaPlayerPrivateRemote::seeked):
(WebKit::MediaPlayerPrivateRemote::rateChanged):
(WebKit::MediaPlayerPrivateRemote::playbackStateChanged):
(WebKit::MediaPlayerPrivateRemote::currentTimeChanged):
(WebKit::MediaPlayerPrivateRemote::performTaskAtMediaTime): fly-by: completionHandler wasn't called under some circumstances.
* Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.h:

Canonical link: https://commits.webkit.org/274278@main
  • Loading branch information
@jyavenard
jyavenard committed Feb 8, 2024
1 parent 91ff641 commit f9a82eb
Show file tree
Hide file tree
Showing 2 changed files with 94 additions and 35 deletions.
105 changes: 74 additions & 31 deletions Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
#include <WebCore/TextTrackRepresentation.h>
#include <WebCore/VideoLayerManager.h>
#include <wtf/HashMap.h>
#include <wtf/Locker.h>
#include <wtf/MachSendRight.h>
#include <wtf/MainThread.h>
#include <wtf/StringPrintStream.h>
Expand Down Expand Up @@ -110,14 +111,67 @@ using namespace WebCore;
} while (0)
#endif

MediaPlayerPrivateRemote::TimeProgressEstimator::TimeProgressEstimator(const MediaPlayerPrivateRemote& parent)
: m_parent(parent)
{
}

MediaTime MediaPlayerPrivateRemote::TimeProgressEstimator::currentTime() const
{
Locker locker { m_lock };

if (!m_timeIsProgressing)
return m_cachedMediaTime;

auto calculatedCurrentTime = m_cachedMediaTime + MediaTime::createWithDouble(m_rate * (MonotonicTime::now() - m_cachedMediaTimeQueryTime).seconds());
return std::min(std::max(calculatedCurrentTime, MediaTime::zeroTime()), m_parent.durationMediaTime());
}

MediaTime MediaPlayerPrivateRemote::TimeProgressEstimator::cachedTime() const
{
Locker locker { m_lock };
return m_cachedMediaTime;
}

bool MediaPlayerPrivateRemote::TimeProgressEstimator::timeIsProgressing() const
{
Locker locker { m_lock };
return m_timeIsProgressing;
}

void MediaPlayerPrivateRemote::TimeProgressEstimator::pause()
{
Locker locker { m_lock };
if (!m_timeIsProgressing)
return;
auto now = MonotonicTime::now();
m_cachedMediaTime += MediaTime::createWithDouble(m_rate * (now - m_cachedMediaTimeQueryTime).value());
m_cachedMediaTimeQueryTime = now;
m_timeIsProgressing = false;
}

void MediaPlayerPrivateRemote::TimeProgressEstimator::setTime(const MediaTime& time, const MonotonicTime& wallTime, std::optional<bool> timeIsProgressing)
{
Locker locker { m_lock };
m_cachedMediaTime = time;
m_cachedMediaTimeQueryTime = wallTime;
if (timeIsProgressing)
m_timeIsProgressing = *timeIsProgressing;
}

void MediaPlayerPrivateRemote::TimeProgressEstimator::setRate(double value)
{
Locker locker { m_lock };
m_rate = value;
}

MediaPlayerPrivateRemote::MediaPlayerPrivateRemote(MediaPlayer* player, MediaPlayerEnums::MediaEngineIdentifier engineIdentifier, MediaPlayerIdentifier playerIdentifier, RemoteMediaPlayerManager& manager)
:
: m_currentTimeEstimator(*this)
#if !RELEASE_LOG_DISABLED
m_logger(player->mediaPlayerLogger())
, m_logger(player->mediaPlayerLogger())
, m_logIdentifier(player->mediaPlayerLogIdentifier())
,
#endif
m_player(*player)
, m_player(*player)
, m_mediaResourceLoader(*player->createResourceLoader())
#if PLATFORM(COCOA)
, m_videoLayerManager(makeUniqueRef<VideoLayerManagerObjC>(logger(), logIdentifier()))
Expand Down Expand Up @@ -235,11 +289,7 @@ void MediaPlayerPrivateRemote::play()
void MediaPlayerPrivateRemote::pause()
{
m_cachedState.paused = true;
if (m_timeIsProgressing) {
auto now = MonotonicTime::now();
m_cachedMediaTime += MediaTime::createWithDouble(m_rate * (now - m_cachedMediaTimeQueryTime).value());
m_cachedMediaTimeQueryTime = now;
}
m_currentTimeEstimator.pause();
connection().send(Messages::RemoteMediaPlayerProxy::Pause(), m_id);
}

Expand Down Expand Up @@ -280,23 +330,20 @@ MediaTime MediaPlayerPrivateRemote::durationMediaTime() const
return m_mediaSourcePrivate->duration();
#endif

ASSERT(isMainRunLoop());
return m_cachedState.duration;
}

MediaTime MediaPlayerPrivateRemote::currentMediaTime() const
{
if (!m_timeIsProgressing)
return m_cachedMediaTime;

auto calculatedCurrentTime = m_cachedMediaTime + MediaTime::createWithDouble(m_rate * (MonotonicTime::now() - m_cachedMediaTimeQueryTime).seconds());
return std::min(std::max(calculatedCurrentTime, MediaTime::zeroTime()), durationMediaTime());
return m_currentTimeEstimator.currentTime();
}

void MediaPlayerPrivateRemote::seekToTarget(const WebCore::SeekTarget& target)
{
ALWAYS_LOG(LOGIDENTIFIER, target);
m_seeking = true;
m_cachedMediaTime = target.time;
m_currentTimeEstimator.setTime(target.time, MonotonicTime::now(), false);
connection().send(Messages::RemoteMediaPlayerProxy::SeekToTarget(target), m_id);
}

Expand Down Expand Up @@ -372,8 +419,7 @@ void MediaPlayerPrivateRemote::seeked(const MediaTime& time)
{
ALWAYS_LOG(LOGIDENTIFIER, time);
m_seeking = false;
m_cachedMediaTime = time;
m_cachedMediaTimeQueryTime = MonotonicTime::now();
m_currentTimeEstimator.setTime(time, MonotonicTime::now());
if (auto player = m_player.get())
player->seeked(time);
}
Expand Down Expand Up @@ -401,15 +447,15 @@ bool MediaPlayerPrivateRemote::seeking() const
void MediaPlayerPrivateRemote::rateChanged(double rate)
{
m_rate = rate;
m_currentTimeEstimator.setRate(rate);
if (auto player = m_player.get())
player->rateChanged();
}

void MediaPlayerPrivateRemote::playbackStateChanged(bool paused, MediaTime&& mediaTime, MonotonicTime&& wallTime)
{
m_cachedState.paused = paused;
m_cachedMediaTime = mediaTime;
m_cachedMediaTimeQueryTime = wallTime;
m_currentTimeEstimator.setTime(mediaTime, wallTime);
if (auto player = m_player.get())
player->playbackStateChanged();
}
Expand Down Expand Up @@ -437,16 +483,16 @@ void MediaPlayerPrivateRemote::sizeChanged(WebCore::FloatSize naturalSize)

void MediaPlayerPrivateRemote::currentTimeChanged(const MediaTime& mediaTime, const MonotonicTime& queryTime, bool timeIsProgressing)
{
auto reverseJump = mediaTime < m_cachedMediaTime;
auto oldCachedTime = m_currentTimeEstimator.cachedTime();
auto oldTimeIsProgressing = m_currentTimeEstimator.timeIsProgressing();
auto reverseJump = mediaTime < oldCachedTime;
if (reverseJump)
ALWAYS_LOG(LOGIDENTIFIER, "time jumped backwards, was ", m_cachedMediaTime, ", is now ", mediaTime);
ALWAYS_LOG(LOGIDENTIFIER, "time jumped backwards, was ", oldCachedTime, ", is now ", mediaTime);

std::swap(timeIsProgressing, m_timeIsProgressing);
auto oldCachedTime = std::exchange(m_cachedMediaTime, mediaTime);
m_cachedMediaTimeQueryTime = queryTime;
m_currentTimeEstimator.setTime(mediaTime, queryTime, timeIsProgressing);

if (reverseJump
|| (timeIsProgressing != m_timeIsProgressing && m_cachedMediaTime != oldCachedTime && !m_cachedState.paused)) {
|| (timeIsProgressing != oldTimeIsProgressing && mediaTime != oldCachedTime && !m_cachedState.paused)) {
if (auto player = m_player.get())
player->timeChanged();
}
Expand Down Expand Up @@ -1407,12 +1453,9 @@ void MediaPlayerPrivateRemote::setPreferredDynamicRangeMode(WebCore::DynamicRang

bool MediaPlayerPrivateRemote::performTaskAtMediaTime(WTF::Function<void()>&& completionHandler, const MediaTime& mediaTime)
{
auto asyncReplyHandler = [weakThis = WeakPtr { *this }, this, completionHandler = WTFMove(completionHandler)](std::optional<MediaTime> currentTime, std::optional<MonotonicTime> queryTime) mutable {
if (!weakThis || !currentTime || !queryTime)
return;

m_cachedMediaTime = *currentTime;
m_cachedMediaTimeQueryTime = *queryTime;
auto asyncReplyHandler = [weakThis = WeakPtr { *this }, completionHandler = WTFMove(completionHandler)](std::optional<MediaTime> currentTime, std::optional<MonotonicTime> queryTime) mutable {
if (RefPtr protectedThis = weakThis.get(); protectedThis && currentTime && queryTime)
protectedThis->m_currentTimeEstimator.setTime(*currentTime, *queryTime);
completionHandler();
};

Expand Down
24 changes: 20 additions & 4 deletions Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
#include <WebCore/MediaPlayerPrivate.h>
#include <WebCore/SecurityOriginData.h>
#include <WebCore/VideoFrameMetadata.h>
#include <wtf/Lock.h>
#include <wtf/LoggerHelper.h>
#include <wtf/MediaTime.h>
#include <wtf/RefCounted.h>
Expand Down Expand Up @@ -197,6 +198,25 @@ class MediaPlayerPrivateRemote final
LayerHostingContextID hostingContextID()const override;
void setLayerHostingContextID(LayerHostingContextID inID);
private:
class TimeProgressEstimator final {
public:
explicit TimeProgressEstimator(const MediaPlayerPrivateRemote& parent);
MediaTime currentTime() const;
MediaTime cachedTime() const;
bool timeIsProgressing() const;
void pause();
void setTime(const MediaTime&, const MonotonicTime& wallTime, std::optional<bool> timeIsProgressing = std::nullopt);
void setRate(double);

private:
mutable Lock m_lock;
bool m_timeIsProgressing WTF_GUARDED_BY_LOCK(m_lock) { false };
MediaTime m_cachedMediaTime WTF_GUARDED_BY_LOCK(m_lock);
MonotonicTime m_cachedMediaTimeQueryTime WTF_GUARDED_BY_LOCK(m_lock);
double m_rate WTF_GUARDED_BY_LOCK(m_lock) { 1.0 };
const MediaPlayerPrivateRemote& m_parent;
};
TimeProgressEstimator m_currentTimeEstimator;

#if !RELEASE_LOG_DISABLED
const Logger& logger() const final { return m_logger; }
Expand Down Expand Up @@ -467,9 +487,6 @@ class MediaPlayerPrivateRemote final
WebCore::SecurityOriginData m_documentSecurityOrigin;
mutable HashMap<WebCore::SecurityOriginData, std::optional<bool>> m_isCrossOriginCache;

MediaTime m_cachedMediaTime;
MonotonicTime m_cachedMediaTimeQueryTime;

WebCore::MediaPlayer::VideoGravity m_videoFullscreenGravity { WebCore::MediaPlayer::VideoGravity::ResizeAspect };
MonotonicTime m_lastPlaybackQualityMetricsQueryTime;
Seconds m_videoPlaybackMetricsUpdateInterval;
Expand All @@ -480,7 +497,6 @@ class MediaPlayerPrivateRemote final
bool m_seeking { false };
bool m_isCurrentPlaybackTargetWireless { false };
bool m_waitingForKey { false };
bool m_timeIsProgressing { false };
std::optional<bool> m_shouldMaintainAspectRatio;
std::optional<bool> m_pageIsVisible;
RefPtr<RemoteVideoFrameProxy> m_videoFrameForCurrentTime;
Expand Down

0 comments on commit f9a82eb

Please sign in to comment.