Skip to content

Commit 0f8f02f

Browse files
paulomorgadopaulomorgado
committed
refactor: improve structured logging and string handling across net/media code
- Replace concatenated/interpolated log messages with structured templates in DTLS, ICE, FFmpeg, VP8, and Windows media code. - Add a Log.Debug(string messageTemplate, params object[] args) overload in SharpSRTP Log with placeholder formatting support. - Use safer string comparisons (StringComparison.OrdinalIgnoreCase) instead of ToLower()-based checks. - Reduce temporary string allocations in VP8 debug helpers by switching loop concatenation to StringBuilder. - Apply small string formatting cleanups (interpolation simplifications) and discard-pattern cleanup in SCTP switch guards. - Normalize file mode bits for test/FFmpegConsoleApp/Program.cs and test/unit/net/STUN/STUNUnitTest.cs.
1 parent e700f43 commit 0f8f02f

16 files changed

Lines changed: 117 additions & 114 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

‎src/SIPSorcery/net/DtlsSrtp/Lib/DTLS/DtlsClient.cs‎

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -171,13 +171,14 @@ public override void NotifyAlertRaised(short alertLevel, short alertDescription,
171171
{
172172
if (Log.DebugEnabled)
173173
{
174-
Log.Debug("DTLS client raised alert: " + AlertLevel.GetText(alertLevel) + ", " + AlertDescription.GetText(alertDescription));
174+
Log.Debug("DTLS client raised alert: {AlertLevel}, {AlertDescription}.",
175+
AlertLevel.GetText(alertLevel), AlertDescription.GetText(alertDescription));
175176
}
176177
if (message != null)
177178
{
178179
if (Log.DebugEnabled)
179180
{
180-
Log.Debug("> " + message);
181+
Log.Debug("> {Message}", message);
181182
}
182183
}
183184
if (cause != null)
@@ -193,7 +194,8 @@ public override void NotifyAlertReceived(short level, short alertDescription)
193194
{
194195
if (Log.DebugEnabled)
195196
{
196-
Log.Debug("DTLS client received alert: " + AlertLevel.GetText(level) + ", " + AlertDescription.GetText(alertDescription));
197+
Log.Debug("DTLS client received alert: {AlertLevel}, {AlertDescription}.",
198+
AlertLevel.GetText(level), AlertDescription.GetText(alertDescription));
197199
}
198200

199201
TlsAlertTypesEnum alertType = TlsAlertTypesEnum.Unassigned;
@@ -217,7 +219,7 @@ public override void NotifyServerVersion(ProtocolVersion serverVersion)
217219

218220
if (Log.DebugEnabled)
219221
{
220-
Log.Debug("DTLS client negotiated " + serverVersion);
222+
Log.Debug("DTLS client negotiated {ServerVersion}.", serverVersion);
221223
}
222224
}
223225

@@ -235,7 +237,7 @@ public override void NotifyHandshakeComplete()
235237
{
236238
if (Log.DebugEnabled)
237239
{
238-
Log.Debug("Client ALPN: " + protocolName.GetUtf8Decoding());
240+
Log.Debug("Client ALPN: {ApplicationProtocol}.", protocolName.GetUtf8Decoding());
239241
}
240242
}
241243

@@ -251,14 +253,14 @@ public override void NotifyHandshakeComplete()
251253
{
252254
if (Log.DebugEnabled)
253255
{
254-
Log.Debug("Client resumed session: " + hex);
256+
Log.Debug("Client resumed session: {SessionIdHex}.", hex);
255257
}
256258
}
257259
else
258260
{
259261
if (Log.DebugEnabled)
260262
{
261-
Log.Debug("Client established session: " + hex);
263+
Log.Debug("Client established session: {SessionIdHex}.", hex);
262264
}
263265
}
264266

@@ -270,14 +272,14 @@ public override void NotifyHandshakeComplete()
270272
{
271273
if (Log.DebugEnabled)
272274
{
273-
Log.Debug("Client 'tls-server-end-point': " + ToHexString(tlsServerEndPoint));
275+
Log.Debug("Client 'tls-server-end-point': {TlsServerEndPoint}.", ToHexString(tlsServerEndPoint));
274276
}
275277
}
276278

277279
byte[] tlsUnique = m_context.ExportChannelBinding(ChannelBinding.tls_unique);
278280
if (Log.DebugEnabled)
279281
{
280-
Log.Debug("Client 'tls-unique': " + ToHexString(tlsUnique));
282+
Log.Debug("Client 'tls-unique': {TlsUnique}.", ToHexString(tlsUnique));
281283
}
282284
}
283285

@@ -326,14 +328,15 @@ public void NotifyServerCertificate(TlsServerCertificate serverCertificate)
326328

327329
if (Log.DebugEnabled)
328330
{
329-
Log.Debug("DTLS client received server certificate chain of length " + chain.Length);
331+
Log.Debug("DTLS client received server certificate chain of length {CertificateCount}.", chain.Length);
330332
}
331333
for (int i = 0; i != chain.Length; i++)
332334
{
333335
X509CertificateStructure entry = X509CertificateStructure.GetInstance(chain[i].GetEncoded());
334336
if (Log.DebugEnabled)
335337
{
336-
Log.Debug("DTLS client fingerprint:SHA-256 " + DtlsCertificateUtils.Fingerprint(entry) + " (" + entry.Subject + ")");
338+
Log.Debug("DTLS client fingerprint:SHA-256 {Fingerprint} ({Subject}).",
339+
DtlsCertificateUtils.Fingerprint(entry), entry.Subject);
337340
}
338341
}
339342

‎src/SIPSorcery/net/DtlsSrtp/Lib/DTLS/DtlsServer.cs‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -164,14 +164,15 @@ public override void NotifyAlertRaised(short alertLevel, short alertDescription,
164164
{
165165
if (Log.DebugEnabled)
166166
{
167-
Log.Debug("DTLS server raised alert: " + AlertLevel.GetText(alertLevel) + ", " + AlertDescription.GetText(alertDescription));
167+
Log.Debug("DTLS server raised alert: {AlertLevel}, {AlertDescription}.",
168+
AlertLevel.GetText(alertLevel), AlertDescription.GetText(alertDescription));
168169
}
169170

170171
if (message != null)
171172
{
172173
if (Log.DebugEnabled)
173174
{
174-
Log.Debug("> " + message);
175+
Log.Debug("> {Message}", message);
175176
}
176177
}
177178
if (cause != null)
@@ -187,7 +188,8 @@ public override void NotifyAlertReceived(short level, short alertDescription)
187188
{
188189
if (Log.DebugEnabled)
189190
{
190-
Log.Debug("DTLS server received alert: " + AlertLevel.GetText(level) + ", " + AlertDescription.GetText(alertDescription));
191+
Log.Debug("DTLS server received alert: {AlertLevel}, {AlertDescription}.",
192+
AlertLevel.GetText(level), AlertDescription.GetText(alertDescription));
191193
}
192194

193195
TlsAlertTypesEnum alertType = TlsAlertTypesEnum.Unassigned;
@@ -210,7 +212,7 @@ public override ProtocolVersion GetServerVersion()
210212
ProtocolVersion serverVersion = base.GetServerVersion();
211213
if (Log.DebugEnabled)
212214
{
213-
Log.Debug("DTLS server negotiated " + serverVersion);
215+
Log.Debug("DTLS server negotiated {ServerVersion}.", serverVersion);
214216
}
215217
return serverVersion;
216218
}
@@ -239,15 +241,16 @@ public override void NotifyClientCertificate(Certificate clientCertificate)
239241

240242
if (Log.DebugEnabled)
241243
{
242-
Log.Debug("DTLS server received client certificate chain of length " + chain.Length);
244+
Log.Debug("DTLS server received client certificate chain of length {CertificateCount}.", chain.Length);
243245
}
244246

245247
for (int i = 0; i != chain.Length; i++)
246248
{
247249
X509CertificateStructure entry = X509CertificateStructure.GetInstance(chain[i].GetEncoded());
248250
if (Log.DebugEnabled)
249251
{
250-
Log.Debug(" fingerprint:SHA-256 " + DtlsCertificateUtils.Fingerprint(entry) + " (" + entry.Subject + ")");
252+
Log.Debug("fingerprint:SHA-256 {Fingerprint} ({Subject}).",
253+
DtlsCertificateUtils.Fingerprint(entry), entry.Subject);
251254
}
252255
}
253256
}
@@ -261,20 +264,20 @@ public override void NotifyHandshakeComplete()
261264
{
262265
if (Log.DebugEnabled)
263266
{
264-
Log.Debug("Server ALPN: " + protocolName.GetUtf8Decoding());
267+
Log.Debug("Server ALPN: {ApplicationProtocol}.", protocolName.GetUtf8Decoding());
265268
}
266269
}
267270

268271
byte[] tlsServerEndPoint = m_context.ExportChannelBinding(ChannelBinding.tls_server_end_point);
269272
if (Log.DebugEnabled)
270273
{
271-
Log.Debug("Server 'tls-server-end-point': " + ToHexString(tlsServerEndPoint));
274+
Log.Debug("Server 'tls-server-end-point': {TlsServerEndPoint}.", ToHexString(tlsServerEndPoint));
272275
}
273276

274277
byte[] tlsUnique = m_context.ExportChannelBinding(ChannelBinding.tls_unique);
275278
if (Log.DebugEnabled)
276279
{
277-
Log.Debug("Server 'tls-unique': " + ToHexString(tlsUnique));
280+
Log.Debug("Server 'tls-unique': {TlsUnique}.", ToHexString(tlsUnique));
278281
}
279282

280283
OnHandshakeCompleted?.Invoke(this, new DtlsHandshakeCompletedEventArgs(m_context.SecurityParameters));

‎src/SIPSorcery/net/DtlsSrtp/Lib/Log.cs‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020
// SOFTWARE.
2121

2222
using System;
23+
using System.Globalization;
24+
using System.Text.RegularExpressions;
2325

2426
namespace SIPSorcery.Net.SharpSRTP
2527
{
@@ -53,6 +55,11 @@ public static void Debug(string message, Exception ex = null)
5355
SinkDebug(message, ex);
5456
}
5557

58+
public static void Debug(string messageTemplate, params object[] args)
59+
{
60+
SinkDebug(FormatTemplate(messageTemplate, args), null);
61+
}
62+
5663
public static bool InfoEnabled { get; set; }
5764
#if DEBUG
5865
= true;
@@ -67,5 +74,19 @@ public static void Info(string message, Exception ex = null)
6774
public static Action<string, Exception> SinkTrace = new Action<string, Exception>((m, ex) => { System.Diagnostics.Debug.WriteLine(m); });
6875
public static Action<string, Exception> SinkDebug = new Action<string, Exception>((m, ex) => { System.Diagnostics.Debug.WriteLine(m); });
6976
public static Action<string, Exception> SinkInfo = new Action<string, Exception>((m, ex) => { System.Diagnostics.Debug.WriteLine(m); });
77+
78+
private static readonly Regex MessageTemplateRegex = new Regex(@"(?<!\{)\{[^{}]+\}(?!\})", RegexOptions.Compiled);
79+
80+
private static string FormatTemplate(string messageTemplate, object[] args)
81+
{
82+
if (string.IsNullOrEmpty(messageTemplate) || args == null || args.Length == 0)
83+
{
84+
return messageTemplate;
85+
}
86+
87+
int index = 0;
88+
string compositeFormat = MessageTemplateRegex.Replace(messageTemplate, _ => $"{{{index++}}}");
89+
return string.Format(CultureInfo.InvariantCulture, compositeFormat, args);
90+
}
7091
}
7192
}

0 commit comments

Comments
 (0)