Skip to content
This repository was archived by the owner on Mar 22, 2022. It is now read-only.

Commit 46ec05d

Browse files
jehumb-havokjehumb-havok
committed
Fix bug in I420AVideoFrame.CopyTo() with stride
Ensure that `I420AVideoFrame.CopyTo()` is using the correct stride for the source, but is packing the data (stride == width) in the destination buffer, as described in the function documentation.
1 parent 3f3641f commit 46ec05d

3 files changed

Lines changed: 186 additions & 10 deletions

File tree

‎libs/Microsoft.MixedReality.WebRTC.Native/test/Microsoft.MixedReality.WebRTC.Native.Tests.vcxproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@
6161
</ItemGroup>
6262
<ItemGroup>
6363
<ClCompile Include="audio_track_tests.cpp" />
64+
<ClCompile Include="memory_tests.cpp" />
6465
<ClCompile Include="peer_connection_tests.cpp" />
6566
<ClCompile Include="sdp_utils_tests.cpp" />
6667
<ClCompile Include="pch.cpp">

‎libs/Microsoft.MixedReality.WebRTC.Native/test/memory_tests.cpp‎

Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,172 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License. See LICENSE in the project root for license
3+
// information.
4+
5+
#include "pch.h"
6+
7+
#include "interop/interop_api.h"
8+
9+
// Test fast path of mrsMemCpyStride() when data is packed.
10+
TEST(MemoryUtils, MemCpyStride_Fast) {
11+
std::vector<uint8_t> s, d;
12+
constexpr int kWidth = 32;
13+
constexpr int kStride = kWidth;
14+
constexpr int kHeight = 13;
15+
s.resize(kStride * kHeight);
16+
d.resize(kStride * kHeight);
17+
{
18+
uint8_t* src = s.data();
19+
for (int j = 0; j < kHeight; ++j) {
20+
for (int i = 0; i < kWidth; ++i) {
21+
*src++ = (rand() & 0xFF);
22+
}
23+
}
24+
}
25+
{
26+
const void* const src = s.data();
27+
void* const dst = d.data();
28+
mrsMemCpyStride(dst, kStride, src, kStride, kWidth, kHeight);
29+
// Data is contiguous
30+
ASSERT_EQ(0, memcmp(src, dst, kStride * kHeight));
31+
}
32+
}
33+
34+
// Test slow path of mrsMemCpyStride() with stride, without changing the
35+
// packing.
36+
TEST(MemoryUtils, MemCpyStride_Stride) {
37+
std::vector<uint8_t> s, d;
38+
constexpr int kWidth = 29;
39+
constexpr int kStride = 32;
40+
constexpr int kHeight = 13;
41+
s.resize(kStride * kHeight);
42+
d.resize(kStride * kHeight);
43+
{
44+
uint8_t* src = s.data();
45+
for (int j = 0; j < kHeight; ++j) {
46+
for (int i = 0; i < kWidth; ++i) {
47+
*src++ = (rand() & 0xFF);
48+
}
49+
for (int i = kWidth; i < kStride; ++i) {
50+
*src++ = 0xCF;
51+
}
52+
}
53+
}
54+
{
55+
const void* const src = s.data();
56+
void* const dst = d.data();
57+
mrsMemCpyStride(dst, kStride, src, kStride, kWidth, kHeight);
58+
}
59+
{
60+
const uint8_t* src = s.data();
61+
const uint8_t* dst = d.data();
62+
for (int j = 0; j < kHeight; ++j) {
63+
// Test row
64+
bool row_equal = true;
65+
for (int i = 0; i < kWidth; ++i) {
66+
row_equal = row_equal && (*src == *dst);
67+
++src;
68+
++dst;
69+
}
70+
ASSERT_TRUE(row_equal);
71+
// Skip row padding
72+
for (int i = kWidth; i < kStride; ++i) {
73+
++src;
74+
++dst;
75+
}
76+
}
77+
}
78+
}
79+
80+
// Test slow path of mrsMemCpyStride() with stride, expanding the one existing
81+
// in the source buffer.
82+
TEST(MemoryUtils, MemCpyStride_ExpandStride) {
83+
std::vector<uint8_t> s, d;
84+
constexpr int kWidth = 29;
85+
constexpr int kSrcStride = 32;
86+
constexpr int kDstStride = 48;
87+
constexpr int kHeight = 13;
88+
s.resize(kSrcStride * kHeight);
89+
d.resize(kDstStride * kHeight);
90+
{
91+
uint8_t* src = s.data();
92+
for (int j = 0; j < kHeight; ++j) {
93+
for (int i = 0; i < kWidth; ++i) {
94+
*src++ = (rand() & 0xFF);
95+
}
96+
for (int i = kWidth; i < kSrcStride; ++i) {
97+
*src++ = 0xCF;
98+
}
99+
}
100+
}
101+
{
102+
const void* const src = s.data();
103+
void* const dst = d.data();
104+
mrsMemCpyStride(dst, kDstStride, src, kSrcStride, kWidth, kHeight);
105+
}
106+
{
107+
const uint8_t* src = s.data();
108+
const uint8_t* dst = d.data();
109+
for (int j = 0; j < kHeight; ++j) {
110+
// Test row
111+
bool row_equal = true;
112+
for (int i = 0; i < kWidth; ++i) {
113+
row_equal = row_equal && (*src == *dst);
114+
++src;
115+
++dst;
116+
}
117+
ASSERT_TRUE(row_equal);
118+
// Skip row padding
119+
for (int i = kWidth; i < kSrcStride; ++i) {
120+
++src;
121+
}
122+
for (int i = kWidth; i < kDstStride; ++i) {
123+
++dst;
124+
}
125+
}
126+
}
127+
}
128+
129+
// Test slow path of mrsMemCpyStride() with stride, packing the data on output.
130+
TEST(MemoryUtils, MemCpyStride_StrideToPack) {
131+
std::vector<uint8_t> s, d;
132+
constexpr int kWidth = 29;
133+
constexpr int kSrcStride = 32;
134+
constexpr int kDstStride = kWidth;
135+
constexpr int kHeight = 13;
136+
s.resize(kSrcStride * kHeight);
137+
d.resize(kDstStride * kHeight);
138+
{
139+
uint8_t* src = s.data();
140+
for (int j = 0; j < kHeight; ++j) {
141+
for (int i = 0; i < kWidth; ++i) {
142+
*src++ = (rand() & 0xFF);
143+
}
144+
for (int i = kWidth; i < kSrcStride; ++i) {
145+
*src++ = 0xCF;
146+
}
147+
}
148+
}
149+
{
150+
const void* const src = s.data();
151+
void* const dst = d.data();
152+
mrsMemCpyStride(dst, kDstStride, src, kSrcStride, kWidth, kHeight);
153+
}
154+
{
155+
const uint8_t* src = s.data();
156+
const uint8_t* dst = d.data();
157+
for (int j = 0; j < kHeight; ++j) {
158+
// Test row
159+
bool row_equal = true;
160+
for (int i = 0; i < kWidth; ++i) {
161+
row_equal = row_equal && (*src == *dst);
162+
++src;
163+
++dst;
164+
}
165+
ASSERT_TRUE(row_equal);
166+
// Skip row padding
167+
for (int i = kWidth; i < kSrcStride; ++i) {
168+
++src;
169+
}
170+
}
171+
}
172+
}

