rtsp: Only do keepalive using GET_PARAMETER if the server supports it

Message ID 1304962153-58647-1-git-send-email-martin@martin.st
State Committed
Commit 0b4949b51816bc2fd23ba4c4de183b877b58aa25
Headers show

Commit Message

Martin Storsjö May 9, 2011, 5:29 p.m.
This is more like what VLC does. If the server doesn't mention
supporting GET_PARAMETER in response to an OPTIONS request,
VLC doesn't send any keepalive requests at all. After this patch,
libavformat will still send OPTIONS keepalives if GET_PARAMETER
isn't explicitly said to be supported.

Some RTSP cameras don't support GET_PARAMETER, and will
close the connection if this is sent as keepalive request
(but support OPTIONS just fine, but probably don't need any
keepalive at all). Some other cameras don't support using
OPTIONS as keepalive, but require GET_PARAMETER instead.
---

This is an attempt to solve the issue reported at
https://ffmpeg.org/trac/ffmpeg/ticket/129. It still is
pending testing both by the reporter of that issue,
and by Tim Ouellette, to make sure it doesn't break
his streams.

 libavformat/rtsp.c    |    4 ++++
 libavformat/rtsp.h    |    5 +++++
 libavformat/rtspdec.c |    4 +++-
 3 files changed, 12 insertions(+), 1 deletions(-)

Comments

Ronald Bultje May 9, 2011, 9:04 p.m. | #1
Hi,

On Mon, May 9, 2011 at 1:29 PM, Martin Storsjö <martin@martin.st> wrote:
> This is more like what VLC does. If the server doesn't mention
> supporting GET_PARAMETER in response to an OPTIONS request,
> VLC doesn't send any keepalive requests at all. After this patch,
> libavformat will still send OPTIONS keepalives if GET_PARAMETER
> isn't explicitly said to be supported.
>
> Some RTSP cameras don't support GET_PARAMETER, and will
> close the connection if this is sent as keepalive request
> (but support OPTIONS just fine, but probably don't need any
> keepalive at all). Some other cameras don't support using
> OPTIONS as keepalive, but require GET_PARAMETER instead.
> ---
>
> This is an attempt to solve the issue reported at
> https://ffmpeg.org/trac/ffmpeg/ticket/129. It still is
> pending testing both by the reporter of that issue,
> and by Tim Ouellette, to make sure it doesn't break
> his streams.
>
>  libavformat/rtsp.c    |    4 ++++
>  libavformat/rtsp.h    |    5 +++++
>  libavformat/rtspdec.c |    4 +++-
>  3 files changed, 12 insertions(+), 1 deletions(-)

Patch looks good, thanks.

Ronald
Martin Storsjö May 11, 2011, 7:45 a.m. | #2
On Mon, 9 May 2011, Ronald S. Bultje wrote:

> On Mon, May 9, 2011 at 1:29 PM, Martin Storsjö <martin@martin.st> wrote:
> > This is more like what VLC does. If the server doesn't mention
> > supporting GET_PARAMETER in response to an OPTIONS request,
> > VLC doesn't send any keepalive requests at all. After this patch,
> > libavformat will still send OPTIONS keepalives if GET_PARAMETER
> > isn't explicitly said to be supported.
> >
> > Some RTSP cameras don't support GET_PARAMETER, and will
> > close the connection if this is sent as keepalive request
> > (but support OPTIONS just fine, but probably don't need any
> > keepalive at all). Some other cameras don't support using
> > OPTIONS as keepalive, but require GET_PARAMETER instead.
> > ---
> >
> > This is an attempt to solve the issue reported at
> > https://ffmpeg.org/trac/ffmpeg/ticket/129. It still is
> > pending testing both by the reporter of that issue,
> > and by Tim Ouellette, to make sure it doesn't break
> > his streams.
> >
> >  libavformat/rtsp.c    |    4 ++++
> >  libavformat/rtsp.h    |    5 +++++
> >  libavformat/rtspdec.c |    4 +++-
> >  3 files changed, 12 insertions(+), 1 deletions(-)
> 
> Patch looks good, thanks.

Pushed.

Didn't hear back from Tim yet, but as far as I can see from the stream 
dumps he sent when he reported the issue, this shouldn't change the 
behaviour in his case.

// Martin

Patch

diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
index 14111e6..2ebf7e0 100644
--- a/libavformat/rtsp.c
+++ b/libavformat/rtsp.c
@@ -808,6 +808,10 @@  void ff_rtsp_parse_line(RTSPMessageHeader *reply, const char *buf,
         p += strspn(p, SPACE_CHARS);
         if (method && !strcmp(method, "PLAY"))
             rtsp_parse_rtp_info(rt, p);
+    } else if (av_stristart(p, "Public:", &p) && rt) {
+        if (strstr(p, "GET_PARAMETER") &&
+            method && !strcmp(method, "OPTIONS"))
+            rt->get_parameter_supported = 1;
     }
 }
 
diff --git a/libavformat/rtsp.h b/libavformat/rtsp.h
index e1f1df9..ff66502 100644
--- a/libavformat/rtsp.h
+++ b/libavformat/rtsp.h
@@ -331,6 +331,11 @@  typedef struct RTSPState {
      * Polling array for udp
      */
     struct pollfd *p;
+
+    /**
+     * Whether the server supports the GET_PARAMETER method.
+     */
+    int get_parameter_supported;
 } RTSPState;
 
 /**
diff --git a/libavformat/rtspdec.c b/libavformat/rtspdec.c
index 866f313..ccfc4d8 100644
--- a/libavformat/rtspdec.c
+++ b/libavformat/rtspdec.c
@@ -341,7 +341,9 @@  retry:
 
     /* send dummy request to keep TCP connection alive */
     if ((av_gettime() - rt->last_cmd_time) / 1000000 >= rt->timeout / 2) {
-        if (rt->server_type != RTSP_SERVER_REAL) {
+        if (rt->server_type == RTSP_SERVER_WMS ||
+           (rt->server_type != RTSP_SERVER_REAL &&
+            rt->get_parameter_supported)) {
             ff_rtsp_send_cmd_async(s, "GET_PARAMETER", rt->control_uri, NULL);
         } else {
             ff_rtsp_send_cmd_async(s, "OPTIONS", "*", NULL);