Skip to content

Commit 225641e

Browse files
authored
Refactor RTSP client: modern parsing & structured logging (#1654)
Refactored RTSP client and protocol parsing for better performance, maintainability, and logging. Adopted structured logging, Span<T>-based parsing, and efficient string handling. Improved exception messages, header normalization, and ToString() methods. Cleaned up code and enhanced error handling throughout.
1 parent 6f49ffa commit 225641e

6 files changed

Lines changed: 245 additions & 144 deletions

File tree

src/SIPSorcery/net/RTSP/RTSPClient.cs

Lines changed: 26 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ public string GetStreamDescription(string url)
8989
string hostname = Regex.Match(url, @"rtsp://(?<hostname>\S+?)/").Result("${hostname}");
9090
//IPEndPoint rtspEndPoint = DNSResolver.R(hostname, DNS_RESOLUTION_TIMEOUT);
9191

92-
logger.LogDebug("RTSP Client Connecting to " + hostname + ".");
92+
logger.LogDebug("RTSP Client connecting to {Hostname}.", hostname);
9393
TcpClient rtspSocket = new TcpClient(hostname, RTSP_PORT);
9494
NetworkStream rtspStream = rtspSocket.GetStream();
9595

@@ -118,14 +118,14 @@ public string GetStreamDescription(string url)
118118
if (rtspMessage.RTSPMessageType == RTSPMessageTypesEnum.Response)
119119
{
120120
rtspResponse = RTSPResponse.ParseRTSPResponse(rtspMessage);
121-
logger.LogDebug("RTSP Response received: " + rtspResponse.StatusCode + " " + rtspResponse.Status + " " + rtspResponse.ReasonPhrase + ".");
121+
logger.LogDebug("RTSP Response received: {StatusCode} {Status} {ReasonPhrase}.", rtspResponse.StatusCode, rtspResponse.Status, rtspResponse.ReasonPhrase);
122122
}
123123

124124
rtspSDP = rtspResponse.Body;
125125
}
126126
else
127127
{
128-
logger.LogWarning("Socket closed prematurely in GetStreamDescription for " + url + ".");
128+
logger.LogWarning("Socket closed prematurely in {Method} for {Url}.", nameof(GetStreamDescription), url);
129129
}
130130

131131
rtspSocket.Close();
@@ -134,7 +134,7 @@ public string GetStreamDescription(string url)
134134
}
135135
catch (Exception excp)
136136
{
137-
logger.LogError("Exception GetStreamDescription. " + excp.Message);
137+
logger.LogError(excp, "Exception in {Method}.", nameof(GetStreamDescription));
138138
throw excp;
139139
}
140140
}
@@ -147,7 +147,7 @@ public void Start(string url)
147147

148148
if (!urlMatch.Success)
149149
{
150-
throw new ApplicationException("The URL provided to the RTSP client was not recognised, " + url + ".");
150+
throw new ApplicationException($"The URL provided to the RTSP client was not recognised, {url}.");
151151
}
152152
else
153153
{
@@ -160,7 +160,7 @@ public void Start(string url)
160160
hostname = SIPSorcery.Sys.IPSocket.ParseHostFromSocket(hostname);
161161
}
162162

163-
logger.LogDebug("RTSP client connecting to " + hostname + ", port " + port + ".");
163+
logger.LogDebug("RTSP client connecting to {Hostname}, port {Port}.", hostname, port);
164164

165165
_rtspConnection = new TcpClient(hostname, port);
166166
_rtspStream = _rtspConnection.GetStream();
@@ -172,7 +172,7 @@ public void Start(string url)
172172

173173
RTSPRequest rtspRequest = new RTSPRequest(RTSPMethodsEnum.SETUP, url);
174174
RTSPHeader rtspHeader = new RTSPHeader(_cseq++, null);
175-
rtspHeader.Transport = new RTSPTransportHeader() { ClientRTPPortRange = _rtspSession.RTPPort + "-" + _rtspSession.ControlPort };
175+
rtspHeader.Transport = new RTSPTransportHeader() { ClientRTPPortRange = $"{_rtspSession.RTPPort}-{_rtspSession.ControlPort}" };
176176
rtspRequest.Header = rtspHeader;
177177
string rtspReqStr = rtspRequest.ToString();
178178

