Re: [FFmpeg-devel] [PATCH] avformat/rtmpproto: Pass rw_timeout to underlying transport protocol

[Zhao Zhili <quinkblack@foxmail.com>]

> On Nov 22, 2023, at 17:33, Martin Storsjö <martin@martin.st> wrote:
> 
> On Wed, 22 Nov 2023, Zhao Zhili wrote:
> 
>> 
>> 
>>> On Nov 15, 2023, at 21:24, Zhao Zhili <quinkblack@foxmail.com> wrote:
>>> 
>>> From: Zhao Zhili <zhilizhao@tencent.com>
>>> 
>>> Signed-off-by: Zhao Zhili <zhilizhao@tencent.com>
>>> ---
>>> libavformat/rtmpproto.c | 3 +++
>>> 1 file changed, 3 insertions(+)
>>> 
>>> diff --git a/libavformat/rtmpproto.c b/libavformat/rtmpproto.c
>>> index 98718bc6da..a0c6195eb2 100644
>>> --- a/libavformat/rtmpproto.c
>>> +++ b/libavformat/rtmpproto.c
>>> @@ -2635,6 +2635,9 @@ static int rtmp_open(URLContext *s, const char *uri, int flags, AVDictionary **o
>>> 
>>>    if (rt->listen_timeout > 0)
>>>        rt->listen = 1;
>>> +    /* Pass rw_timeout to underlying transport protocol */
>>> +    if (s->rw_timeout > 0)
>>> +        av_dict_set_int(opts, "rw_timeout", s->rw_timeout, 0);
>> 
>> OK, I made a mistake. Since ffurl_open_whitelist copy from parent automatically,
>> this is a NOP. Will revert this commit.
> 
> Thanks! I was just starting to look into this, to confirm this as well, but it's good if you've come to the same conclusion already.
> 
> It's interesting how we end up with similar changes for this multiple times, see https://ffmpeg.org/pipermail/ffmpeg-devel/2023-July/312362.html for another similar patch a few months ago, when it should be working already.
> 
> The fact that so many protocols have similar but vaguely different timeout options, each defined as a per-protocol private option, is a bit of a mess; that's why this approach, of actually sharing the same variable through the URLContext hierarchy, tries to avoid that to some extent.

I have taken code from ftp.c as reference:
```
        if (s->rw_timeout != -1) {
            av_dict_set_int(&opts, "timeout", s->rw_timeout, 0);
        } /* if option is not given, don't pass it and let tcp use its own default */
```

Now it’s obvious that code comes before 
```
commit fab8156b2f30666adabe227b3d7712fd193873b1
Author: Martin Storsjö
Date:   Sat Feb 28 02:00:50 2015 +0200

    avio: Copy URLContext generic options into child URLContexts
```

Should we remove that part from ftp.c?

It worth to note that there is a small difference between rw_timeout and timeout
option for TCP:

1. timeout apply to accept, read and write
2. rw_timeout apply to read and write, but not accept.

What do you think?

> 
> // 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".