Skip to content

Commit

Permalink
Race condition in LibWebRTCVPXInternalVideoDecoder::pixelBufferPool l…
Browse files Browse the repository at this point in the history 
…eading to memory corruption

rdar://125957410

Reviewed by Chris Dumez.

Add a lock to make sure creation of the pixel buffer happens correctly.

* LayoutTests/http/wpt/webcodecs/copyTo-same-decoder-expected.txt: Added.
* LayoutTests/http/wpt/webcodecs/copyTo-same-decoder.html: Added.
* Source/WebCore/platform/libwebrtc/LibWebRTCVPXVideoDecoder.cpp:
(WebCore::LibWebRTCVPXInternalVideoDecoder::createPixelBuffer):
(WebCore::LibWebRTCVPXInternalVideoDecoder::Decoded):

Canonical link: https://commits.webkit.org/272448.909@safari-7618-branch
  • Loading branch information
@youennf @cdumez
youennf authored and cdumez committed Apr 12, 2024
1 parent 9a9141f commit b50f199
Show file tree
Hide file tree
Showing 3 changed files with 160 additions and 32 deletions.
3 changes: 3 additions & 0 deletions LayoutTests/http/wpt/webcodecs/copyTo-same-decoder-expected.txt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@

PASS Test copyTo on different contexts

114 changes: 114 additions & 0 deletions LayoutTests/http/wpt/webcodecs/copyTo-same-decoder.html
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
<!DOCTYPE html>
<html>
<header>
<script src='/resources/testharness.js'></script>
<script src='/resources/testharnessreport.js'></script>
</header>
<body>
<script>
const WORKER_CODE = `self.onmessage = event => {
const frames = event.data;
frames.a.copyTo(new ArrayBuffer(1024 * 1024 * 4));
frames.b.copyTo(new ArrayBuffer(1024 * 1024 * 4));
frames.c.copyTo(new ArrayBuffer(1024 * 1024 * 4));
frames.d.copyTo(new ArrayBuffer(1024 * 1024 * 4));
frames.a.close();
frames.b.close();
frames.d.close();
frames.d.close();
};
self.postMessage('ready');
`

promise_test(async () => {
const encodedFrames = [];
let resolve, reject;
const encoderPromise = new Promise((res,rej) => {
resolve = res;
reject = rej;
});

for (const size of [2, 1024]) {
const encoder = new VideoEncoder({
output: chunk => {
encodedFrames.push(chunk);
if (encodedFrames.length === 2)
resolve();
},
error: e => reject(e),
});
setTimeout(() => {
reject("timed out waiting for encoded chunks");
}, 5000);
encoder.configure({
codec: 'vp8',
width: size,
height: size,
bitrate: 10e6,
framerate: 1,
});

const frame = new VideoFrame(new ArrayBuffer(size * size * 4), {format: 'RGBA', codedWidth: size, codedHeight: size, timestamp: 0});
encoder.encode(frame, {keyFrame: false});
frame.close();
}
await encoderPromise;

const decoderPromise = new Promise((res,rej) => {
resolve = res;
reject = rej;
});

const frames = [];
const decoder = new VideoDecoder({
output: frame => {
frames.push(frame);
if (frames.length === 8)
resolve();
},
error: e => reject(e),
});
setTimeout(() => {
reject("timed out waiting for decoded frames");
}, 5000);

decoder.configure({
codec: 'vp8',
codedWidth: 16,
codedHeight: 16,
});

for (let i = 0; i < 4; ++i) {
decoder.decode(encodedFrames[0]);
decoder.decode(encodedFrames[1]);
}

await decoderPromise;

const worker = new Worker(URL.createObjectURL(new Blob([WORKER_CODE])));
await new Promise(resolve => worker.onmessage = resolve);

worker.postMessage({a: frames[0], b: frames[1], c: frames[2], d: frames[3]}, [frames[0], frames[1], frames[2], frames[3]]);

frames[4].copyTo(new ArrayBuffer(1024 * 1024 * 4));
await new Promise(resolve => setTimeout(resolve, 0));

frames[5].copyTo(new ArrayBuffer(1024 * 1024 * 4));
await new Promise(resolve => setTimeout(resolve, 0));

frames[6].copyTo(new ArrayBuffer(1024 * 1024 * 4));
await new Promise(resolve => setTimeout(resolve, 0));

frames[7].copyTo(new ArrayBuffer(1024 * 1024 * 4));
await new Promise(resolve => setTimeout(resolve, 0));

frames[4].close();
frames[5].close();
frames[6].close();
frames[7].close();
}, "Test copyTo on different contexts");
</script>
</body>
</html>
75 changes: 43 additions & 32 deletions Source/WebCore/platform/libwebrtc/LibWebRTCVPXVideoDecoder.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,8 @@ class LibWebRTCVPXInternalVideoDecoder : public ThreadSafeRefCounted<LibWebRTCVP
private:
LibWebRTCVPXInternalVideoDecoder(LibWebRTCVPXVideoDecoder::Type, const VideoDecoder::Config&, VideoDecoder::OutputCallback&&, VideoDecoder::PostTaskCallback&&);
int32_t Decoded(webrtc::VideoFrame&) final;
CVPixelBufferPoolRef pixelBufferPool(size_t width, size_t height, OSType);
CVPixelBufferPoolRef pixelBufferPool(size_t width, size_t height, OSType) WTF_REQUIRES_LOCK(m_pixelBufferPoolLock);
CVPixelBufferRef createPixelBuffer(size_t width, size_t height, webrtc::BufferType);