@@ -202,7 +202,8 @@ public void Start(string url)
202202
_rtspSession.RemoteEndPoint = new IPEndPoint((_rtspConnection.Client.RemoteEndPoint as IPEndPoint).Address, setupResponse.Header.Transport.GetServerRTPPort());
203203
_rtspSession.Start();
204204

205-
logger.LogDebug("RTSP Response received to SETUP: " + setupResponse.Status + ", session ID " + _rtspSession.SessionID + ", server RTP endpoint " + _rtspSession.RemoteEndPoint + ".");
205+
logger.LogDebug("RTSP Response received to SETUP: {Status}, session ID {SessionId}, server RTP endpoint {RemoteEndPoint}.",
206+
setupResponse.Status, _rtspSession.SessionID, _rtspSession.RemoteEndPoint);
206207

207208
if (OnSetupSuccess != null)
208209
{
@@ -211,8 +212,8 @@ public void Start(string url)
211212
}
212213
else
213214
{
214-
logger.LogWarning("RTSP Response received to SETUP: " + setupResponse.Status + ".");
215-
throw new ApplicationException("An error response of " + setupResponse.Status + " was received for an RTSP setup request.");
215+
logger.LogWarning("RTSP Response received to SETUP: {Status}.", setupResponse.Status);
216+
throw new ApplicationException($"An error response of {setupResponse.Status} was received for an RTSP setup request.");
216217
}
217218
}
218219
}
@@ -239,7 +240,7 @@ private void RTPQueueFull()
239240
}
240241
catch (Exception excp)
241242
{
242-
logger.LogError("Exception RTSPClient.RTPQueueFull. " + excp);
243+
logger.LogError(excp, "Exception in {Method}.", nameof(RTPQueueFull));
243244
}
244245
}
245246

@@ -274,7 +275,7 @@ public void Play()
274275
if (rtspMessage.RTSPMessageType == RTSPMessageTypesEnum.Response)
275276
{
276277
var playResponse = RTSPResponse.ParseRTSPResponse(rtspMessage);
277-
logger.LogDebug("RTSP Response received to PLAY: " + playResponse.StatusCode + " " + playResponse.Status + " " + playResponse.ReasonPhrase + ".");
278+
logger.LogDebug("RTSP Response received to PLAY: {StatusCode} {Status} {ReasonPhrase}.", playResponse.StatusCode, playResponse.Status, playResponse.ReasonPhrase);
278279
}
279280
}
280281
else
@@ -292,7 +293,7 @@ private void Teardown()
292293
{
293294
if (_rtspStream != null && _rtspConnection.Connected)
294295
{
295-
logger.LogDebug("RTSP client sending teardown request for " + _url + ".");
296+
logger.LogDebug("RTSP client sending teardown request for {Url}.", _url);
296297

297298
RTSPRequest teardownRequest = new RTSPRequest(RTSPMethodsEnum.TEARDOWN, _url);
298299
RTSPHeader teardownHeader = new RTSPHeader(_cseq++, _rtspSession.SessionID);
@@ -305,12 +306,12 @@ private void Teardown()
305306
}
306307
else
307308
{
308-
logger.LogDebug("RTSP client did not send teardown request for " + _url + ", the socket was closed.");
309+
logger.LogDebug("RTSP client did not send teardown request for {Url}, the socket was closed.", _url);
309310
}
310311
}
311312
catch (Exception excp)
312313
{
313-
logger.LogError("Exception RTSPClient.Teardown. " + excp);
314+
logger.LogError(excp, "Exception in {Method}.", nameof(Teardown));
314315
}
315316
}
316317

@@ -323,7 +324,7 @@ public void Close()
323324
{
324325
if (!_isClosed)
325326
{
326-
logger.LogDebug("RTSP client, closing connection for " + _url + ".");
327+
logger.LogDebug("RTSP client, closing connection for {Url}.", _url);
327328

328329
_isClosed = true;
329330
_sendKeepAlivesMRE.Set();
@@ -343,7 +344,7 @@ public void Close()
343344
}
344345
catch (Exception rtpStreamExcp)
345346
{
346-
logger.LogError("Exception RTSPClient.Close closing RTP stream. " + rtpStreamExcp);
347+
logger.LogError(rtpStreamExcp, "Exception in {Method} closing RTP stream.", nameof(Close));
347348
}
348349
}
349350

