[webkit-changes] [WebKit/WebKit] ad4c63: VideoMediaSampleRenderer will produce out of order...

[Jean-Yves Avenard]

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: ad4c63c079db6dd47dd2487858aae1a850e64bbf
      https://github.com/WebKit/WebKit/commit/ad4c63c079db6dd47dd2487858aae1a850e64bbf
  Author: Jean-Yves Avenard <jya at apple.com>
  Date:   2024-11-18 (Mon, 18 Nov 2024)

  Changed paths:
    M Source/WebCore/platform/graphics/cocoa/VideoMediaSampleRenderer.h
    M Source/WebCore/platform/graphics/cocoa/VideoMediaSampleRenderer.mm

  Log Message:
  -----------
  VideoMediaSampleRenderer will produce out of order, corrupted frame when used with b-frames.
https://bugs.webkit.org/show_bug.cgi?id=283235
rdar://140046888

Reviewed by Youenn Fablet.

1- The queue used to feed the decoder was a CMBufferQueue which re-order samples in presentation order.
So we ended up feeding the decoder the frames in presentation order rather than decode order, leading to decoding artifacts
2- The way the WebCoreDecompression was used by the VideoMediaSampleRenderer, was to immediately queue the decoded frame to the AVSampleBufferDisplayLayer,
without letting the CMBufferQueue re-sort the frames by presentation order. So frames were displayed in decode order instead.

To solve 1-, we remove the use of a sorted CMBufferQueue and instead use a plain Dequeue. The Dequeue store the compressed
image along the flushing session id. Whenever we process the compressed frame, if the Ids don't match, the frame is dropped.
For 2- we only add the decoded frame at the time it is ready for display, using the AV synchroniser's CMTimeBase timer.
By only ever dequeuing the video frame at the time we should be displaying it, we guarantee that the frames have been fully
sorted in presentation order, without the downside of added latency (as we would otherwise have to wait until we've reached the
minimum required number of frames in the queue: the "H264 sliding window").
If we were to display a frame out of order, those future incoming earlier frames would have been dropped anyway by the time they are received
being now too late.

Fly-by: setTimeBase if called multiple times, could potentially cause access to the m_timebase member to be access concurrently.
We limit the use so that it can be either set once or cleared and protect the variable member with a lock.

Manually tested. The VideoMediaSampleRenderer isn't currently used with players supporting B-Frames.

* Source/WebCore/platform/graphics/cocoa/VideoMediaSampleRenderer.h:
(WebCore::VideoMediaSampleRenderer::timebase const): Deleted.
* Source/WebCore/platform/graphics/cocoa/VideoMediaSampleRenderer.mm:
(WebCore::VideoMediaSampleRenderer::~VideoMediaSampleRenderer): Cancel (flush) any pending decoding tasks.
(WebCore::VideoMediaSampleRenderer::isReadyForMoreMediaData const):
(WebCore::VideoMediaSampleRenderer::maybeBecomeReadyForMoreMediaData):
(WebCore::VideoMediaSampleRenderer::setTimebase): Whenever the CMTimeSource calls, maybe enqueue for display the earliest decoded frame.
(WebCore::VideoMediaSampleRenderer::clearTimebase):
(WebCore::VideoMediaSampleRenderer::timebase const):
(WebCore::VideoMediaSampleRenderer::enqueueSample):
(WebCore::VideoMediaSampleRenderer::decodeNextSample):
(WebCore::VideoMediaSampleRenderer::initializeDecompressionSession): Fly-by: the current code can't deal with m_decodedSampleQueue changing mid-flight
as accessed to this member would become racy. As such, we make it an error to clear m_decodedSampleQueue.
There's also no need to set the timebase and ResourceOwner to the WebCoreDecompression as we aren't using its internal samples queueing.
(WebCore::VideoMediaSampleRenderer::decodedFrameAvailable):
(WebCore::VideoMediaSampleRenderer::maybeQueueFrameForDisplay):
(WebCore::VideoMediaSampleRenderer::flushCompressedSampleQueue):
(WebCore::VideoMediaSampleRenderer::flushDecodedSampleQueue):
(WebCore::VideoMediaSampleRenderer::cancelTimer):
(WebCore::VideoMediaSampleRenderer::purgeDecodedSampleQueue):
(WebCore::VideoMediaSampleRenderer::flush):
(WebCore::VideoMediaSampleRenderer::expectMinimumUpcomingSampleBufferPresentationTime): Fly-by: When using a decompression session
frames queued for display are always earlier than the value set in expectMinimumUpcomingSampleBufferPresentationTime as
the time given if of the compressed frame about to be enqueued leading to very AVFoundation verbose warnings.
(WebCore::VideoMediaSampleRenderer::copyDisplayedPixelBuffer): Fly-By, if we don't have an active WebCoreDecompressionSession and [AVSBDL copyDisplayedPixelBuffer]
didn't return an image, exit early as there's nothing to retrieve in the decoded queue.
(WebCore::VideoMediaSampleRenderer::copyDisplayedPixelBuffer): add workQueue assertion.

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



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