Re: [PATCH] Fix building with Clang

Reimar Döffinger <[email protected]> Thu, 23 Dec 2021 20:55:30 +0100
Newsgroups gmane.comp.video.mplayer.devel
Message-ID <[email protected]> 

> On 23 Dec 2021, at 15:57, Brad Smith <[email protected]> wrote:
> 
> On 12/20/2021 10:47 AM, Reimar Döffinger wrote:
> 
>> 
>>> On 10 Dec 2021, at 07:48, Brad Smith <[email protected]> wrote:
>>> 
>>> Fix building with Clang.
>>> 
>>> 
>>> ./stream/stream.h:322:46: error: invalid suffix on literal; C++11 requires a space between literal and identifier [-Wreserved-user-defined-literal]
>>>  mp_dbg(MSGT_DEMUX, MSGL_DBG3, "seek to 0x%"PRIX64"\n", pos);
>>>                                             ^
>>> 
>>> ./stream/stream.h:326:49: error: invalid suffix on literal; C++11 requires a space between literal and identifier [-Wreserved-user-defined-literal]
>>>           "Invalid seek to negative position %"PRIx64"!\n", pos);
>> Sorry for the late reply, but I think this needs a bit more details/explanation.
>> 1) I’ve quickly tested on macOS which uses clang and it has no issues, so this can’t be a problem with clang in general I think?
>> 2) MPlayer is not C++11, in fact it is not C++ at all, so this messages does not really make sense. Sure it’s not a bug in clang, or maybe you used clang++ instead of clang as compiler?
>> 
>> Now the change is not a big deal, it’s just that I am not a big fan of working around issues that are not properly understood, also because if it’s a real issue the commit message should make it clear how we can avoid re-introducing such issues in the future.
> 
> 1) Looks like you're not building with Live555 support.

I did now, it makes no difference.
Are you actually using it? Because MPlayer doesn't work with any version current enough to not be full of security issues I think...
I had to use attached hack to make it just compile.

> 2) Although almost all of MPlayer is C code there are two files in the tree that are C++ code.

Why does it fail with your compilers when it seems to work with all the ones I have at hand?
It seems the problem might be your compiler defaulting to C++11, and in the future even newer C++ versions.
So the real issue might be that this old code really should be compiled with -std=c++98 if that option is available?
After all your change might fix the issue with C++11 compatibility, but who knows what other issues would pop up if the compiler tried to compile it as C++30 or whatever...
That said I don't know if there is a point in keeping these mp_dbg macros at all.

_______________________________________________
MPlayer-dev-eng mailing list
[email protected]
https://lists.mplayerhq.hu/mailman/listinfo/mplayer-dev-eng
live555.diff (application/octet-stream, 3.7 KB) 
Index: configure
===================================================================
--- configure	(revision 38328)
+++ configure	(working copy)
@@ -7372,11 +7372,11 @@
 #error Please upgrade to version 2006.03.03 or later of the "LIVE555 Streaming Media" libraries - available from <www.live555.com/liveMedia/>
 #endif
 #include "BasicUsageEnvironment.hh"
-int main(void) { RTSPClient::createNew(*BasicUsageEnvironment::createNew(*BasicTaskScheduler::createNew()), 0, "", 0); return 0; }
+int main(void) { RTSPClient::createNew(*BasicUsageEnvironment::createNew(*BasicTaskScheduler::createNew()), "", 0, "", 0); return 0; }
 EOF

   _live=no
-  for I in $extra_cflags "-I$_libdir/live" "-I/usr/lib/live" "-I/usr/lib64/live" "-I/usr/local/live" "-I/usr/local/lib/live" ; do
+  for I in $extra_cflags "-I$_libdir/live" "-I/usr/lib/live" "-I/usr/lib64/live" "-I/usr/local/live" "-I/usr/local/lib/live" "-I/opt/local/lib/live" ; do
     _livelibdir=$(echo $I| sed s/-I//)
     inc_tmp="-I$_livelibdir/liveMedia/include \
              -I$_livelibdir/UsageEnvironment/include \
Index: libmpdemux/demux_rtp.cpp
===================================================================
--- libmpdemux/demux_rtp.cpp	(revision 38327)
+++ libmpdemux/demux_rtp.cpp	(working copy)
@@ -92,16 +92,23 @@
   struct timeval firstSyncTime;
 };

+static char * volatile respStr;
+static void respHandler(RTSPClient *, int, char *str)
+{
+    respStr = str;
+}
+
 extern "C" char* network_username;
 extern "C" char* network_password;
 static char* openURL_rtsp(RTSPClient* client, char const* url) {
   // If we were given a user name (and optional password), then use them:
-  if (network_username != NULL) {
-    char const* password = network_password == NULL ? "" : network_password;
-    return client->describeWithPassword(url, network_username, password);
-  } else {
-    return client->describeURL(url);
-  }
+  char const* username = network_username == NULL ? "" : network_username;
+  char const* password = network_password == NULL ? "" : network_password;
+  Authenticator auth(username, password);
+  respStr = NULL;
+  client->sendDescribeCommand(respHandler, &auth);
+  while (!respStr) ;
+  return respStr;
 }

 static char* openURL_sip(SIPClient* client, char const* url) {
@@ -146,7 +153,7 @@
 	  rtsp_transport_http = demuxer->stream->streaming_ctrl->url->port;
 	  rtsp_transport_tcp = 1;
 	}
-	rtspClient = RTSPClient::createNew(*env, verbose, "MPlayer", rtsp_transport_http);
+	rtspClient = RTSPClient::createNew(*env, url, verbose, "MPlayer", rtsp_transport_http);
 	if (rtspClient == NULL) {
 	  fprintf(stderr, "Failed to create RTSP client: %s\n",
 		  env->getResultMsg());
@@ -236,7 +243,7 @@

 	if (rtspClient != NULL) {
 	  // Issue a RTSP "SETUP" command on the chosen subsession:
-	  if (!rtspClient->setupMediaSubsession(*subsession, False,
+	  if (!rtspClient->sendSetupCommand(*subsession, respHandler, False,
 						rtsp_transport_tcp)) break;
 	  if (!strcmp(subsession->mediumName(), "audio"))
 	    audiofound = 1;
@@ -248,7 +255,7 @@

     if (rtspClient != NULL) {
       // Issue a RTSP aggregate "PLAY" command on the whole session:
-      if (!rtspClient->playMediaSession(*mediaSession)) break;
+      if (!rtspClient->sendPlayCommand(*mediaSession, respHandler)) break;
     } else if (sipClient != NULL) {
       sipClient->sendACK(); // to start the stream flowing
     }
@@ -637,7 +644,7 @@
   MediaSession* mediaSession = rtpState->mediaSession;
   if (mediaSession == NULL) return;
   if (rtpState->rtspClient != NULL) {
-    rtpState->rtspClient->teardownMediaSession(*mediaSession);
+    rtpState->rtspClient->sendTeardownCommand(*mediaSession, respHandler);
   } else if (rtpState->sipClient != NULL) {
     rtpState->sipClient->sendBYE();
   }