Skip to content

Commit bc510e6

Browse files
committed
[GStreamer] Make GstMappedFrame assert when it is not initialized
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
1 parent cb82384 commit bc510e6

3 files changed

Lines changed: 43 additions & 24 deletions

File tree

Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.h

Lines changed: 28 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -223,68 +223,73 @@ class GstMappedFrame {
223223
WTF_MAKE_NONCOPYABLE(GstMappedFrame);
224224
public:
225225

226-
GstMappedFrame(GstBuffer* buffer, GstVideoInfo info, GstMapFlags flags)
226+
GstMappedFrame(GstBuffer* buffer, GstVideoInfo* info, GstMapFlags flags)
227227
{
228-
m_isValid = gst_video_frame_map(&m_frame, &info, buffer, flags);
228+
m_isValid = gst_video_frame_map(&m_frame, info, buffer, flags);
229229
}
230230

231-
GstMappedFrame(GRefPtr<GstSample> sample, GstMapFlags flags)
231+
GstMappedFrame(const GRefPtr<GstSample>& sample, GstMapFlags flags)
232232
{
233233
GstVideoInfo info;
234234

235-
if (!gst_video_info_from_caps(&info, gst_sample_get_caps(sample.get()))) {
236-
m_isValid = false;
235+
if (!gst_video_info_from_caps(&info, gst_sample_get_caps(sample.get())))
237236
return;
238-
}
239237

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

243241
GstVideoFrame* get()
244242
{
245-
if (!m_isValid) {
246-
GST_INFO("Invalid frame, returning NULL");
247-
248-
return nullptr;
249-
}
250-
243+
RELEASE_ASSERT(m_isValid);
251244
return &m_frame;
252245
}
253246

254247
uint8_t* ComponentData(int comp)
255248
{
249+
RELEASE_ASSERT(m_isValid);
256250
return GST_VIDEO_FRAME_COMP_DATA(&m_frame, comp);
257251
}
258252

259253
int ComponentStride(int stride)
260254
{
255+
RELEASE_ASSERT(m_isValid);
261256
return GST_VIDEO_FRAME_COMP_STRIDE(&m_frame, stride);
262257
}
263258

264259
GstVideoInfo* info()
265260
{
266-
if (!m_isValid) {
267-
GST_INFO("Invalid frame, returning NULL");
268-
269-
return nullptr;
270-
}
271-
261+
RELEASE_ASSERT(m_isValid);
272262
return &m_frame.info;
273263
}
274264

275265
int width()
276266
{
277-
return m_isValid ? GST_VIDEO_FRAME_WIDTH(&m_frame) : -1;
267+
RELEASE_ASSERT(m_isValid);
268+
return GST_VIDEO_FRAME_WIDTH(&m_frame);
278269
}
279270

280271
int height()
281272
{
282-
return m_isValid ? GST_VIDEO_FRAME_HEIGHT(&m_frame) : -1;
273+
RELEASE_ASSERT(m_isValid);
274+
return GST_VIDEO_FRAME_HEIGHT(&m_frame);
283275
}
284276

285277
int format()
286278
{
287-
return m_isValid ? GST_VIDEO_FRAME_FORMAT(&m_frame) : GST_VIDEO_FORMAT_UNKNOWN;
279+
RELEASE_ASSERT(m_isValid);
280+
return GST_VIDEO_FRAME_FORMAT(&m_frame);
281+
}
282+
283+
void* planeData(uint32_t planeIndex) const
284+
{
285+
RELEASE_ASSERT(m_isValid);
286+
return GST_VIDEO_FRAME_PLANE_DATA(&m_frame, planeIndex);
287+
}
288+
289+
int planeStride(uint32_t planeIndex) const
290+
{
291+
RELEASE_ASSERT(m_isValid);
292+
return GST_VIDEO_FRAME_PLANE_STRIDE(&m_frame, planeIndex);
288293
}
289294

290295
~GstMappedFrame()
@@ -294,7 +299,9 @@ class GstMappedFrame {
294299
m_isValid = false;
295300
}
296301

302+
bool isValid() const { return m_isValid; }
297303
explicit operator bool() const { return m_isValid; }
304+
bool operator!() const { return !m_isValid; }
298305

299306
private:
300307
GstVideoFrame m_frame;

Source/WebCore/platform/graphics/gstreamer/VideoFrameGStreamer.cpp

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -363,7 +363,13 @@ void VideoFrame::copyTo(std::span<uint8_t> destination, VideoPixelFormat pixelFo
363363
auto* inputBuffer = gst_sample_get_buffer(sample);
364364
auto* inputCaps = gst_sample_get_caps(sample);
365365
gst_video_info_from_caps(&inputInfo, inputCaps);
366-
GstMappedFrame inputFrame(inputBuffer, inputInfo, GST_MAP_READ);
366+
GstMappedFrame inputFrame(inputBuffer, &inputInfo, GST_MAP_READ);
367+
if (!inputFrame) {
368+
GST_WARNING("could not map the input frame");
369+
ASSERT_NOT_REACHED_WITH_MESSAGE("could not map the input frame");
370+
callback({ });
371+
return;
372+
}
367373

368374
GST_TRACE("Copying frame data to pixel format %d", static_cast<int>(pixelFormat));
369375
if (pixelFormat == VideoPixelFormat::NV12) {

Source/WebCore/platform/mediastream/libwebrtc/gstreamer/GStreamerVideoFrameLibWebRTC.cpp

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,8 @@ rtc::scoped_refptr<webrtc::I420BufferInterface> GStreamerVideoFrameLibWebRTC::To
7676
{
7777
GstMappedFrame inFrame(m_sample, GST_MAP_READ);
7878
if (!inFrame) {
79-
GST_WARNING("Could not map frame");
79+
GST_WARNING("Could not map input frame");
80+
ASSERT_NOT_REACHED_WITH_MESSAGE("Could not map input frame");
8081
return nullptr;
8182
}
8283

@@ -89,7 +90,12 @@ rtc::scoped_refptr<webrtc::I420BufferInterface> GStreamerVideoFrameLibWebRTC::To
8990
outInfo.fps_d = info->fps_d;
9091

9192
auto buffer = adoptGRef(gst_buffer_new_allocate(nullptr, GST_VIDEO_INFO_SIZE(&outInfo), nullptr));
92-
GstMappedFrame outFrame(buffer.get(), outInfo, GST_MAP_WRITE);
93+
GstMappedFrame outFrame(buffer.get(), &outInfo, GST_MAP_WRITE);
94+
if (!outFrame) {
95+
GST_WARNING("Could not map output frame");
96+
ASSERT_NOT_REACHED_WITH_MESSAGE("Could not map output frame");
97+
return nullptr;
98+
}
9399
GUniquePtr<GstVideoConverter> videoConverter(gst_video_converter_new(inFrame.info(), &outInfo, gst_structure_new("GstVideoConvertConfig",
94100
GST_VIDEO_CONVERTER_OPT_THREADS, G_TYPE_UINT, std::max(std::thread::hardware_concurrency(), 1u), nullptr)));
95101

0 commit comments

Comments
 (0)