rtsp: punch holes again after pause

Message ID 1424639240-59150-1-git-send-email-martin@martin.st
State Superseded
Headers show

Commit Message

Martin Storsjö Feb. 22, 2015, 9:07 p.m.
From: Gilles Chanteperdrix <gilles.chanteperdrix@xenomai.org>

When a client behind a NAT issues a pause command, and stay paused for a
long time, the router may stop the RTP/RTCP port redirection. Resend the
hole punching packets before each PLAY command to cause the router to
restart the port redirection in that case.

Move the existing code for sending the packets from the SETUP phase
to the PLAY phase.
---
Compared to Gilles' original patch, I removed the extra conditions
(CONFIG_RTPDEC and s->iformat, because within rtspdec.c they are
always set - they are only needed in the common shared codepaths
in rtsp.c).

I also moved the code to a block before sending the PLAY command
instead of immediately after (and the loop/block after PLAY was a
block only executed if reply->range_start != AV_NOPTS_VALUE), and
removed sending them in the SETUP phase - we don't need to send
them twice for each port at startup.

I also added checks for the lower transport method and checking
that rtp_handle is non-null - this avoids crashing if trying to
play back streams with tcp as lower transport.
---
 libavformat/rtsp.c    | 7 -------
 libavformat/rtspdec.c | 8 ++++++++
 2 files changed, 8 insertions(+), 7 deletions(-)

Comments

Gilles Chanteperdrix Feb. 22, 2015, 9:11 p.m. | #1
On Sun, Feb 22, 2015 at 11:07:20PM +0200, Martin Storsjö wrote:
> -            /* Try to initialize the connection state in a
> -             * potential NAT router by sending dummy packets.
> -             * RTP/RTCP dummy packets are used for RDT, too.
> -             */
> -            if (CONFIG_RTPDEC &&
> -                !(rt->server_type == RTSP_SERVER_WMS && i > 1) && s->iformat)
> -                ff_rtp_send_punch_packets(rtsp_st->rtp_handle);
>              break;

The reason why I did not remove this hunk is the comment which seem
to imply that these punch packets are used for something else than
RTP.
Martin Storsjö Feb. 22, 2015, 9:17 p.m. | #2
On Sun, 22 Feb 2015, Gilles Chanteperdrix wrote:

> On Sun, Feb 22, 2015 at 11:07:20PM +0200, Martin Storsjö wrote:
>> -            /* Try to initialize the connection state in a
>> -             * potential NAT router by sending dummy packets.
>> -             * RTP/RTCP dummy packets are used for RDT, too.
>> -             */
>> -            if (CONFIG_RTPDEC &&
>> -                !(rt->server_type == RTSP_SERVER_WMS && i > 1) && s->iformat)
>> -                ff_rtp_send_punch_packets(rtsp_st->rtp_handle);
>>              break;
>
> The reason why I did not remove this hunk is the comment which seem
> to imply that these punch packets are used for something else than
> RTP.

Ah - RDT is the transport layer used in RealRTSP. For these purposes, it 
is treated the same as normal RTP (it uses UDP, but the packets are parsed 
slightly differently) - setting up RDT goes through the same SETUP/PLAY 
phases. But you're right, the existing loop I added it in isn't executed 
for RDT either - I'll move it to a loop of its own.

// Martin

Patch

diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
index bd6494a..2aa19dc 100644
--- a/libavformat/rtsp.c
+++ b/libavformat/rtsp.c
@@ -1540,13 +1540,6 @@  int ff_rtsp_make_setup_request(AVFormatContext *s, const char *host, int port,
                 err = AVERROR_INVALIDDATA;
                 goto fail;
             }
-            /* Try to initialize the connection state in a
-             * potential NAT router by sending dummy packets.
-             * RTP/RTCP dummy packets are used for RDT, too.
-             */
-            if (CONFIG_RTPDEC &&
-                !(rt->server_type == RTSP_SERVER_WMS && i > 1) && s->iformat)
-                ff_rtp_send_punch_packets(rtsp_st->rtp_handle);
             break;
         }
         case RTSP_LOWER_TRANSPORT_UDP_MULTICAST: {
diff --git a/libavformat/rtspdec.c b/libavformat/rtspdec.c
index 418f383..abc0818 100644
--- a/libavformat/rtspdec.c
+++ b/libavformat/rtspdec.c
@@ -509,6 +509,14 @@  static int rtsp_read_play(AVFormatContext *s)
             for (i = 0; i < rt->nb_rtsp_streams; i++) {
                 RTSPStream *rtsp_st = rt->rtsp_streams[i];
                 RTPDemuxContext *rtpctx = rtsp_st->transport_priv;
+                /* Try to initialize the connection state in a
+                 * potential NAT router by sending dummy packets.
+                 * RTP/RTCP dummy packets are used for RDT, too.
+                 */
+                if (rt->lower_transport == RTSP_LOWER_TRANSPORT_UDP &&
+                    rtsp_st->rtp_handle &&
+                    !(rt->server_type == RTSP_SERVER_WMS && i > 1))
+                    ff_rtp_send_punch_packets(rtsp_st->rtp_handle);
                 if (!rtpctx)
                     continue;
                 ff_rtp_reset_packet_queue(rtpctx);