[FFmpeg-devel] [PATCH] avformat/rtmpproto: forward rw_timeout to tcp proto

[FFmpeg-devel] [PATCH] avformat/rtmpproto: forward rw_timeout to tcp proto

[Timo Rothenpieler]

---
 libavformat/rtmpproto.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/libavformat/rtmpproto.c b/libavformat/rtmpproto.c
index f0ef223f05..a18cc78eac 100644
--- a/libavformat/rtmpproto.c
+++ b/libavformat/rtmpproto.c
@@ -127,6 +127,7 @@ typedef struct RTMPContext {
     int           nb_streamid;                ///< The next stream id to return on createStream calls
     double        duration;                   ///< Duration of the stream in seconds as returned by the server (only valid if non-zero)
     int           tcp_nodelay;                ///< Use TCP_NODELAY to disable Nagle's algorithm if set to 1
+    int           rw_timeout;                 ///< timeout of socket I/O operations
     char          username[50];
     char          password[50];
     char          auth_params[500];
@@ -2656,10 +2657,12 @@ static int rtmp_open(URLContext *s, const char *uri, int flags, AVDictionary **o
             port = RTMP_DEFAULT_PORT;
         if (rt->listen)
             ff_url_join(buf, sizeof(buf), "tcp", NULL, hostname, port,
-                        "?listen&listen_timeout=%d&tcp_nodelay=%d",
-                        rt->listen_timeout * 1000, rt->tcp_nodelay);
+                        "?listen&listen_timeout=%d&timeout=%d&tcp_nodelay=%d",
+                        rt->listen_timeout * 1000, rt->rw_timeout,
+                        rt->tcp_nodelay);
         else
-            ff_url_join(buf, sizeof(buf), "tcp", NULL, hostname, port, "?tcp_nodelay=%d", rt->tcp_nodelay);
+            ff_url_join(buf, sizeof(buf), "tcp", NULL, hostname, port,
+                        "?tcp_nodelay=%d&timeout=%d", rt->tcp_nodelay, rt->rw_timeout);
     }
 
 reconnect:
@@ -3120,6 +3123,7 @@ static const AVOption rtmp_options[] = {
     {"listen",      "Listen for incoming rtmp connections", OFFSET(listen), AV_OPT_TYPE_INT, {.i64 = 0}, INT_MIN, INT_MAX, DEC, "rtmp_listen" },
     {"tcp_nodelay", "Use TCP_NODELAY to disable Nagle's algorithm", OFFSET(tcp_nodelay), AV_OPT_TYPE_INT, {.i64 = 0}, 0, 1, DEC|ENC},
     {"timeout", "Maximum timeout (in seconds) to wait for incoming connections. -1 is infinite. Implies -rtmp_listen 1",  OFFSET(listen_timeout), AV_OPT_TYPE_INT, {.i64 = -1}, INT_MIN, INT_MAX, DEC, "rtmp_listen" },
+    {"rw_timeout", "Maximum timeout (in microseconds) to wait for socket i/o. -1 is infinite.",  OFFSET(rw_timeout), AV_OPT_TYPE_INT, {.i64 = -1}, INT_MIN, INT_MAX, DEC|ENC },
     { NULL },
 };
 
-- 
2.34.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avformat/rtmpproto: forward rw_timeout to tcp proto

[Martin Storsjö]

On Thu, 20 Jul 2023, Timo Rothenpieler wrote:

> ---
> libavformat/rtmpproto.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)

Hmm, I would have somewhat expected that rw_timeout should be honored here 
already...

Note that URLContext already has got a rw_timeout field and AVOption, so 
instead of adding a new option, it should be enough to just use the 
existing URLContext field.

But also, have a look at fab8156b2f30666adabe227b3d7712fd193873b1. When 
opening a chained URLContext, we pass in the parent one, and propagate any 
options from that URLContext into the child. So as long as the URLContext 
rw_timeout is set for the rtmp protocol, the nested tcp protocol also 
should be getting it already implicitly.

// Martin

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avformat/rtmpproto: forward rw_timeout to tcp proto

[Timo Rothenpieler]

