Skip to content

Commit

Permalink
[GStreamer] Make GstMappedFrame assert when it is not initialized
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=269980

Reviewed by Carlos Garcia Campos.

This way we can align the implementation of GstMappedFrame with GstMappedBuffer and assert when its data is accessed but
it was not properly mapped at initialization.

Even when now it is properly protected at members access, additional checks were added in some places where its
instantiation was not being checked.

A fly-by improvement is making the constructor taking GRefPtr<GstSample> to take const & to avoid unnecessary ref/unref.

* Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.h:
(WebCore::GstMappedFrame::GstMappedFrame):
(WebCore::GstMappedFrame::get):
(WebCore::GstMappedFrame::ComponentData const):
(WebCore::GstMappedFrame::ComponentStride const):
(WebCore::GstMappedFrame::info):
(WebCore::GstMappedFrame::width const):
(WebCore::GstMappedFrame::height const):
(WebCore::GstMappedFrame::format const):
(WebCore::GstMappedFrame::planeData const):
(WebCore::GstMappedFrame::planeStride const):
(WebCore::GstMappedFrame::isValid const):
(WebCore::GstMappedFrame::operator! const):
* Source/WebCore/platform/graphics/gstreamer/VideoFrameGStreamer.cpp:
(WebCore::VideoFrame::copyTo):
* Source/WebCore/platform/mediastream/libwebrtc/gstreamer/GStreamerVideoFrameLibWebRTC.cpp:
(WebCore::GStreamerVideoFrameLibWebRTC::ToI420):

Canonical link: https://commits.webkit.org/275320@main
  • Loading branch information
calvaris committed Feb 26, 2024
1 parent 0d63879 commit 8ce9ffa
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 22 deletions.
39 changes: 18 additions & 21 deletions Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.h
Expand Up @@ -229,73 +229,68 @@ class GstMappedFrame {
m_isValid = gst_video_frame_map(&m_frame, info, buffer, flags);
}

GstMappedFrame(GRefPtr<GstSample> sample, GstMapFlags flags)
GstMappedFrame(const GRefPtr<GstSample>& sample, GstMapFlags flags)
{
GstVideoInfo info;

if (!gst_video_info_from_caps(&info, gst_sample_get_caps(sample.get()))) {
m_isValid = false;
if (!gst_video_info_from_caps(&info, gst_sample_get_caps(sample.get())))
return;
}

m_isValid = gst_video_frame_map(&m_frame, &info, gst_sample_get_buffer(sample.get()), flags);
}

GstVideoFrame* get()
{
if (!m_isValid) {
GST_INFO("Invalid frame, returning NULL");

return nullptr;
}

RELEASE_ASSERT(m_isValid);
return &m_frame;
}

uint8_t* ComponentData(int comp) const
{
RELEASE_ASSERT(m_isValid);
return GST_VIDEO_FRAME_COMP_DATA(&m_frame, comp);
}

int ComponentStride(int stride) const
{
RELEASE_ASSERT(m_isValid);
return GST_VIDEO_FRAME_COMP_STRIDE(&m_frame, stride);
}

GstVideoInfo* info()
{
if (!m_isValid) {
GST_INFO("Invalid frame, returning NULL");

return nullptr;
}

RELEASE_ASSERT(m_isValid);
return &m_frame.info;
}

int width() const
{
return m_isValid ? GST_VIDEO_FRAME_WIDTH(&m_frame) : -1;
RELEASE_ASSERT(m_isValid);
return GST_VIDEO_FRAME_WIDTH(&m_frame);
}

int height() const
{
return m_isValid ? GST_VIDEO_FRAME_HEIGHT(&m_frame) : -1;
RELEASE_ASSERT(m_isValid);
return GST_VIDEO_FRAME_HEIGHT(&m_frame);
}

int format() const
{
return m_isValid ? GST_VIDEO_FRAME_FORMAT(&m_frame) : GST_VIDEO_FORMAT_UNKNOWN;
RELEASE_ASSERT(m_isValid);
return GST_VIDEO_FRAME_FORMAT(&m_frame);
}

void* planeData(uint32_t planeIndex) const
{
return m_isValid ? GST_VIDEO_FRAME_PLANE_DATA(&m_frame, planeIndex) : nullptr;
RELEASE_ASSERT(m_isValid);
return GST_VIDEO_FRAME_PLANE_DATA(&m_frame, planeIndex);
}

int planeStride(uint32_t planeIndex) const
{
return m_isValid ? GST_VIDEO_FRAME_PLANE_STRIDE(&m_frame, planeIndex) : -1;
RELEASE_ASSERT(m_isValid);
return GST_VIDEO_FRAME_PLANE_STRIDE(&m_frame, planeIndex);
}

~GstMappedFrame()
Expand All @@ -305,7 +300,9 @@ class GstMappedFrame {
m_isValid = false;
}

bool isValid() const { return m_isValid; }
explicit operator bool() const { return m_isValid; }
bool operator!() const { return !m_isValid; }

private:
GstVideoFrame m_frame;
Expand Down
Expand Up @@ -368,6 +368,12 @@ void VideoFrame::copyTo(std::span<uint8_t> destination, VideoPixelFormat pixelFo
auto* inputCaps = gst_sample_get_caps(sample);
gst_video_info_from_caps(&inputInfo, inputCaps);
GstMappedFrame inputFrame(inputBuffer, &inputInfo, GST_MAP_READ);
if (!inputFrame) {
GST_WARNING("could not map the input frame");
ASSERT_NOT_REACHED_WITH_MESSAGE("could not map the input frame");
callback({ });
return;
}

GST_TRACE("Copying frame data to pixel format %d", static_cast<int>(pixelFormat));
if (pixelFormat == VideoPixelFormat::NV12) {
Expand Down
Expand Up @@ -76,7 +76,8 @@ rtc::scoped_refptr<webrtc::I420BufferInterface> GStreamerVideoFrameLibWebRTC::To
{
GstMappedFrame inFrame(m_sample, GST_MAP_READ);
if (!inFrame) {
GST_WARNING("Could not map frame");
GST_WARNING("Could not map input frame");
ASSERT_NOT_REACHED_WITH_MESSAGE("Could not map input frame");
return nullptr;
}

Expand All @@ -90,6 +91,11 @@ rtc::scoped_refptr<webrtc::I420BufferInterface> GStreamerVideoFrameLibWebRTC::To

auto buffer = adoptGRef(gst_buffer_new_allocate(nullptr, GST_VIDEO_INFO_SIZE(&outInfo), nullptr));
GstMappedFrame outFrame(buffer.get(), &outInfo, GST_MAP_WRITE);
if (!outFrame) {
GST_WARNING("Could not map output frame");
ASSERT_NOT_REACHED_WITH_MESSAGE("Could not map output frame");
return nullptr;
}
GUniquePtr<GstVideoConverter> videoConverter(gst_video_converter_new(inFrame.info(), &outInfo, gst_structure_new("GstVideoConvertConfig",
GST_VIDEO_CONVERTER_OPT_THREADS, G_TYPE_UINT, std::max(std::thread::hardware_concurrency(), 1u), nullptr)));

Expand Down

0 comments on commit 8ce9ffa

Please sign in to comment.