Re: [FFmpeg-devel] [PATCH v2] avformat/rtsp: fix timeout option

[Andriy Gelman]

On Sun, 30. May 16:24, Andriy Gelman wrote:
> From: Andriy Gelman 
> 
> 92c40ef882be115e72d2aa02f9032b7ce88f8537 added a listen_timeout option
> for sdp. This allowed a user to set variable timeout in sdp which was
> originally hard coded to 10 seconds.
> 
> The commit used the initial_timeout variable to store the value.
> However, in rtsp this variable is also used to infer a "listen" mode. Thus,
> the timeout value could not be set when connecting to an rtsp server.
> The default value of -1 would also result in a 100ms total timeout.
> 
> This was attempted to be fixed in c8101aabee654f6d147a4d89f77fa73e18908610,
> which changed the meaning of initial_timeout = -1 to be an infinite
> timeout in rtsp. However, it did not address the issue that the timeout could
> still not be set. Being able to set the timeout is useful because it
> allows to automatically reconfigure from a udp to tcp connection in the
> lower transport.
> 
> In this commit, this is fixed by using the stimeout variable/option to
> set the timeout in rtsp.
> 
> Signed-off-by: Andriy Gelman 
> ---
> 
> Patch didn't apply cleanly anymore. Rebased to master
> 
>  libavformat/rtsp.c | 12 ++--
>  libavformat/rtsp.h |  2 +-
>  2 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
> index 9f509a229f..d4ab8f28bf 100644
> --- a/libavformat/rtsp.c
> +++ b/libavformat/rtsp.c
> @@ -94,7 +94,7 @@ const AVOption ff_rtsp_options[] = {
>  { "min_port", "set minimum local UDP port", OFFSET(rtp_port_min), 
> AV_OPT_TYPE_INT, {.i64 = RTSP_RTP_PORT_MIN}, 0, 65535, DEC|ENC },
>  { "max_port", "set maximum local UDP port", OFFSET(rtp_port_max), 
> AV_OPT_TYPE_INT, {.i64 = RTSP_RTP_PORT_MAX}, 0, 65535, DEC|ENC },
>  { "listen_timeout", "set maximum timeout (in seconds) to wait for 
> incoming connections (-1 is infinite, imply flag listen)", 
> OFFSET(initial_timeout), AV_OPT_TYPE_INT, {.i64 = -1}, INT_MIN, INT_MAX, DEC 
> },
> -{ "timeout", "set timeout (in microseconds) of socket TCP I/O 
> operations", OFFSET(stimeout), AV_OPT_TYPE_INT, {.i64 = 0}, INT_MIN, INT_MAX, 
> DEC },
> +{ "timeout", "set timeout (in microseconds) of socket I/O operations", 
> OFFSET(stimeout), AV_OPT_TYPE_INT64, {.i64 = 0}, INT_MIN, INT64_MAX, DEC },
>  COMMON_OPTS(),
>  { "user_agent", "override User-Agent header", OFFSET(user_agent), 
> AV_OPT_TYPE_STRING, {.str = LIBAVFORMAT_IDENT}, 0, 0, DEC },
>  { NULL },
> @@ -104,7 +104,7 @@ static const AVOption sdp_options[] = {
>  RTSP_FLAG_OPTS("sdp_flags", "SDP flags"),
>  { "custom_io", "use custom I/O", 0, AV_OPT_TYPE_CONST, {.i64 = 
> RTSP_FLAG_CUSTOM_IO}, 0, 0, DEC, "rtsp_flags" },
>  { "rtcp_to_source", "send RTCP packets to the source address of received 
> packets", 0, AV_OPT_TYPE_CONST, {.i64 = RTSP_FLAG_RTCP_TO_SOURCE}, 0, 0, DEC, 
> "rtsp_flags" },
> -{ "listen_timeout", "set maximum timeout (in seconds) to wait for 
> incoming connections", OFFSET(initial_timeout), AV_OPT_TYPE_INT, {.i64 = 
> READ_PACKET_TIMEOUT_S}, INT_MIN, INT_MAX, DEC },
> +{ "listen_timeout", "set maximum timeout (in seconds) to wait for 
> incoming connections", OFFSET(stimeout), AV_OPT_TYPE_DURATION, {.i64 = 
> READ_PACKET_TIMEOUT_S*100}, INT_MIN, INT64_MAX, DEC },
>  RTSP_MEDIATYPE_OPTS("allowed_media_types", "set media types to accept 
> from the server"),
>  COMMON_OPTS(),
>  { NULL },
> @@ -112,7 +112,7 @@ static const AVOption sdp_options[] = {
>  
>  static const AVOption rtp_options[] = {
>  RTSP_FLAG_OPTS("rtp_flags", "set RTP flags"),
> -{ "listen_timeout", "set maximum timeout (in seconds) to wait for 
> incoming connections", OFFSET(initial_timeout), AV_OPT_TYPE_INT, {.i64 = 
> READ_PACKET_TIMEOUT_S}, INT_MIN, INT_MAX, DEC },
> +{ "listen_timeout", "set maximum timeout (in seconds) to wait for 
> incoming connections", OFFSET(stimeout), AV_OPT_TYPE_DURATION, {.i64 = 
> READ_PACKET_TIMEOUT_S*100}, INT_MIN, INT64_MAX, DEC },
>  RTSP_MEDIATYPE_OPTS("allowed_media_types", "set media types to accept 
> from the server"),
>  COMMON_OPTS(),
>  { NULL },
> @@ -1874,7 +1874,7 @@ redirect:
>  /* open the tcp connection */
>  ff_url_join(tcpname, sizeof(tcpname), lower_rtsp_proto, NULL,
>  host, port,
> -"?timeout=%d", rt->stimeout);
> +"?timeout=%"PRId64, rt->stimeout);
>  if ((ret = ffurl_open_whitelist(>rtsp_hd, tcpname, 
> AVIO_FLAG_READ_WRITE,
> >interrupt_callback, NULL, s->protocol_whitelist, 
> s->protocol_blacklist, NULL)) < 0) {
>  err = ret;
> @@ -2027,7 +2027,7 @@ static int udp_read_packet(AVFormatContext *s, 
> RTSPStream **prtsp_st,
>  int n, i, ret;
>  struct pollfd *p = rt->p;
>  int *fds = NULL, fdsnum, fdsidx;
> -int runs = rt->initial_timeout * 1000LL / POLLING_TIME;
> +int64_t runs = rt->stimeout / POLLING_TIME / 1000;
>  
>  if (!p) 

[FFmpeg-devel] [PATCH v2] avformat/rtsp: fix timeout option

[Andriy Gelman]

From: Andriy Gelman 

92c40ef882be115e72d2aa02f9032b7ce88f8537 added a listen_timeout option
for sdp. This allowed a user to set variable timeout in sdp which was
originally hard coded to 10 seconds.

The commit used the initial_timeout variable to store the value.
However, in rtsp this variable is also used to infer a "listen" mode. Thus,
the timeout value could not be set when connecting to an rtsp server.
The default value of -1 would also result in a 100ms total timeout.

This was attempted to be fixed in c8101aabee654f6d147a4d89f77fa73e18908610,
which changed the meaning of initial_timeout = -1 to be an infinite
timeout in rtsp. However, it did not address the issue that the timeout could
still not be set. Being able to set the timeout is useful because it
allows to automatically reconfigure from a udp to tcp connection in the
lower transport.

In this commit, this is fixed by using the stimeout variable/option to
set the timeout in rtsp.

Signed-off-by: Andriy Gelman 
---

Patch didn't apply cleanly anymore. Rebased to master

 libavformat/rtsp.c | 12 ++--
 libavformat/rtsp.h |  2 +-
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
index 9f509a229f..d4ab8f28bf 100644
--- a/libavformat/rtsp.c
+++ b/libavformat/rtsp.c
@@ -94,7 +94,7 @@ const AVOption ff_rtsp_options[] = {
 { "min_port", "set minimum local UDP port", OFFSET(rtp_port_min), 
AV_OPT_TYPE_INT, {.i64 = RTSP_RTP_PORT_MIN}, 0, 65535, DEC|ENC },
 { "max_port", "set maximum local UDP port", OFFSET(rtp_port_max), 
AV_OPT_TYPE_INT, {.i64 = RTSP_RTP_PORT_MAX}, 0, 65535, DEC|ENC },
 { "listen_timeout", "set maximum timeout (in seconds) to wait for incoming 
connections (-1 is infinite, imply flag listen)", OFFSET(initial_timeout), 
AV_OPT_TYPE_INT, {.i64 = -1}, INT_MIN, INT_MAX, DEC },
-{ "timeout", "set timeout (in microseconds) of socket TCP I/O operations", 
OFFSET(stimeout), AV_OPT_TYPE_INT, {.i64 = 0}, INT_MIN, INT_MAX, DEC },
+{ "timeout", "set timeout (in microseconds) of socket I/O operations", 
OFFSET(stimeout), AV_OPT_TYPE_INT64, {.i64 = 0}, INT_MIN, INT64_MAX, DEC },
 COMMON_OPTS(),
 { "user_agent", "override User-Agent header", OFFSET(user_agent), 
AV_OPT_TYPE_STRING, {.str = LIBAVFORMAT_IDENT}, 0, 0, DEC },
 { NULL },
@@ -104,7 +104,7 @@ static const AVOption sdp_options[] = {
 RTSP_FLAG_OPTS("sdp_flags", "SDP flags"),
 { "custom_io", "use custom I/O", 0, AV_OPT_TYPE_CONST, {.i64 = 
RTSP_FLAG_CUSTOM_IO}, 0, 0, DEC, "rtsp_flags" },
 { "rtcp_to_source", "send RTCP packets to the source address of received 
packets", 0, AV_OPT_TYPE_CONST, {.i64 = RTSP_FLAG_RTCP_TO_SOURCE}, 0, 0, DEC, 
"rtsp_flags" },
-{ "listen_timeout", "set maximum timeout (in seconds) to wait for incoming 
connections", OFFSET(initial_timeout), AV_OPT_TYPE_INT, {.i64 = 
READ_PACKET_TIMEOUT_S}, INT_MIN, INT_MAX, DEC },
+{ "listen_timeout", "set maximum timeout (in seconds) to wait for incoming 
connections", OFFSET(stimeout), AV_OPT_TYPE_DURATION, {.i64 = 
READ_PACKET_TIMEOUT_S*100}, INT_MIN, INT64_MAX, DEC },
 RTSP_MEDIATYPE_OPTS("allowed_media_types", "set media types to accept from 
the server"),
 COMMON_OPTS(),
 { NULL },
@@ -112,7 +112,7 @@ static const AVOption sdp_options[] = {
 
 static const AVOption rtp_options[] = {
 RTSP_FLAG_OPTS("rtp_flags", "set RTP flags"),
-{ "listen_timeout", "set maximum timeout (in seconds) to wait for incoming 
connections", OFFSET(initial_timeout), AV_OPT_TYPE_INT, {.i64 = 
READ_PACKET_TIMEOUT_S}, INT_MIN, INT_MAX, DEC },
+{ "listen_timeout", "set maximum timeout (in seconds) to wait for incoming 
connections", OFFSET(stimeout), AV_OPT_TYPE_DURATION, {.i64 = 
READ_PACKET_TIMEOUT_S*100}, INT_MIN, INT64_MAX, DEC },
 RTSP_MEDIATYPE_OPTS("allowed_media_types", "set media types to accept from 
the server"),
 COMMON_OPTS(),
 { NULL },
@@ -1874,7 +1874,7 @@ redirect:
 /* open the tcp connection */
 ff_url_join(tcpname, sizeof(tcpname), lower_rtsp_proto, NULL,
 host, port,
-"?timeout=%d", rt->stimeout);
+"?timeout=%"PRId64, rt->stimeout);
 if ((ret = ffurl_open_whitelist(>rtsp_hd, tcpname, 
AVIO_FLAG_READ_WRITE,
>interrupt_callback, NULL, s->protocol_whitelist, 
s->protocol_blacklist, NULL)) < 0) {
 err = ret;
@@ -2027,7 +2027,7 @@ static int udp_read_packet(AVFormatContext *s, RTSPStream 
**prtsp_st,
 int n, i, ret;
 struct pollfd *p = rt->p;
 int *fds = NULL, fdsnum, fdsidx;
-int runs = rt->initial_timeout * 1000LL / POLLING_TIME;
+int64_t runs = rt->stimeout / POLLING_TIME / 1000;
 
 if (!p) {
 p = rt->p = av_malloc_array(2 * rt->nb_rtsp_streams + 1, sizeof(*p));
@@ -2088,7 +2088,7 @@ static int udp_read_packet(AVFormatContext *s, RTSPStream 
**prtsp_st,
 }
 }
 #endif
-} else if (n == 0 && 

[FFmpeg-devel] [PATCH v2] avformat/rtsp: fix timeout option

[Andriy Gelman]

From: Andriy Gelman 

92c40ef882be115e72d2aa02f9032b7ce88f8537 added a listen_timeout option
for sdp. This allowed a user to set variable timeout in sdp which was
originally hard coded to 10 seconds.

The commit used the initial_timeout variable to store the value.
However, in rtsp this variable is also used to infer a "listen" mode. Thus,
the timeout value could not be set when connecting to an rtsp server.
The default value of -1 would also result in a 100ms total timeout.

This was attempted to be fixed in c8101aabee654f6d147a4d89f77fa73e18908610,
which changed the meaning of initial_timeout = -1 to be an infinite
timeout in rtsp. However, it did not address the issue that the timeout could
still not be set. Being able to set the timeout is useful because it
allows to automatically reconfigure from a udp to tcp connection in the
lower transport.

In this commit, this is fixed by using the stimeout variable/option to
set the timeout in rtsp.

Signed-off-by: Andriy Gelman 
---

Resending this patch with a small change to docs. I removed TCP because 
-stimeout
would also now apply to lower transport where it could be UDP.

 doc/protocols.texi |  2 +-
 libavformat/rtsp.c | 14 +++---
 libavformat/rtsp.h |  2 +-
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/doc/protocols.texi b/doc/protocols.texi
index d3f6cbefcf..8593e17d57 100644
--- a/doc/protocols.texi
+++ b/doc/protocols.texi
@@ -1174,7 +1174,7 @@ A value of -1 means infinite (default). This option 
implies the
 Set number of packets to buffer for handling of reordered packets.
 
 @item stimeout
-Set socket TCP I/O timeout in microseconds.
+Set socket I/O timeout in microseconds.
 
 @item user-agent
 Override User-Agent header. If not specified, it defaults to the
diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
index 25bdf475b3..fff53b0a01 100644
--- a/libavformat/rtsp.c
+++ b/libavformat/rtsp.c
@@ -96,9 +96,9 @@ const AVOption ff_rtsp_options[] = {
 { "listen_timeout", "set maximum timeout (in seconds) to wait for incoming 
connections (-1 is infinite, imply flag listen)", OFFSET(initial_timeout), 
AV_OPT_TYPE_INT, {.i64 = -1}, INT_MIN, INT_MAX, DEC },
 #if FF_API_OLD_RTSP_OPTIONS
 { "timeout", "set maximum timeout (in seconds) to wait for incoming 
connections (-1 is infinite, imply flag listen) (deprecated, use 
listen_timeout)", OFFSET(initial_timeout), AV_OPT_TYPE_INT, {.i64 = -1}, 
INT_MIN, INT_MAX, DEC|AV_OPT_FLAG_DEPRECATED },
-{ "stimeout", "set timeout (in microseconds) of socket TCP I/O 
operations", OFFSET(stimeout), AV_OPT_TYPE_INT, {.i64 = 0}, INT_MIN, INT_MAX, 
DEC },
+{ "stimeout", "set timeout (in microseconds) of socket I/O operations", 
OFFSET(stimeout), AV_OPT_TYPE_INT64, {.i64 = 0}, INT_MIN, INT64_MAX, DEC },
 #else
-{ "timeout", "set timeout (in microseconds) of socket TCP I/O operations", 
OFFSET(stimeout), AV_OPT_TYPE_INT, {.i64 = 0}, INT_MIN, INT_MAX, DEC },
+{ "timeout", "set timeout (in microseconds) of socket I/O operations", 
OFFSET(stimeout), AV_OPT_TYPE_INT64, {.i64 = 0}, INT_MIN, INT64_MAX, DEC },
 #endif
 COMMON_OPTS(),
 { "user_agent", "override User-Agent header", OFFSET(user_agent), 
AV_OPT_TYPE_STRING, {.str = LIBAVFORMAT_IDENT}, 0, 0, DEC },
@@ -112,7 +112,7 @@ static const AVOption sdp_options[] = {
 RTSP_FLAG_OPTS("sdp_flags", "SDP flags"),
 { "custom_io", "use custom I/O", 0, AV_OPT_TYPE_CONST, {.i64 = 
RTSP_FLAG_CUSTOM_IO}, 0, 0, DEC, "rtsp_flags" },
 { "rtcp_to_source", "send RTCP packets to the source address of received 
packets", 0, AV_OPT_TYPE_CONST, {.i64 = RTSP_FLAG_RTCP_TO_SOURCE}, 0, 0, DEC, 
"rtsp_flags" },
-{ "listen_timeout", "set maximum timeout (in seconds) to wait for incoming 
connections", OFFSET(initial_timeout), AV_OPT_TYPE_INT, {.i64 = 
READ_PACKET_TIMEOUT_S}, INT_MIN, INT_MAX, DEC },
+{ "listen_timeout", "set maximum timeout (in seconds) to wait for incoming 
connections", OFFSET(stimeout), AV_OPT_TYPE_DURATION, {.i64 = 
READ_PACKET_TIMEOUT_S*100}, INT_MIN, INT64_MAX, DEC },
 RTSP_MEDIATYPE_OPTS("allowed_media_types", "set media types to accept from 
the server"),
 COMMON_OPTS(),
 { NULL },
@@ -120,7 +120,7 @@ static const AVOption sdp_options[] = {
 
 static const AVOption rtp_options[] = {
 RTSP_FLAG_OPTS("rtp_flags", "set RTP flags"),
-{ "listen_timeout", "set maximum timeout (in seconds) to wait for incoming 
connections", OFFSET(initial_timeout), AV_OPT_TYPE_INT, {.i64 = 
READ_PACKET_TIMEOUT_S}, INT_MIN, INT_MAX, DEC },
+{ "listen_timeout", "set maximum timeout (in seconds) to wait for incoming 
connections", OFFSET(stimeout), AV_OPT_TYPE_DURATION, {.i64 = 
READ_PACKET_TIMEOUT_S*100}, INT_MIN, INT64_MAX, DEC },
 RTSP_MEDIATYPE_OPTS("allowed_media_types", "set media types to accept from 
the server"),
 COMMON_OPTS(),
 { NULL },
@@ -1882,7 +1882,7 @@ redirect:
 /* open the tcp connection */
 ff_url_join(tcpname, sizeof(tcpname), lower_rtsp_proto,