Skip to content

Commit

Permalink
Update WebCodecsVideoFrame color space to create a valid VideoColorSp…
Browse files Browse the repository at this point in the history
…ace even when closed

https://bugs.webkit.org/show_bug.cgi?id=258802
rdar://111679281

Reviewed by Eric Carlson.

Align with Chrome by creating an empty color space if the frame is closed before color space is created.
Do not nullify color space when frame is closed, as per spec.

* LayoutTests/imported/w3c/web-platform-tests/webcodecs/videoFrame-construction.any-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/webcodecs/videoFrame-construction.any.worker-expected.txt:
* LayoutTests/platform/glib/imported/w3c/web-platform-tests/webcodecs/videoFrame-construction.any-expected.txt: Removed.
* Source/WebCore/Modules/webcodecs/WebCodecsVideoFrame.cpp:
(WebCore::WebCodecsVideoFrame::clone):
(WebCore::WebCodecsVideoFrame::colorSpace const):
* Source/WebCore/Modules/webcodecs/WebCodecsVideoFrame.h:

Canonical link: https://commits.webkit.org/265748@main
  • Loading branch information
youennf committed Jul 4, 2023
1 parent 484172c commit b350aaf
Show file tree
Hide file tree
Showing 6 changed files with 9 additions and 44 deletions.
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@

PASS Test we can construct a VideoFrame.
FAIL Test closed VideoFrame. null is not an object (evaluating 'frame.colorSpace.primaries')
PASS Test closed VideoFrame.
PASS Test we can construct a VideoFrame with a negative timestamp.
PASS Test that timestamp is required when constructing VideoFrame from ImageBitmap
PASS Test that timestamp is required when constructing VideoFrame from OffscreenCanvas
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@

PASS Test we can construct a VideoFrame.
FAIL Test closed VideoFrame. null is not an object (evaluating 'frame.colorSpace.primaries')
PASS Test closed VideoFrame.
PASS Test we can construct a VideoFrame with a negative timestamp.
PASS Test that timestamp is required when constructing VideoFrame from ImageBitmap
PASS Test that timestamp is required when constructing VideoFrame from OffscreenCanvas
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
Harness Error (TIMEOUT), message = null

PASS Test we can construct a VideoFrame.
FAIL Test closed VideoFrame. null is not an object (evaluating 'frame.colorSpace.primaries')
PASS Test closed VideoFrame.
PASS Test we can construct a VideoFrame with a negative timestamp.
PASS Test that timestamp is required when constructing VideoFrame from ImageBitmap
PASS Test that timestamp is required when constructing VideoFrame from OffscreenCanvas
Expand Down
11 changes: 5 additions & 6 deletions Source/WebCore/Modules/webcodecs/WebCodecsVideoFrame.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -501,7 +501,7 @@ ExceptionOr<Ref<WebCodecsVideoFrame>> WebCodecsVideoFrame::clone(ScriptExecution

auto clone = adoptRef(*new WebCodecsVideoFrame(context, WebCodecsVideoFrameData { m_data }));

clone->m_colorSpace = colorSpace();
clone->m_colorSpace = &colorSpace();
clone->m_codedRect = codedRect();
clone->m_visibleRect = visibleRect();
clone->m_isDetached = m_isDetached;
Expand Down Expand Up @@ -529,7 +529,6 @@ void WebCodecsVideoFrame::close()

m_codedRect = nullptr;
m_visibleRect = nullptr;
m_colorSpace = nullptr;
}

DOMRectReadOnly* WebCodecsVideoFrame::codedRect() const
Expand Down Expand Up @@ -566,12 +565,12 @@ void WebCodecsVideoFrame::setVisibleRect(const DOMRectInit& rect)
m_data.visibleHeight = rect.height;
}

VideoColorSpace* WebCodecsVideoFrame::colorSpace() const
VideoColorSpace& WebCodecsVideoFrame::colorSpace() const
{
if (!m_colorSpace && m_data.internalFrame)
m_colorSpace = VideoColorSpace::create(m_data.internalFrame->colorSpace());
if (!m_colorSpace)
m_colorSpace = m_data.internalFrame ? VideoColorSpace::create(m_data.internalFrame->colorSpace()) : VideoColorSpace::create();

return m_colorSpace.get();
return *m_colorSpace.get();
}

} // namespace WebCore
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/Modules/webcodecs/WebCodecsVideoFrame.h
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ class WebCodecsVideoFrame : public RefCounted<WebCodecsVideoFrame>, public Conte
size_t displayHeight() const { return m_data.displayHeight; }
std::optional<uint64_t> duration() const { return m_data.duration; }
int64_t timestamp() const { return m_data.timestamp; }
VideoColorSpace* colorSpace() const;
VideoColorSpace& colorSpace() const;

struct CopyToOptions {
std::optional<DOMRectInit> rect;
Expand Down

0 comments on commit b350aaf

Please sign in to comment.