From: "Martin Storsjö" <martin@martin.st>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH] avformat/rtmpproto: Pass rw_timeout to underlying transport protocol
Date: Wed, 22 Nov 2023 11:52:26 +0200 (EET)
Message-ID: <784927ad-674e-adf3-fee0-453495dd132f@martin.st> (raw)
In-Reply-To: <tencent_FF4CF3ED737B35D3490EEEBDA559F31F0805@qq.com>
On Wed, 22 Nov 2023, Zhao Zhili wrote:
> 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 */
> ```
Ah, I see!
> 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?
Hmm, I'm not entirely sure.
I wonder if it would be best to change the tcp protocol to apply the
rw_timeout option for connect timeout as well (open_timeout). With so many
different options on different levels, that all do almost the same but
with subtle differences (as for which cases it applies to), it's really
hard to use them properly.
It's kinda problematic if a user searches for a way to make sure that the
protocol always would abort if things stall, at any point, for more than X
seconds, the user finds and picks one option, and it only applies in some
case but not others.
But changing existing behaviours can of course always break someone as
well... Although I haven't looked through all the protocols and their
individual uses of options for this, so I'm not sure how much potential
breakage there would be if we'd try to unify it.
Then again, I also see that it's quite possible for someone to want to set
one kind of timeout for open but a different one for stalling once the
connection has been opened.
So overall, I don't have a firm settled opinion on this matter right now.
But anything to unify the code, reducing the amount of per-protocol
setting of almost similar but slightly different options, would be good.
And ideally using less per-protocol code and more of the common shared
framework within e.g. URLContext.
// 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".
prev parent reply other threads:[~2023-11-22 9:52 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-15 13:24 Zhao Zhili
2023-11-22 9:20 ` Zhao Zhili
2023-11-22 9:33 ` Martin Storsjö
2023-11-22 9:40 ` Zhao Zhili
2023-11-22 9:45 ` Zhao Zhili
2023-11-22 9:52 ` Martin Storsjö [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=784927ad-674e-adf3-fee0-453495dd132f@martin.st \
--to=martin@martin.st \
--cc=ffmpeg-devel@ffmpeg.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
This inbox may be cloned and mirrored by anyone:
git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git
# If you have public-inbox 1.1+ installed, you may
# initialize and index your mirror using the following commands:
public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
ffmpegdev@gitmailbox.com
public-inbox-index ffmpegdev
Example config snippet for mirrors.
AGPL code for this site: git clone https://public-inbox.org/public-inbox.git