Skip to content

Commit 85b827d

Browse files
committed
VideoFrame constructor is not handling video range properly for NV12
rdar://170299037 https://bugs.webkit.org/show_bug.cgi?id=307766 Reviewed by Eric Carlson. Creating BT601 video frame from JS was not handling fullrange/videorange. We fix this in VideoFrame::createNV12 and VideoFrame::createI420. Covered by added test. This test is adding an extra tolerance for some values, due to differences on how BT601 is converted to RGB with VT. Canonical link: https://commits.webkit.org/307649@main
1 parent 2f996ea commit 85b827d

File tree

6 files changed

+97
-6
lines changed

6 files changed

+97
-6
lines changed
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
2+
PASS Test SMTPE170m video frame rendering, full range=true
3+
PASS Test SMTPE170m video frame rendering, full range=false
4+
Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
<!DOCTYPE html>
2+
<script src="/resources/testharness.js"></script>
3+
<script src="/resources/testharnessreport.js"></script>
4+
<script src="resources/videoframe-utilities.js"></script>
5+
<style>
6+
body { margin: 0 }
7+
canvas { image-rendering: pixelated }
8+
</style>
9+
<canvas style="display: none" id="referenceCanvas"></canvas>
10+
<canvas style="display: none" id="resultCanvas"></canvas>
11+
<script>
12+
13+
function getRGBImageData(fullRange, frameFormat, frameData)
14+
{
15+
const init = {
16+
format: frameFormat,
17+
timestamp: 1234,
18+
codedWidth: 2,
19+
codedHeight: 2,
20+
colorSpace: { "primaries":"smpte170m", "transfer":"smpte170m", "matrix":"smpte170m", fullRange }
21+
};
22+
23+
frame = new VideoFrame(frameData, init);
24+
referenceCanvas.getContext('2d').drawImage(frame, 0, 0);
25+
frame.close();
26+
return referenceCanvas.getContext('2d').getImageData(0, 0, 1, 1).data;
27+
}
28+
29+
const testData = [
30+
{
31+
format: "NV12",
32+
fullRange: new Uint8Array([93, 93, 93, 93, 159, 101 ]),
33+
videoRange: new Uint8Array([94, 94, 94, 94, 157, 102 ]),
34+
rgba: [50, 100, 150, 255],
35+
threshold: 20
36+
},
37+
{
38+
format: "NV12",
39+
fullRange: new Uint8Array([255, 255, 255, 255, 128, 128 ]),
40+
videoRange: new Uint8Array([235, 235, 235, 235, 128, 128 ]),
41+
rgba: [255, 255, 255, 255],
42+
threshold: 2
43+
},
44+
{
45+
format: "NV12",
46+
fullRange: new Uint8Array([0, 0, 0, 0, 128, 128 ]),
47+
videoRange: new Uint8Array([16, 16, 16, 16, 128, 128 ]),
48+
rgba: [0, 0, 0, 255],
49+
threshold: 2
50+
},
51+
{
52+
format: "I420",
53+
fullRange: new Uint8Array([93, 93, 93, 93, 159, 101 ]),
54+
videoRange: new Uint8Array([94, 94, 94, 94, 157, 102 ]),
55+
rgba: [50, 100, 150, 255],
56+
threshold: 20
57+
},
58+
{
59+
format: "I420",
60+
fullRange: new Uint8Array([255, 255, 255, 255, 128, 128 ]),
61+
videoRange: new Uint8Array([235, 235, 235, 235, 128, 128 ]),
62+
rgba: [255, 255, 255, 255],
63+
threshold: 2
64+
},
65+
{
66+
format: "I420",
67+
fullRange: new Uint8Array([0, 0, 0, 0, 128, 128 ]),
68+
videoRange: new Uint8Array([16, 16, 16, 16, 128, 128 ]),
69+
rgba: [0, 0, 0, 255],
70+
threshold: 2
71+
},
72+
];
73+
74+
async function checkSMPTE170Frame(fullRange)
75+
{
76+
test(t => {
77+
testData.forEach((item, index) => {
78+
const result = getRGBImageData(fullRange, item.format, fullRange ? item.fullRange : item.videoRange);
79+
assert_array_approx_equals(result, item.rgba, item.threshold, "test " + index);
80+
});
81+
}, `Test SMTPE170m video frame rendering, full range=${fullRange}`);
82+
}
83+
84+
checkSMPTE170Frame(true);
85+
checkSMPTE170Frame(false);
86+
</script>
87+
</html>