‎libs/Microsoft.MixedReality.WebRTC/VideoFrame.cs‎

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -78,22 +78,25 @@ public void CopyTo(byte[] buffer)
7878
{
7979
fixed (void* ptr = buffer)
8080
{
81+
// Destination buffer is packed and contiguous
82+
ulong dstSizeYA = (ulong)width * height;
83+
ulong dstSizeUV = dstSizeYA / 4;
84+
int dstStrideYA = (int)width;
85+
int dstStrideUV = dstStrideYA / 2;
86+
8187
// Note : System.Buffer.MemoryCopy() essentially does the same (without stride), but gets transpiled by IL2CPP
8288
// into the C++ corresponding to the IL instead of a single memcpy() call. This results in a large overhead,
8389
// especially in Debug config where one can lose 5-10 FPS just because of this.
8490
void* dst = ptr;
85-
ulong sizeY = (ulong)strideY * height;
86-
Utils.MemCpyStride(dst, strideY, (void*)dataY, strideY, (int)width, (int)height);
87-
dst = (void*)((ulong)dst + sizeY);
88-
ulong sizeU = (ulong)strideU * height / 2;
89-
Utils.MemCpyStride(dst, strideU, (void*)dataU, strideU, (int)width / 2, (int)height / 2);
90-
dst = (void*)((ulong)dst + sizeU);
91-
ulong sizeV = (ulong)strideV * height / 2;
92-
Utils.MemCpyStride(dst, strideV, (void*)dataV, strideV, (int)width / 2, (int)height / 2);
91+
Utils.MemCpyStride(dst, dstStrideYA, (void*)dataY, strideY, (int)width, (int)height);
92+
dst = (void*)((ulong)dst + dstSizeYA);
93+
Utils.MemCpyStride(dst, dstStrideUV, (void*)dataU, strideU, (int)width / 2, (int)height / 2);
94+
dst = (void*)((ulong)dst + dstSizeUV);
95+
Utils.MemCpyStride(dst, dstStrideUV, (void*)dataV, strideV, (int)width / 2, (int)height / 2);
9396
if (dataA.ToPointer() != null)
9497
{
95-
dst = (void*)((ulong)dst + sizeV);
96-
Utils.MemCpyStride(dst, strideA, (void*)dataA, strideA, (int)width, (int)height);
98+
dst = (void*)((ulong)dst + dstSizeUV);
99+
Utils.MemCpyStride(dst, dstStrideYA, (void*)dataA, strideA, (int)width, (int)height);
97100
}
98101
}
99102
}

0 commit comments

Comments
 (0)