[Live-devel] [PATCH] Register response callback prior sending RTSP command

2020-12-09 Thread Lionel Koenig
When sending RTSP command from another thread than the event loop (it is
actually unsupported see:
http://www.live555.com/liveMedia/faq.html#threads), the response of the
command might arrive prior the handler callback is registered. When that
happen, the handler will never be called even if the response was
received.
In this patch, the registration of the callback is done prior the
command is sent to ensure that the response callback is available when
the reply come back.
In case of failing sending the command, the response callback is
deregistered.
---
 liveMedia/RTSPClient.cpp | 20 ++--
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/liveMedia/RTSPClient.cpp b/liveMedia/RTSPClient.cpp
index aaae3ba..8bdabc0 100644
--- a/liveMedia/RTSPClient.cpp
+++ b/liveMedia/RTSPClient.cpp
@@ -555,7 +555,20 @@ unsigned RTSPClient::sendRequest(RequestRecord* request) {
   delete[] origCmd;
 }
 
+// Enqueue the request record, so that its response (when it comes) can be 
handled.
+// However, note that we do not expect a response to a POST command with 
RTSP-over-HTTP, so don't enqueue that.
+Boolean awaitingResponse = false;
+if (fTunnelOverHTTPPortNum == 0 || strcmp(request->commandName(), "POST") 
!= 0) {
+  fRequestsAwaitingResponse.enqueue(request);
+  awaitingResponse = true;
+}
+
 if (write(cmd, strlen(cmd)) < 0) {
+  if (awaitingResponse) {
+// Sending the request failed, unregister the response handler.
+fRequestsAwaitingResponse.dequeue();
+awaitingResponse = false;
+  }
   char const* errFmt = "%s write() failed: ";
   unsigned const errLength = strlen(errFmt) + 
strlen(request->commandName());
   char* err = new char[errLength];
@@ -565,13 +578,8 @@ unsigned RTSPClient::sendRequest(RequestRecord* request) {
   break;
 }
 
-// The command send succeeded, so enqueue the request record, so that its 
response (when it comes) can be handled.
-// However, note that we do not expect a response to a POST command with 
RTSP-over-HTTP, so don't enqueue that.
 int cseq = request->cseq();
-
-if (fTunnelOverHTTPPortNum == 0 || strcmp(request->commandName(), "POST") 
!= 0) {
-  fRequestsAwaitingResponse.enqueue(request);
-} else {
+if (!awaitingResponse) {
   delete request;
 }
 
-- 
___
live-devel mailing list
live-devel@lists.live555.com
http://lists.live555.com/mailman/listinfo/live-devel


Re: [Live-devel] [PATCH] Register response callback prior sending RTSP command

2020-12-09 Thread Ross Finlayson
OK, so let me get this straight: You are doing something (calling LIVE555 code 
(beyond just “triggerEvent()”) from a thread outside the event loop) that you 
know is ‘unsupported’, and which you (should) also know could produce many race 
conditions that may cause your application to fail in unpredictable ways.  And 
you expect me to accept a patch that removes just one of these many possible 
race conditions that can occur if you do something that you shouldn’t be doing?

Sorry, but I’m not going to be applying this patch.  
http://www.live555.com/liveMedia/faq.html#threads means what it says.  You 
should only be calling this code from the event loop.  If you do this, then - 
after the call to “write()” - the current code will always get to enqueue the 
request record before it ends up returning to the event loop to handle the 
response.

What I don’t understand is why you even wanted to send RTSP commands from 
outside the event loop in the first place?  A RTSP command is usually sent only 
after handling a RTSP response, or after some time has expired - both of which 
are events that are handled within the event loop.  And if you want to open 
multiple RTSP stream (each with its own “rtsp://“ URL) concurrently, then you 
can easily do this within a single event loop, as illustrated by our 
“testRTSPClient” command; see the code at lines 77-80 and 182-184 in 
“testProgs/testRTSPClient.cpp”.


Ross Finlayson
Live Networks, Inc.
http://www.live555.com/


___
live-devel mailing list
live-devel@lists.live555.com
http://lists.live555.com/mailman/listinfo/live-devel