On 20.07.2023 22:47, Martin Storsjö wrote:
> On Thu, 20 Jul 2023, Timo Rothenpieler wrote:
> 
>> ---
>> libavformat/rtmpproto.c | 10 +++++++---
>> 1 file changed, 7 insertions(+), 3 deletions(-)
> 
> Hmm, I would have somewhat expected that rw_timeout should be honored 
> here already...
> 
> Note that URLContext already has got a rw_timeout field and AVOption, so 
> instead of adding a new option, it should be enough to just use the 
> existing URLContext field.
> 
> But also, have a look at fab8156b2f30666adabe227b3d7712fd193873b1. When 
> opening a chained URLContext, we pass in the parent one, and propagate 
> any options from that URLContext into the child. So as long as the 
> URLContext rw_timeout is set for the rtmp protocol, the nested tcp 
> protocol also should be getting it already implicitly.

I'm not entirely sure how that works with in regards to which options 
are getting forwarded where.
But doesn't the "timeout" (in seconds, used as listen timeout) option 
from rtmpproto mask over the "timeout" option from tcp (in microseconds, 
used as rw/open timeout)?
Hence the "renaming" here, to unshadow that tcp.c option.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avformat/rtmpproto: forward rw_timeout to tcp proto

[Martin Storsjö]

On Thu, 20 Jul 2023, Timo Rothenpieler wrote:

> On 20.07.2023 22:47, Martin Storsjö wrote:
>> On Thu, 20 Jul 2023, Timo Rothenpieler wrote:
>> 
>>> ---
>>> libavformat/rtmpproto.c | 10 +++++++---
>>> 1 file changed, 7 insertions(+), 3 deletions(-)
>> 
>> Hmm, I would have somewhat expected that rw_timeout should be honored 
>> here already...
>> 
>> Note that URLContext already has got a rw_timeout field and AVOption, so 
>> instead of adding a new option, it should be enough to just use the 
>> existing URLContext field.
>> 
>> But also, have a look at fab8156b2f30666adabe227b3d7712fd193873b1. When 
>> opening a chained URLContext, we pass in the parent one, and propagate 
>> any options from that URLContext into the child. So as long as the 
>> URLContext rw_timeout is set for the rtmp protocol, the nested tcp 
>> protocol also should be getting it already implicitly.
>
> I'm not entirely sure how that works with in regards to which options 
> are getting forwarded where.
> But doesn't the "timeout" (in seconds, used as listen timeout) option 
> from rtmpproto mask over the "timeout" option from tcp (in microseconds, 
> used as rw/open timeout)?
> Hence the "renaming" here, to unshadow that tcp.c option.

Hmm, geez we have many similar-looking and randomly named timeout options 
here...

The av_opt_copy() call in ffurl_open_whitelist() should only iterate over 
the options within the URLContext itself, not recurse into the protocol 
private options (as those probably don't match across protocols anyway). 
So I wouldn't think that the tcp "timeout" option ends up implicitly set 
from the rtmpproto "timeout" (listen_timeout) option.

I didn't try running and observing it right now though.

// Martin
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avformat/rtmpproto: forward rw_timeout to tcp proto

[Timo Rothenpieler]

On 20.07.2023 23:09, Martin Storsjö wrote:
> Hmm, geez we have many similar-looking and randomly named timeout 
> options here...
> 
> The av_opt_copy() call in ffurl_open_whitelist() should only iterate 
> over the options within the URLContext itself, not recurse into the 
> protocol private options (as those probably don't match across protocols 
> anyway). So I wouldn't think that the tcp "timeout" option ends up 
> implicitly set from the rtmpproto "timeout" (listen_timeout) option.

My goal here is to specifically be able to set the i/o timeout on the 
tcp side, since I just had an ffmpeg rtmp process wait for more data 
indefinitely after a network hickup, even though the connection was long 
dead.

I just discovered that the URLContext has global options, and rw_timeout 
is one of them.
So setting -rw_timeout should have already been possible, I'll try if 
that ends up in the right place.

> I didn't try running and observing it right now though.
> 
> // Martin
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".