On Sat, 10 Mar 2018 20:37:20 +0100 Nicolas George <geo...@nsup.org> wrote:
> A timeout is a duration of time, therefore the correct type for it > is AV_OPT_TYPE_DURATION. It has the benefit of offering a better > user interface, with units specification. > Unfortunately, ff46124b0df17a1d35249e09ae8eae9a61f16e04 was pushed > before that mistake could be corrected. > > Signed-off-by: Nicolas George <geo...@nsup.org> > --- > libavformat/rtsp.c | 5 +++-- > libavformat/rtsp.h | 4 ++++ > 2 files changed, 7 insertions(+), 2 deletions(-) > > diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c > index ceb770a3a4..1fbdcfcedd 100644 > --- a/libavformat/rtsp.c > +++ b/libavformat/rtsp.c > @@ -98,7 +98,7 @@ const AVOption ff_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 }, > { "stimeout", "set timeout (in microseconds) of socket TCP I/O > operations", OFFSET(stimeout), AV_OPT_TYPE_INT, {.i64 = 0}, INT_MIN, INT_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 of socket TCP I/O operations", > OFFSET(stimeout), AV_OPT_TYPE_DURATION, {.i64 = 0}, 0, 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 }, > @@ -1818,7 +1818,8 @@ redirect: > /* open the tcp connection */ > ff_url_join(tcpname, sizeof(tcpname), lower_rtsp_proto, NULL, > host, port, > - "?timeout=%d", rt->stimeout); > + /* cast necessary until FF_API_OLD_RTSP_OPTIONS removed > */ > + "?timeout=%"PRId64, (int64_t)rt->stimeout); > if ((ret = ffurl_open_whitelist(&rt->rtsp_hd, tcpname, > AVIO_FLAG_READ_WRITE, > &s->interrupt_callback, NULL, s->protocol_whitelist, > s->protocol_blacklist, NULL)) < 0) { > err = ret; > diff --git a/libavformat/rtsp.h b/libavformat/rtsp.h > index 9a7f366b39..1524962e1b 100644 > --- a/libavformat/rtsp.h > +++ b/libavformat/rtsp.h > @@ -395,7 +395,11 @@ typedef struct RTSPState { > /** > * timeout of socket i/o operations. > */ > +#if FF_API_OLD_RTSP_OPTIONS > int stimeout; > +#else > + int64_t stimeout; > +#endif > > /** > * Size of RTP packet reordering queue. On IRC I was asked to explain my NACK: The whole point of ff46124b0df17a1d35249e09ae8eae9a61f16e04 was to harmonize the timeout options with that of other protocols. In particular, http/tcp (the most used network protocol of them all) uses an AV_OPT_TYPE_INT "timeout" option, using microseconds. AV_OPT_TYPE_DURATION parses the time in seconds, so this is incompatible. It's incompatible on both CLI and API, because the API usually requires you to pass all options as string in AVDictionary entries (this is how dispatching general options to underlying protocols work, and it's impossible to access the options for sub protocols directly). Thus your change negates the whole point of the previous change, which is why I'm rejecting it. Reverting others patches after the discussion of it was done and everything finalized isn't exactly how team work works either. You have 2 choices: - change all timeout options to AV_OPT_TYPE_DURATION (after a deprecation period) - drop the patch Also, your patch message implies I pushed the previous patch too early. This is not true - I waited long enough, and because there was a flamewar, certainly it can't be claimed that you didn't notice it, or didn't have enough time to do whatever. Especially considering you replied to the thread on the same day as I posted the patch, and I pushed the patch over a week later. So I'm politely asking you to stop making such implications. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel