[Cocoa] Make the hostTime parameter to playSession a Monotonic time
Created attachment 430317 [details] Patch
Comment on attachment 430317 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=430317&action=review > Source/WebCore/Modules/mediasession/MediaSessionCoordinator.cpp:355 > + if (!std::isfinite(delta) || delta <= 0.5_s) { Why delta <= 0.5_s, it seems we could use dispatchAfter? Maybe a name for 0.5_s would help understanding. > Source/WebCore/Modules/mediasession/MediaSessionCoordinatorPrivate.h:53 > + } Is it needed to keep it? Can we just move all call sites to use MonotonicTime? Not sure we need WTFMove(atTime), especially since we are passing by value anyway. > Source/WebKit/UIProcess/API/Cocoa/WKWebViewTesting.mm:606 > + m_coordinatorClient->playSession({ }, std::optional<MonotonicTime>(), makeBlockPtr(completionHandler)); I guess the change would not be needed if we would keep only one variant of playSession.
<rdar://problem/79036032>
(In reply to youenn fablet from comment #2) > Comment on attachment 430317 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=430317&action=review > > > Source/WebCore/Modules/mediasession/MediaSessionCoordinator.cpp:355 > > + if (!std::isfinite(delta) || delta <= 0.5_s) { > > Why delta <= 0.5_s, it seems we could use dispatchAfter? > Maybe a name for 0.5_s would help understanding. Sure, it'll take some time to pre-roll the decoder; so giving a half second head start is just a magic number. I have an idea how to get rid of this in a future patch, but for now, I'll make it a named constant. > > Source/WebCore/Modules/mediasession/MediaSessionCoordinatorPrivate.h:53 > > + } > > Is it needed to keep it? Can we just move all call sites to use > MonotonicTime? > Not sure we need WTFMove(atTime), especially since we are passing by value > anyway. Now that we've upstreamed, we can get rid of this. > > Source/WebKit/UIProcess/API/Cocoa/WKWebViewTesting.mm:606 > > + m_coordinatorClient->playSession({ }, std::optional<MonotonicTime>(), makeBlockPtr(completionHandler)); > > I guess the change would not be needed if we would keep only one variant of > playSession. Yep!
Created attachment 432061 [details] Patch for landing
Created attachment 432063 [details] Patch for landing
Committed r279186 (239081@main): <https://commits.webkit.org/239081@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 432063 [details].
Comment on attachment 432063 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=432063&action=review > Source/WebCore/Modules/mediasession/MediaSessionCoordinatorPrivate.h:36 > +#include <wtf/Optional.h> Oops! This is wrong!