VideoDecoder::OutputCallback m_outputCallback;
VideoDecoder::PostTaskCallback m_postTaskCallback;
Expand All @@ -81,7 +82,8 @@ class LibWebRTCVPXInternalVideoDecoder : public ThreadSafeRefCounted<LibWebRTCVP
bool m_isClosed { false };
bool m_useIOSurface { false };
ProcessIdentity m_resourceOwner;
RetainPtr<CVPixelBufferPoolRef> m_pixelBufferPool;
RetainPtr<CVPixelBufferPoolRef> m_pixelBufferPool WTF_GUARDED_BY_LOCK(m_pixelBufferPoolLock);
Lock m_pixelBufferPoolLock;
size_t m_pixelBufferPoolWidth { 0 };
size_t m_pixelBufferPoolHeight { 0 };
OSType m_pixelBufferPoolType;
Expand Down Expand Up @@ -198,42 +200,51 @@ CVPixelBufferPoolRef LibWebRTCVPXInternalVideoDecoder::pixelBufferPool(size_t wi
return m_pixelBufferPool.get();
}

CVPixelBufferRef LibWebRTCVPXInternalVideoDecoder::createPixelBuffer(size_t width, size_t height, webrtc::BufferType bufferType)
{
OSType pixelBufferType;
switch (bufferType) {
case webrtc::BufferType::I420:
pixelBufferType = kCVPixelFormatType_420YpCbCr8BiPlanarVideoRange;
break;
case webrtc::BufferType::I010:
pixelBufferType = kCVPixelFormatType_420YpCbCr10BiPlanarVideoRange;
break;
default:
return nullptr;
}

CVPixelBufferRef pixelBuffer = nullptr;
CVReturn status = kCVReturnError;

{
Locker locker(m_pixelBufferPoolLock);
if (auto bufferPool = pixelBufferPool(width, height, pixelBufferType))
status = CVPixelBufferPoolCreatePixelBuffer(nullptr, bufferPool, &pixelBuffer);
}

if (status != kCVReturnSuccess || !pixelBuffer) {
RELEASE_LOG_ERROR(Media, "Failed creating a pixel buffer for converting a VPX frame with error %d", status);
return nullptr;
}

if (m_resourceOwner) {
if (auto surface = CVPixelBufferGetIOSurface(pixelBuffer))
IOSurface::setOwnershipIdentity(surface, m_resourceOwner);
}

return pixelBuffer;
}

int32_t LibWebRTCVPXInternalVideoDecoder::Decoded(webrtc::VideoFrame& frame)
{
m_postTaskCallback([protectedThis = Ref { *this }, colorSpace = VideoFrameLibWebRTC::colorSpaceFromFrame(frame), buffer = frame.video_frame_buffer(), timestamp = m_timestamp, duration = m_duration]() mutable {
if (protectedThis->m_isClosed)
return;

auto videoFrame = VideoFrameLibWebRTC::create({ }, false, VideoFrame::Rotation::None, WTFMove(colorSpace), WTFMove(buffer), [protectedThis](auto& buffer) {
return adoptCF(webrtc::createPixelBufferFromFrameBuffer(buffer, [protectedThis](size_t width, size_t height, webrtc::BufferType bufferType) -> CVPixelBufferRef {
OSType pixelBufferType;
switch (bufferType) {
case webrtc::BufferType::I420:
pixelBufferType = kCVPixelFormatType_420YpCbCr8BiPlanarVideoRange;
break;
case webrtc::BufferType::I010:
pixelBufferType = kCVPixelFormatType_420YpCbCr10BiPlanarVideoRange;
break;
default:
return nullptr;
}

CVPixelBufferRef pixelBuffer = nullptr;
CVReturn status = kCVReturnError;
if (auto bufferPool = protectedThis->pixelBufferPool(width, height, pixelBufferType))
status = CVPixelBufferPoolCreatePixelBuffer(nullptr, bufferPool, &pixelBuffer);

if (status != kCVReturnSuccess || !pixelBuffer) {
RELEASE_LOG_ERROR(Media, "Failed creating a pixel buffer for converting a VPX frame with error %d", status);
return nullptr;
}

if (protectedThis->m_resourceOwner) {
if (auto surface = CVPixelBufferGetIOSurface(pixelBuffer))
IOSurface::setOwnershipIdentity(surface, protectedThis->m_resourceOwner);
}

return pixelBuffer;
auto videoFrame = VideoFrameLibWebRTC::create({ }, false, VideoFrame::Rotation::None, WTFMove(colorSpace), WTFMove(buffer), [protectedThis] (auto& buffer) {
return adoptCF(webrtc::createPixelBufferFromFrameBuffer(buffer, [protectedThis] (size_t width, size_t height, webrtc::BufferType bufferType) -> CVPixelBufferRef {
return protectedThis->createPixelBuffer(width, height, bufferType);
}));
});

Expand Down

0 comments on commit b50f199

Please sign in to comment.