Skip to content

Commit df03dc3

Browse files
calvariscalvaris
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 92ddde7 commit df03dc3

3 files changed

Lines changed: 57 additions & 29 deletions

File tree

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

Lines changed: 28 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -234,68 +234,73 @@ class GstMappedFrame {
234234
WTF_MAKE_NONCOPYABLE(GstMappedFrame);
235235
public:
236236

237-
GstMappedFrame(GstBuffer* buffer, GstVideoInfo info, GstMapFlags flags)
237+
GstMappedFrame(GstBuffer* buffer, GstVideoInfo* info, GstMapFlags flags)
238238
{
239-
m_isValid = gst_video_frame_map(&m_frame, &info, buffer, flags);
239+
m_isValid = gst_video_frame_map(&m_frame, info, buffer, flags);
240240
}
241241

242-
GstMappedFrame(GRefPtr<GstSample> sample, GstMapFlags flags)
242+
GstMappedFrame(const GRefPtr<GstSample>& sample, GstMapFlags flags)
243243
{
244244
GstVideoInfo info;
245245

246-
if (!gst_video_info_from_caps(&info, gst_sample_get_caps(sample.get()))) {
247-
m_isValid = false;
246+
if (!gst_video_info_from_caps(&info, gst_sample_get_caps(sample.get())))
248247
return;
249-
}
250248

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

254252
GstVideoFrame* get()
255253
{
256-
if (!m_isValid) {
257-
GST_INFO("Invalid frame, returning NULL");
258-
259-
return nullptr;
260-
}
261-
254+
RELEASE_ASSERT(m_isValid);
262255
return &m_frame;
263256
}
264257

265258
uint8_t* ComponentData(int comp)
266259
{
260+
RELEASE_ASSERT(m_isValid);
267261
return GST_VIDEO_FRAME_COMP_DATA(&m_frame, comp);
268262
}
269263

270264
int ComponentStride(int stride)
271265
{
266+
RELEASE_ASSERT(m_isValid);
272267
return GST_VIDEO_FRAME_COMP_STRIDE(&m_frame, stride);
273268
}
274269

275270
GstVideoInfo* info()
276271
{
277-
if (!m_isValid) {
278-
GST_INFO("Invalid frame, returning NULL");
279-
280-
return nullptr;
281-
}
282-
272+
RELEASE_ASSERT(m_isValid);
283273
return &m_frame.info;
284274
}
285275

286276
int width()
287277
{
288-
return m_isValid ? GST_VIDEO_FRAME_WIDTH(&m_frame) : -1;
278+
RELEASE_ASSERT(m_isValid);
279+
return GST_VIDEO_FRAME_WIDTH(&m_frame);
289280
}
290281

291282
int height()
292283
{
293-
return m_isValid ? GST_VIDEO_FRAME_HEIGHT(&m_frame) : -1;
284+
RELEASE_ASSERT(m_isValid);
285+
return GST_VIDEO_FRAME_HEIGHT(&m_frame);
294286
}
295287

296288
int format()
297289
{
298-
return m_isValid ? GST_VIDEO_FRAME_FORMAT(&m_frame) : GST_VIDEO_FORMAT_UNKNOWN;
290+
RELEASE_ASSERT(m_isValid);
291+
return GST_VIDEO_FRAME_FORMAT(&m_frame);
292+
}
293+
294+
void* planeData(uint32_t planeIndex) const
295+
{
296+
RELEASE_ASSERT(m_isValid);
297+
return GST_VIDEO_FRAME_PLANE_DATA(&m_frame, planeIndex);
298+
}
299+
300+
int planeStride(uint32_t planeIndex) const
301+
{
302+
RELEASE_ASSERT(m_isValid);
303+
return GST_VIDEO_FRAME_PLANE_STRIDE(&m_frame, planeIndex);
299304
}
300305

301306
~GstMappedFrame()
@@ -305,7 +310,9 @@ class GstMappedFrame {
305310
m_isValid = false;
306311
}
307312

313+
bool isValid() const { return m_isValid; }
308314
explicit operator bool() const { return m_isValid; }
315+
bool operator!() const { return !m_isValid; }
309316

310317
private:
311318
GstVideoFrame m_frame;

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

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,13 @@ Ref<VideoFrameGStreamer> VideoFrameGStreamer::createFromPixelBuffer(Ref<PixelBuf
8989
auto outputBuffer = adoptGRef(gst_buffer_new_allocate(nullptr, GST_VIDEO_INFO_SIZE(&outputInfo), nullptr));
9090
{
9191
GUniquePtr<GstVideoConverter> converter(gst_video_converter_new(&inputInfo, &outputInfo, nullptr));
92-
GstMappedFrame inputFrame(buffer.get(), inputInfo, GST_MAP_READ);
93-
GstMappedFrame outputFrame(outputBuffer.get(), outputInfo, GST_MAP_WRITE);
92+
GstMappedFrame inputFrame(buffer.get(), &inputInfo, GST_MAP_READ);
93+
GstMappedFrame outputFrame(outputBuffer.get(), &outputInfo, GST_MAP_WRITE);
94+
if (!inputFrame || !outputFrame) {
95+
GST_WARNING("frames could not be mapped");
96+
ASSERT_NOT_REACHED();
97+
return nullptr;
98+
}
9499
gst_video_converter_frame(converter.get(), inputFrame.get(), outputFrame.get());
95100
}
96101

@@ -154,8 +159,13 @@ GRefPtr<GstSample> VideoFrameGStreamer::resizedSample(const IntSize& destination
154159
auto outputBuffer = adoptGRef(gst_buffer_new_allocate(nullptr, GST_VIDEO_INFO_SIZE(&outputInfo), nullptr));
155160
{
156161
GUniquePtr<GstVideoConverter> converter(gst_video_converter_new(&inputInfo, &outputInfo, nullptr));
157-
GstMappedFrame inputFrame(buffer, inputInfo, GST_MAP_READ);
158-
GstMappedFrame outputFrame(outputBuffer.get(), outputInfo, GST_MAP_WRITE);
162+
GstMappedFrame inputFrame(buffer, &inputInfo, GST_MAP_READ);
163+
GstMappedFrame outputFrame(outputBuffer.get(), &outputInfo, GST_MAP_WRITE);
164+
if (!inputFrame || !outputFrame) {
165+
GST_WARNING("frames could not be mapped");
166+
ASSERT_NOT_REACHED();
167+
return nullptr;
168+
}
159169
gst_video_converter_frame(converter.get(), inputFrame.get(), outputFrame.get());
160170
}
161171

@@ -198,10 +208,15 @@ RefPtr<JSC::Uint8ClampedArray> VideoFrameGStreamer::computeRGBAImageData() const
198208
unsigned byteLength = GST_VIDEO_INFO_SIZE(&inputInfo);
199209
auto bufferStorage = JSC::ArrayBuffer::create(width * height, 4);
200210
auto outputBuffer = adoptGRef(gst_buffer_new_wrapped_full(GST_MEMORY_FLAG_NO_SHARE, bufferStorage->data(), byteLength, 0, byteLength, nullptr, [](gpointer) { }));
201-
GstMappedFrame outputFrame(outputBuffer.get(), outputInfo, GST_MAP_WRITE);
211+
GstMappedFrame outputFrame(outputBuffer.get(), &outputInfo, GST_MAP_WRITE);
202212

203213
GUniquePtr<GstVideoConverter> converter(gst_video_converter_new(&inputInfo, &outputInfo, nullptr));
204-
GstMappedFrame inputFrame(gst_sample_get_buffer(m_sample.get()), inputInfo, GST_MAP_READ);
214+
GstMappedFrame inputFrame(gst_sample_get_buffer(m_sample.get()), &inputInfo, GST_MAP_READ);
215+
if (!inputFrame || !outputFrame) {
216+
GST_WARNING("frames could not be mapped");
217+
ASSERT_NOT_REACHED();
218+
return nullptr;
219+
}
205220
gst_video_converter_frame(converter.get(), inputFrame.get(), outputFrame.get());
206221
return JSC::Uint8ClampedArray::tryCreate(WTFMove(bufferStorage), 0, byteLength);
207222
}

‎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)