[FFmpeg-devel] [PATCH] rtsp: rename certain options after a deprecation period

[wm4]

On Thu, 25 Jan 2018 15:12:27 -0300
James Almer <jamrial at gmail.com> wrote:

> On 1/25/2018 3:00 PM, wm4 wrote:
> > The names inherently clash with the meanings of the HTTP libavformat
> > protocol options. Rename them after a deprecation period to make them
> > compatible with the HTTP ones.
> > ---
> > I see no better way that wouldn't require more effort than justified.
> > The incompatible semantics of the "timeout" option while still clashing
> > with the HTTP one caused major problems to me as API user, and I'm
> > hoping that this will solve itself in 2 years.
> > ---
> >  doc/APIchanges        | 5 +++++
> >  libavformat/rtsp.c    | 9 +++++++++
> >  libavformat/version.h | 5 ++++-
> >  3 files changed, 18 insertions(+), 1 deletion(-)
> > 
> > diff --git a/doc/APIchanges b/doc/APIchanges
> > index 59e3b20c08..bf8664c799 100644
> > --- a/doc/APIchanges
> > +++ b/doc/APIchanges
> > @@ -15,6 +15,11 @@ libavutil:     2017-10-21
> >  
> >  API changes, most recent first:
> >  
> > +2018-01-xx - xxxxxxx - lavf 58.7.100 - avformat.h
> > +  Deprecate the current names of the RTSP "timeout", "stimeout", "user-agent"
> > +  options. Once the deprecation is over, "timeout" will be renamed to
> > +  "listen_timeout", "stimeout" to "timeout", and "user-agent" to "user_agent".
> > +
> >  2018-01-xx - xxxxxxx - lavf 58.6.100 - avformat.h
> >    Add AVFMTCTX_UNSEEKABLE (for HLS demuxer).
> >  
> > diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
> > index cf7cdb2f2b..bed5f1ea11 100644
> > --- a/libavformat/rtsp.c
> > +++ b/libavformat/rtsp.c
> > @@ -93,10 +93,19 @@ const AVOption ff_rtsp_options[] = {
> >      RTSP_MEDIATYPE_OPTS("allowed_media_types", "set media types to accept from the server"),
> >      { "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 },
> > +#if FF_API_OLD_RTSP_OPTIONS
> >      { "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 },
> >      { "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
> > +    { "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 },
> > +#endif
> >      COMMON_OPTS(),
> > +#if FF_API_OLD_RTSP_OPTIONS
> >      { "user-agent", "override User-Agent header", OFFSET(user_agent), AV_OPT_TYPE_STRING, {.str = LIBAVFORMAT_IDENT}, 0, 0, DEC },
> > +#else
> > +    { "user_agent", "override User-Agent header", OFFSET(user_agent), AV_OPT_TYPE_STRING, {.str = LIBAVFORMAT_IDENT}, 0, 0, DEC },
> > +#endif  
> 
> Wouldn't it be better to add the new one right now instead of right
> after the deprecated one is removed? People should start moving to the
> new one before that happens, rather than right when it happens.
> That way you can also add "(deprecated, use user_agent instead)" to the
> description of the old one.

Probably, but it can't be solved like this for the timeout options. (If
the patch is accepted I can always expose the user_agent option and
edit the description of user-agent to mention it as deprecated.)