@@ -355,7 +356,7 @@ public void Close()
355356
}
356357
catch (Exception excp)
357358
{
358-
logger.LogError("Exception RTSPClient.Close. " + excp);
359+
logger.LogError(excp, "Exception in {Method}.", nameof(Close));
359360
}
360361
}
361362

@@ -392,7 +393,7 @@ private void ProcessRTPPackets()
392393
}
393394

394395
var abbrevURL = (_url.Length <= 50) ? _url : _url.Substring(0, 50);
395-
string rtpTrackingText = String.Format("Url: {0}\r\nRcvd At: {1}\r\nSeq Num: {2}\r\nTS: {3}\r\nPayoad: {4}\r\nFrame Size: {5}\r\nBW: {6}\r\nFrame Rate: {7}", abbrevURL, DateTime.Now.ToString("HH:mm:ss:fff"), rtpPacket.Header.SequenceNumber, rtpPacket.Header.Timestamp, ((SDPMediaFormatsEnum)rtpPacket.Header.PayloadType).ToString(), _lastFrameSize + " bytes", _lastBWCalc.ToString("0.#") + "bps", _lastFrameRate.ToString("0.##") + "fps");
396+
string rtpTrackingText = $"Url: {abbrevURL}\r\nRcvd At: {DateTime.Now:HH:mm:ss:fff}\r\nSeq Num: {rtpPacket.Header.SequenceNumber}\r\nTS: {rtpPacket.Header.Timestamp}\r\nPayoad: {(SDPMediaFormatsEnum)rtpPacket.Header.PayloadType}\r\nFrame Size: {_lastFrameSize} bytes\r\nBW: {_lastBWCalc:0.#}bps\r\nFrame Rate: {_lastFrameRate:0.##}fps";
396397
_rtpTrackingAction(rtpTrackingText);
397398
}
398399

@@ -440,7 +441,7 @@ private void ProcessRTPPackets()
440441
foreach (var oldFrame in _frames.Where(x => x.Timestamp <= rtpPacket.Header.Timestamp).ToList())
441442
{
442443
System.Diagnostics.Debug.WriteLine("Discarding old frame for timestamp " + oldFrame.Timestamp + ".");
443-
logger.LogWarning("Discarding old frame for timestamp " + oldFrame.Timestamp + ".");
444+
logger.LogWarning("Discarding old frame for timestamp {Timestamp}.", oldFrame.Timestamp);
444445
_frames.Remove(oldFrame);
445446
}
446447

@@ -461,7 +462,7 @@ private void ProcessRTPPackets()
461462
}
462463
catch (Exception frameReadyExcp)
463464
{
464-
logger.LogError("Exception RTSPClient.ProcessRTPPackets OnFrameReady. " + frameReadyExcp);
465+
logger.LogError(frameReadyExcp, "Exception in {Method} OnFrameReady.", nameof(ProcessRTPPackets));
465466
}
466467
}
467468
}
@@ -471,7 +472,8 @@ private void ProcessRTPPackets()
471472

472473
if (DateTime.Now.Subtract(_lastRTPReceivedAt).TotalSeconds > RTP_TIMEOUT_SECONDS)
473474
{
474-
logger.LogWarning("No RTP packets were received on RTSP session " + _rtspSession.SessionID + " for " + RTP_TIMEOUT_SECONDS + ". The session will now be closed.");
475+
logger.LogWarning("No RTP packets were received on RTSP session {SessionId} for {TimeoutSeconds}. The session will now be closed.",
476+
_rtspSession.SessionID, RTP_TIMEOUT_SECONDS);
475477
Close();
476478
}
477479
else
@@ -482,7 +484,7 @@ private void ProcessRTPPackets()
482484
}
483485
catch (Exception excp)
484486
{
485-
logger.LogError("Exception RTSPClient.ProcessRTPPackets. " + excp);
487+
logger.LogError(excp, "Exception in {Method}.", nameof(ProcessRTPPackets));
486488
}
487489
}
488490

@@ -538,7 +540,7 @@ private void SendKeepAlives()
538540
}
539541
catch (Exception excp)
540542
{
541-
logger.LogError("Exception RTSPClient.SendKeepAlives. " + excp);
543+
logger.LogError(excp, "Exception in {Method}.", nameof(SendKeepAlives));
542544
}
543545
}
544546
}

0 commit comments

Comments
 (0)