DO NOT MERGE: Work around several issues with non-compliant RTSP servers.

In this particular case these RTSP servers were implemented as local services,
retransmitting live streams via a local RTSP server instance.

They picked wrong rtp/rtcp port pairs (odd rtp port), blank lines in the session
description, wrong case of the format description, relative base URLs...

Change-Id: I502a04a7e1d690fd461b7ecf0b56c6a6c2ac1325
related-to-bug: 3452103

diff --git a/media/libstagefright/rtsp/AMPEG4ElementaryAssembler.cpp b/media/libstagefright/rtsp/AMPEG4ElementaryAssembler.cpp
index 13988cd..9f6bd29 100644 (file)
--- a/media/libstagefright/rtsp/AMPEG4ElementaryAssembler.cpp
+++ b/media/libstagefright/rtsp/AMPEG4ElementaryAssembler.cpp
@@ -104,7 +104,7 @@ AMPEG4ElementaryAssembler::AMPEG4ElementaryAssembler(
       mNextExpectedSeqNoValid(false),
       mNextExpectedSeqNo(0),
       mAccessUnitDamaged(false) {
-    mIsGeneric = desc.startsWith("mpeg4-generic/");
+    mIsGeneric = !strncasecmp(desc.c_str(),"mpeg4-generic/", 14);
 
     if (mIsGeneric) {
         AString value;

diff --git a/media/libstagefright/rtsp/APacketSource.cpp b/media/libstagefright/rtsp/APacketSource.cpp
index 10cc88b..7f09248 100644 (file)
--- a/media/libstagefright/rtsp/APacketSource.cpp
+++ b/media/libstagefright/rtsp/APacketSource.cpp
@@ -627,7 +627,7 @@ APacketSource::APacketSource(
 
         mFormat->setInt32(kKeyWidth, width);
         mFormat->setInt32(kKeyHeight, height);
-    } else if (!strncmp(desc.c_str(), "mpeg4-generic/", 14)) {
+    } else if (!strncasecmp(desc.c_str(), "mpeg4-generic/", 14)) {
         AString val;
         if (!GetAttribute(params.c_str(), "mode", &val)
                 || (strcasecmp(val.c_str(), "AAC-lbr")

diff --git a/media/libstagefright/rtsp/ARTPConnection.cpp b/media/libstagefright/rtsp/ARTPConnection.cpp
index 5a1ea5c..72943ff 100644 (file)
--- a/media/libstagefright/rtsp/ARTPConnection.cpp
+++ b/media/libstagefright/rtsp/ARTPConnection.cpp
@@ -123,7 +123,7 @@ void ARTPConnection::MakePortPair(
         struct sockaddr_in addr;
         memset(addr.sin_zero, 0, sizeof(addr.sin_zero));
         addr.sin_family = AF_INET;
-        addr.sin_addr.s_addr = INADDR_ANY;
+        addr.sin_addr.s_addr = htonl(INADDR_ANY);
         addr.sin_port = htons(port);
 
         if (bind(*rtpSocket,
@@ -346,6 +346,8 @@ void ARTPConnection::onPollStreams() {
 }
 
 status_t ARTPConnection::receive(StreamInfo *s, bool receiveRTP) {
+    LOGV("receiving %s", receiveRTP ? "RTP" : "RTCP");
+
     CHECK(!s->mIsInjected);
 
     sp<ABuffer> buffer = new ABuffer(65536);

diff --git a/media/libstagefright/rtsp/ARTPSource.cpp b/media/libstagefright/rtsp/ARTPSource.cpp
index 5aae4e7..87b5a7e 100644 (file)
--- a/media/libstagefright/rtsp/ARTPSource.cpp
+++ b/media/libstagefright/rtsp/ARTPSource.cpp
@@ -67,7 +67,7 @@ ARTPSource::ARTPSource(
     } else  if (!strncmp(desc.c_str(), "AMR-WB/", 7)) {
         mAssembler = new AAMRAssembler(notify, true /* isWide */, params);
     } else if (!strncmp(desc.c_str(), "MP4V-ES/", 8)
-            || !strncmp(desc.c_str(), "mpeg4-generic/", 14)) {
+            || !strncasecmp(desc.c_str(), "mpeg4-generic/", 14)) {
         mAssembler = new AMPEG4ElementaryAssembler(notify, desc, params);
         mIssueFIRRequests = true;
     } else {

diff --git a/media/libstagefright/rtsp/ASessionDescription.cpp b/media/libstagefright/rtsp/ASessionDescription.cpp
index 2843ee6..aa2618e 100644 (file)
--- a/media/libstagefright/rtsp/ASessionDescription.cpp
+++ b/media/libstagefright/rtsp/ASessionDescription.cpp
@@ -71,6 +71,11 @@ bool ASessionDescription::parse(const void *data, size_t size) {
             line.setTo(desc, i, eolPos - i);
         }
 
+        if (line.empty()) {
+            i = eolPos + 1;
+            continue;
+        }
+
         if (line.size() < 2 || line.c_str()[1] != '=') {
             return false;
         }

diff --git a/media/libstagefright/rtsp/MyHandler.h b/media/libstagefright/rtsp/MyHandler.h
index 5c6ff82..18145e4 100644 (file)
--- a/media/libstagefright/rtsp/MyHandler.h
+++ b/media/libstagefright/rtsp/MyHandler.h
@@ -246,7 +246,7 @@ struct MyHandler : public AHandler {
     // In case we're behind NAT, fire off two UDP packets to the remote
     // rtp/rtcp ports to poke a hole into the firewall for future incoming
     // packets. We're going to send an RR/SDES RTCP packet to both of them.
-    void pokeAHole(int rtpSocket, int rtcpSocket, const AString &transport) {
+    bool pokeAHole(int rtpSocket, int rtcpSocket, const AString &transport) {
         AString source;
         AString server_port;
         if (!GetAttribute(transport.c_str(),
@@ -255,16 +255,25 @@ struct MyHandler : public AHandler {
                 || !GetAttribute(transport.c_str(),
                                  "server_port",
                                  &server_port)) {
-            return;
+            return false;
         }
 
         int rtpPort, rtcpPort;
         if (sscanf(server_port.c_str(), "%d-%d", &rtpPort, &rtcpPort) != 2
                 || rtpPort <= 0 || rtpPort > 65535
                 || rtcpPort <=0 || rtcpPort > 65535
-                || rtcpPort != rtpPort + 1
-                || (rtpPort & 1) != 0) {
-            return;
+                || rtcpPort != rtpPort + 1) {
+            LOGE("Server picked invalid RTP/RTCP port pair %s,"
+                 " RTP port must be even, RTCP port must be one higher.",
+                 server_port.c_str());
+
+            return false;
+        }
+
+        if (rtpPort & 1) {
+            LOGW("Server picked an odd RTP port, it should've picked an "
+                 "even one, we'll let it pass for now, but this may break "
+                 "in the future.");
         }
 
         struct sockaddr_in addr;
@@ -273,7 +282,12 @@ struct MyHandler : public AHandler {
         addr.sin_addr.s_addr = inet_addr(source.c_str());
 
         if (addr.sin_addr.s_addr == INADDR_NONE) {
-            return;
+            return true;
+        }
+
+        if (IN_LOOPBACK(ntohl(addr.sin_addr.s_addr))) {
+            // No firewalls to traverse on the loopback interface.
+            return true;
         }
 
         // Make up an RR/SDES RTCP packet.
@@ -287,16 +301,26 @@ struct MyHandler : public AHandler {
         ssize_t n = sendto(
                 rtpSocket, buf->data(), buf->size(), 0,
                 (const sockaddr *)&addr, sizeof(addr));
-        CHECK_EQ(n, (ssize_t)buf->size());
+
+        if (n < (ssize_t)buf->size()) {
+            LOGE("failed to poke a hole for RTP packets");
+            return false;
+        }
 
         addr.sin_port = htons(rtcpPort);
 
         n = sendto(
                 rtcpSocket, buf->data(), buf->size(), 0,
                 (const sockaddr *)&addr, sizeof(addr));
-        CHECK_EQ(n, (ssize_t)buf->size());
+
+        if (n < (ssize_t)buf->size()) {
+            LOGE("failed to poke a hole for RTCP packets");
+            return false;
+        }
 
         LOGV("successfully poked holes.");
+
+        return true;
     }
 
     virtual void onMessageReceived(const sp<AMessage> &msg) {
@@ -379,6 +403,7 @@ struct MyHandler : public AHandler {
                                 response->mContent->size());
 
                         if (!mSessionDesc->isValid()) {
+                            LOGE("Failed to parse session description.");
                             result = ERROR_MALFORMED;
                         } else {
                             ssize_t i = response->mHeaders.indexOfKey("content-base");
@@ -393,6 +418,25 @@ struct MyHandler : public AHandler {
                                 }
                             }
 
+                            if (!mBaseURL.startsWith("rtsp://")) {
+                                // Some misbehaving servers specify a relative
+                                // URL in one of the locations above, combine
+                                // it with the absolute session URL to get
+                                // something usable...
+
+                                LOGW("Server specified a non-absolute base URL"
+                                     ", combining it with the session URL to "
+                                     "get something usable...");
+
+                                AString tmp;
+                                CHECK(MakeURL(
+                                            mSessionURL.c_str(),
+                                            mBaseURL.c_str(),
+                                            &tmp));
+
+                                mBaseURL = tmp;
+                            }
+
                             CHECK_GT(mSessionDesc->countTracks(), 1u);
                             setupTrack(1);
                         }
@@ -453,17 +497,22 @@ struct MyHandler : public AHandler {
                         if (!track->mUsingInterleavedTCP) {
                             AString transport = response->mHeaders.valueAt(i);
 
-                            pokeAHole(track->mRTPSocket,
-                                      track->mRTCPSocket,
-                                      transport);
+                            if (!pokeAHole(
+                                        track->mRTPSocket,
+                                        track->mRTCPSocket,
+                                        transport)) {
+                                result = UNKNOWN_ERROR;
+                            }
                         }
 
-                        mRTPConn->addStream(
-                                track->mRTPSocket, track->mRTCPSocket,
-                                mSessionDesc, index,
-                                notify, track->mUsingInterleavedTCP);
+                        if (result == OK) {
+                            mRTPConn->addStream(
+                                    track->mRTPSocket, track->mRTCPSocket,
+                                    mSessionDesc, index,
+                                    notify, track->mUsingInterleavedTCP);
 
-                        mSetupTracksSuccessful = true;
+                            mSetupTracksSuccessful = true;
+                        }
                     }
                 }
 
@@ -865,10 +914,7 @@ struct MyHandler : public AHandler {
             case 'tiou':
             {
                 if (!mReceivedFirstRTCPPacket) {
-                    if (mTryFakeRTCP) {
-                        LOGW("Never received any data, disconnecting.");
-                        (new AMessage('abor', id()))->post();
-                    } else if (mTryTCPInterleaving && mReceivedFirstRTPPacket) {
+                    if (mReceivedFirstRTPPacket && !mTryFakeRTCP) {
                         LOGW("We received RTP packets but no RTCP packets, "
                              "using fake timestamps.");
 
@@ -876,7 +922,7 @@ struct MyHandler : public AHandler {
 
                         mReceivedFirstRTCPPacket = true;
                         mRTPConn->fakeTimestamps();
-                    } else {
+                    } else if (!mReceivedFirstRTPPacket && !mTryTCPInterleaving) {
                         LOGW("Never received any data, switching transports.");
 
                         mTryTCPInterleaving = true;
@@ -884,6 +930,9 @@ struct MyHandler : public AHandler {
                         sp<AMessage> msg = new AMessage('abor', id());
                         msg->setInt32("reconnect", true);
                         msg->post();
+                    } else {
+                        LOGW("Never received any data, disconnecting.");
+                        (new AMessage('abor', id()))->post();
                     }
                 }
                 break;