Source/ThirdParty/libwebrtc/Configurations/libwebrtc.exp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ __ZN6webrtc20copyVideoFrameBufferERNS_16VideoFrameBufferEPh
202202
__ZN6webrtc32createPixelBufferFromFrameBufferERNS_16VideoFrameBufferERKNSt3__18functionIFP10__CVBuffermmNS_10BufferTypeEEEE
203203
__ZN6webrtc25CreateTaskQueueGcdFactoryEv
204204
__ZN6webrtc16convertBGRAToYUVEP10__CVBufferS1_
205-
__ZN6webrtc31createPixelBufferFromI420BufferEPKhmmmNS_16I420BufferLayoutE
205+
__ZN6webrtc31createPixelBufferFromI420BufferEPKhmmmNS_16I420BufferLayoutEb
206206
__ZN6webrtc12EncodedImageC1Ev
207207
__ZN6webrtc12EncodedImageD1Ev
208208
__ZN6webrtc12VideoDecoder8Settings19set_number_of_coresEi

Source/ThirdParty/libwebrtc/Source/webrtc/webkit_sdk/WebKit/WebKitUtilities.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ struct I420ABufferLayout : I420BufferLayout {
7070
size_t strideA { 0 };
7171
};
7272

73-
CVPixelBufferRef createPixelBufferFromI420Buffer(const uint8_t* buffer, size_t length, size_t width, size_t height, I420BufferLayout) CF_RETURNS_RETAINED;
73+
CVPixelBufferRef createPixelBufferFromI420Buffer(const uint8_t* buffer, size_t length, size_t width, size_t height, I420BufferLayout, bool isFullRange) CF_RETURNS_RETAINED;
7474
CVPixelBufferRef createPixelBufferFromI420ABuffer(const uint8_t* buffer, size_t length, size_t width, size_t height, I420ABufferLayout) CF_RETURNS_RETAINED;
7575

7676
}

Source/ThirdParty/libwebrtc/Source/webrtc/webkit_sdk/WebKit/WebKitUtilities.mm

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,11 +116,11 @@ static bool copyI420BufferToPixelBuffer(CVPixelBufferRef pixelBuffer, const uint
116116

117117
}
118118

119-
CVPixelBufferRef createPixelBufferFromI420Buffer(const uint8_t* buffer, size_t length, size_t width, size_t height, I420BufferLayout layout)
119+
CVPixelBufferRef createPixelBufferFromI420Buffer(const uint8_t* buffer, size_t length, size_t width, size_t height, I420BufferLayout layout, bool isFullRange)
120120
{
121121
CVPixelBufferRef pixelBuffer = nullptr;
122122

123-
auto status = CVPixelBufferCreate(kCFAllocatorDefault, width, height, kCVPixelFormatType_420YpCbCr8BiPlanarFullRange, nullptr, &pixelBuffer);
123+
auto status = CVPixelBufferCreate(kCFAllocatorDefault, width, height, isFullRange ? kCVPixelFormatType_420YpCbCr8BiPlanarFullRange : kCVPixelFormatType_420YpCbCr8BiPlanarVideoRange, nullptr, &pixelBuffer);
124124
if (status != noErr || !pixelBuffer)
125125
return nullptr;
126126

Source/WebCore/platform/graphics/cv/VideoFrameCV.mm

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ static vImage_Buffer makeVImageBuffer8888(CVPixelBufferRef buffer)
108108
{
109109
CVPixelBufferRef rawPixelBuffer = nullptr;
110110

111-
auto status = CVPixelBufferCreate(kCFAllocatorDefault, width, height, kCVPixelFormatType_420YpCbCr8BiPlanarFullRange, nullptr, &rawPixelBuffer);
111+
auto status = CVPixelBufferCreate(kCFAllocatorDefault, width, height, colorSpace.fullRange.value_or(false) ? kCVPixelFormatType_420YpCbCr8BiPlanarFullRange : kCVPixelFormatType_420YpCbCr8BiPlanarVideoRange, nullptr, &rawPixelBuffer);
112112
if (status != noErr || !rawPixelBuffer)
113113
return nullptr;
114114
RetainPtr pixelBuffer = adoptCF(rawPixelBuffer);
@@ -203,7 +203,7 @@ static vImage_Buffer makeVImageBuffer8888(CVPixelBufferRef buffer)
203203
offsetLayoutU, layoutU.sourceWidthBytes,
204204
offsetLayoutV, layoutV.sourceWidthBytes
205205
};
206-
auto pixelBuffer = adoptCF(webrtc::createPixelBufferFromI420Buffer(buffer.data(), buffer.size(), width, height, layout));
206+
auto pixelBuffer = adoptCF(webrtc::createPixelBufferFromI420Buffer(buffer.data(), buffer.size(), width, height, layout, colorSpace.fullRange.value_or(false)));
207207

208208
if (!pixelBuffer)
209209
return nullptr;

0 commit comments

Comments
 (0)