Skip to content

Commit f8b1518

Browse files
authored
Refactor string handling and improve logging practices for VP8 (#1666)
Refactored string concatenation to use StringBuilder in DebugProbe.cs for better performance. Updated VP8Codec.cs to use structured logging instead of string interpolation. Performed minor code cleanup for clarity.
1 parent 33f8291 commit f8b1518

2 files changed

Lines changed: 27 additions & 19 deletions

File tree

src/SIPSorcery.VP8/DebugProbe.cs

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
//-----------------------------------------------------------------------------
2020

2121
using System.Diagnostics;
22+
using System.Text;
2223

2324
namespace Vpx.Net
2425
{
@@ -30,13 +31,14 @@ public static void DumpMotionVectors(MODE_INFO[] mip, int macroBlockCols, int ma
3031
Debug.WriteLine("Macro Block Modes:");
3132
for (int i = 0; i < macroBlockRows + 1; i++)
3233
{
33-
string rowStr = $"Row {i} | ";
34+
var rowBuilder = new StringBuilder().Append("Row ").Append(i).Append(" | ");
3435
for (int j = 0; j < macroBlockCols + 1; j++)
3536
{
3637
byte yMode = mip[i * (macroBlockRows + 1) + j].mbmi.mode;
3738
byte uvMode = mip[i * (macroBlockRows + 1) + j].mbmi.uv_mode;
38-
rowStr += $"y={yMode}, uvMode={uvMode} | ";
39+
rowBuilder.Append("y=").Append(yMode).Append(", uvMode=").Append(uvMode).Append(" | ");
3940
}
41+
string rowStr = rowBuilder.ToString();
4042
Debug.WriteLine(rowStr);
4143
}
4244

@@ -54,15 +56,20 @@ public static void DumpMotionVectors(MODE_INFO[] mip, int macroBlockCols, int ma
5456
public static string GetBModeInfoMatrix(b_mode_info[] bModes)
5557
{
5658
// The array will always be 16 elements.
57-
string matrixStr = null;
59+
var matrixBuilder = new StringBuilder();
5860

5961
for(int row=0; row<4; row++)
6062
{
61-
matrixStr += $"[{bModes[row * 4].mv.as_int},{bModes[row * 4 + 1].mv.as_int}" +
62-
$",{bModes[row * 4 +2].mv.as_int},{bModes[row * 4 + 3].mv.as_int}]\n";
63+
matrixBuilder
64+
.Append('[')
65+
.Append(bModes[row * 4].mv.as_int).Append(',')
66+
.Append(bModes[row * 4 + 1].mv.as_int).Append(',')
67+
.Append(bModes[row * 4 + 2].mv.as_int).Append(',')
68+
.Append(bModes[row * 4 + 3].mv.as_int)
69+
.Append("]\n");
6370
}
6471

65-
return matrixStr;
72+
return matrixBuilder.ToString();
6673
}
6774

6875
public static unsafe void DumpMacroBlock(MACROBLOCKD macroBlock, int macroBlockIndex)
@@ -89,11 +96,12 @@ public static unsafe void DumpSubBlockCoefficients(MACROBLOCKD macroBlock)
8996
for(int i=0; i< macroBlock.block.Length; i++)
9097
{
9198
var subBlock = macroBlock.block[i];
92-
string qCoeff = null;
99+
var qCoeffBuilder = new StringBuilder();
93100
for(int j=subBlock.qcoeff.Index; j< subBlock.qcoeff.Index+16; j++)
94101
{
95-
qCoeff += subBlock.qcoeff.src()[j].ToString() + ",";
102+
qCoeffBuilder.Append(subBlock.qcoeff.src()[j]).Append(',');
96103
}
104+
string qCoeff = qCoeffBuilder.ToString();
97105
Debug.WriteLine($"block[{i}].qcoeff={qCoeff}");
98106
}
99107

@@ -104,11 +112,12 @@ public static unsafe void DumpSubBlockCoefficients(MACROBLOCKD macroBlock)
104112
for (int i = 0; i < macroBlock.block.Length; i++)
105113
{
106114
var subBlock = macroBlock.block[i];
107-
string dqCoeff = null;
115+
var dqCoeffBuilder = new StringBuilder();
108116
for (int j = subBlock.dqcoeff.Index; j < subBlock.dqcoeff.Index + 16; j++)
109117
{
110-
dqCoeff += subBlock.dqcoeff.src()[j].ToString() + ",";
118+
dqCoeffBuilder.Append(subBlock.dqcoeff.src()[j]).Append(',');
111119
}
120+
string dqCoeff = dqCoeffBuilder.ToString();
112121
Debug.WriteLine($"block[{i}].dqcoeff={dqCoeff}");
113122
}
114123

@@ -160,21 +169,21 @@ public static class DebugProbeHexStr
160169

161170
public unsafe static string ToHexStr(byte* buffer, int length, char? separator = null)
162171
{
163-
string rv = string.Empty;
172+
var builder = new StringBuilder(length * (separator == null ? 2 : 3));
164173

165174
for (int i = 0; i < length; i++)
166175
{
167176
var val = buffer[i];
168-
rv += hexmap[val >> 4];
169-
rv += hexmap[val & 15];
177+
builder.Append(hexmap[val >> 4]);
178+
builder.Append(hexmap[val & 15]);
170179

171180
if (separator != null && i != length - 1)
172181
{
173-
rv += separator;
182+
builder.Append(separator);
174183
}
175184
}
176185

177-
return rv.ToLower();
186+
return builder.ToString().ToLower();
178187
}
179188
}
180189
}

src/SIPSorcery.VP8/VP8Codec.cs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ public byte[] EncodeVideo(int width, int height, byte[] sample, VideoPixelFormat
155155
// The foundation encoder requires multiples of 16 — no
156156
// padding/cropping support yet.
157157
throw new NotSupportedException(
158-
"Width and height must be positive multiples of 16. Got " + width + "x" + height + ".");
158+
$"Width and height must be positive multiples of 16. Got {width}x{height}.");
159159
}
160160

161161
// Convert the input sample into planar I420.
@@ -168,8 +168,7 @@ public byte[] EncodeVideo(int width, int height, byte[] sample, VideoPixelFormat
168168
if (i420.Length != ySize + 2 * cSize)
169169
{
170170
throw new ArgumentException(
171-
"I420 buffer length " + i420.Length + " does not match expected " +
172-
(ySize + 2 * cSize) + " for " + width + "x" + height + ".");
171+
$"I420 buffer length {i420.Length} does not match expected {ySize + 2 * cSize} for {width}x{height}.");
173172
}
174173

175174
if (_srcY == null || _srcY.Length < ySize) { _srcY = new byte[ySize]; }
@@ -228,7 +227,7 @@ public unsafe IEnumerable<VideoSample> DecodeVideo(byte[] frame, VideoPixelForma
228227
//logger.LogDebug($"VP8 decode result {result}.");
229228
if (result != vpx_codec_err_t.VPX_CODEC_OK)
230229
{
231-
logger.LogWarning($"VP8 decode of video sample failed with {result}.");
230+
logger.LogWarning("VP8 decode of video sample failed with {Result}.", result);
232231
}
233232
}
234233

0 commit comments

Comments
 (0)