diff mbox

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

Message ID 20180125180043.12502-1-nfxjfg@googlemail.com
State Accepted
Headers show

Commit Message

wm4 Jan. 25, 2018, 6 p.m. UTC
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(-)

Comments

wm4 Jan. 25, 2018, 6:16 p.m. UTC | #1
On Thu, 25 Jan 2018 15:12:27 -0300
James Almer <jamrial@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.)
Michael Niedermayer Jan. 25, 2018, 7:46 p.m. UTC | #2
On Thu, Jan 25, 2018 at 07:00:43PM +0100, 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(-)

Make sure all newly added options are in standard SI units
that is seconds (not micro seconds for example)

thx

[...]
wm4 Jan. 25, 2018, 7:54 p.m. UTC | #3
On Thu, 25 Jan 2018 20:46:13 +0100
Michael Niedermayer <michael@niedermayer.cc> wrote:

> On Thu, Jan 25, 2018 at 07:00:43PM +0100, 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(-)  
> 
> Make sure all newly added options are in standard SI units
> that is seconds (not micro seconds for example)

1. you can not use seconds if the option type is int because that would
   not be fine grained enough
2. HTTP already uses microseconds and the whole point is making this
   compatible with the HTTP impl. (as it establishes sort of a standard
   being the most used protocol other than file)
3. Microsecond actually counts as SI unit
4. I'm not going to change the HTTP impl. as that would be a much
   larger and intrusive change and would not solve the RTSP problem
   either
Michael Niedermayer Jan. 25, 2018, 8:17 p.m. UTC | #4
On Thu, Jan 25, 2018 at 08:54:51PM +0100, wm4 wrote:
> On Thu, 25 Jan 2018 20:46:13 +0100
> Michael Niedermayer <michael@niedermayer.cc> wrote:
> 
> > On Thu, Jan 25, 2018 at 07:00:43PM +0100, 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(-)  
> > 
> > Make sure all newly added options are in standard SI units
> > that is seconds (not micro seconds for example)
> 
> 1. you can not use seconds if the option type is int because that would
>    not be fine grained enough
> 2. HTTP already uses microseconds and the whole point is making this
>    compatible with the HTTP impl. (as it establishes sort of a standard
>    being the most used protocol other than file)
> 3. Microsecond actually counts as SI unit
> 4. I'm not going to change the HTTP impl. as that would be a much
>    larger and intrusive change and would not solve the RTSP problem
>    either

if the user specifies "-whatever_timeout 500m" That should consistently be
interpreted as 500 milli seconds.
All new code should follow this consistently

The user always provides a string never an integer. That is parsed, it can
be parsed into a double representing seconds, or an integer representing 
micro/nano/milli/whatever.

If you want to use an integer there are many ways to achive this, adding
a
AV_OPT_TYPE_TIMEOUT with int64 in nano seconds would be one. Using a
double in seconds would be much easier though

If you do not want to change http, you can leave that to someone else
iam not asking you to do any extra work, just that we move toward
having timeouts handled consistently

thanks

[...]
Nicolas George Jan. 25, 2018, 8:20 p.m. UTC | #5
Michael Niedermayer (2018-01-25):
> If you want to use an integer there are many ways to achive this,
> adding a AV_OPT_TYPE_TIMEOUT with int64 in nano seconds would be one.
> Using a double in seconds would be much easier though

We already have AV_OPT_TYPE_DURATION, in microseconds rather than
nanoseconds, but with all the good properties.

I fully agree with the points you make.

Regards,
wm4 Jan. 25, 2018, 8:54 p.m. UTC | #6
On Thu, 25 Jan 2018 21:17:54 +0100
Michael Niedermayer <michael@niedermayer.cc> wrote:

> On Thu, Jan 25, 2018 at 08:54:51PM +0100, wm4 wrote:
> > On Thu, 25 Jan 2018 20:46:13 +0100
> > Michael Niedermayer <michael@niedermayer.cc> wrote:
> >   
> > > On Thu, Jan 25, 2018 at 07:00:43PM +0100, 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(-)    
> > > 
> > > Make sure all newly added options are in standard SI units
> > > that is seconds (not micro seconds for example)  
> > 
> > 1. you can not use seconds if the option type is int because that would
> >    not be fine grained enough
> > 2. HTTP already uses microseconds and the whole point is making this
> >    compatible with the HTTP impl. (as it establishes sort of a standard
> >    being the most used protocol other than file)
> > 3. Microsecond actually counts as SI unit
> > 4. I'm not going to change the HTTP impl. as that would be a much
> >    larger and intrusive change and would not solve the RTSP problem
> >    either  
> 
> if the user specifies "-whatever_timeout 500m" That should consistently be
> interpreted as 500 milli seconds.
> All new code should follow this consistently
> 
> The user always provides a string never an integer. That is parsed, it can
> be parsed into a double representing seconds, or an integer representing 
> micro/nano/milli/whatever.
> 
> If you want to use an integer there are many ways to achive this, adding
> a
> AV_OPT_TYPE_TIMEOUT with int64 in nano seconds would be one. Using a
> double in seconds would be much easier though
> 
> If you do not want to change http, you can leave that to someone else
> iam not asking you to do any extra work, just that we move toward
> having timeouts handled consistently
> 

Clearly you're trying to bikeshed me here. I'll just ignore this
instead of wasting my time on you.
Carl Eugen Hoyos Jan. 25, 2018, 9:41 p.m. UTC | #7
2018-01-25 21:54 GMT+01:00 wm4 <nfxjfg@googlemail.com>:

> Clearly you're trying to bikeshed me here. I'll just ignore this
> instead of wasting my time on you.

Is there really no hope that you go away for good?

Carl Eugen
wm4 Jan. 25, 2018, 10:04 p.m. UTC | #8
On Thu, 25 Jan 2018 22:41:48 +0100
Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:

> 2018-01-25 21:54 GMT+01:00 wm4 <nfxjfg@googlemail.com>:
> 
> > Clearly you're trying to bikeshed me here. I'll just ignore this
> > instead of wasting my time on you.  
> 
> Is there really no hope that you go away for good?

Oh not you again.

Is there any hope that you will?
Carl Eugen Hoyos Jan. 25, 2018, 10:08 p.m. UTC | #9
2018-01-25 23:04 GMT+01:00 wm4 <nfxjfg@googlemail.com>:
> On Thu, 25 Jan 2018 22:41:48 +0100
> Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
>
>> 2018-01-25 21:54 GMT+01:00 wm4 <nfxjfg@googlemail.com>:
>>
>> > Clearly you're trying to bikeshed me here. I'll just ignore this
>> > instead of wasting my time on you.
>>
>> Is there really no hope that you go away for good?
>
> Oh not you again.

Isn't it more like "you again"?

This is the second time within a few hours that you have
without any imaginable reason attacked another developer
(that it happened both times without any action from the
mailing list admins is just a detail).
Some people here argue that you are simply ill, I tend to
disagree.

Carl Eugen
wm4 Jan. 25, 2018, 10:13 p.m. UTC | #10
On Thu, 25 Jan 2018 23:08:21 +0100
Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:

> 2018-01-25 23:04 GMT+01:00 wm4 <nfxjfg@googlemail.com>:
> > On Thu, 25 Jan 2018 22:41:48 +0100
> > Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
> >  
> >> 2018-01-25 21:54 GMT+01:00 wm4 <nfxjfg@googlemail.com>:
> >>  
> >> > Clearly you're trying to bikeshed me here. I'll just ignore this
> >> > instead of wasting my time on you.  
> >>
> >> Is there really no hope that you go away for good?  
> >
> > Oh not you again.  
> 
> Isn't it more like "you again"?
> 
> This is the second time within a few hours that you have
> without any imaginable reason attacked another developer
> (that it happened both times without any action from the
> mailing list admins is just a detail).
> Some people here argue that you are simply ill, I tend to
> disagree.
> 

Please cease your childish and unprofessional behavior and in particular
stop outright harassing me into leaving.

Thank you.
Carl Eugen Hoyos Jan. 25, 2018, 10:35 p.m. UTC | #11
2018-01-25 23:13 GMT+01:00 wm4 <nfxjfg@googlemail.com>:
> On Thu, 25 Jan 2018 23:08:21 +0100
> Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
>
>> 2018-01-25 23:04 GMT+01:00 wm4 <nfxjfg@googlemail.com>:
>> > On Thu, 25 Jan 2018 22:41:48 +0100
>> > Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
>> >
>> >> 2018-01-25 21:54 GMT+01:00 wm4 <nfxjfg@googlemail.com>:
>> >>
>> >> > Clearly you're trying to bikeshed me here. I'll just ignore this
>> >> > instead of wasting my time on you.
>> >>
>> >> Is there really no hope that you go away for good?
>> >
>> > Oh not you again.
>>
>> Isn't it more like "you again"?
>>
>> This is the second time within a few hours that you have
>> without any imaginable reason attacked another developer
>> (that it happened both times without any action from the
>> mailing list admins is just a detail).
>> Some people here argue that you are simply ill, I tend to
>> disagree.
>>
>
> Please cease your childish

(I have never understood this insult, sorry...)

> and unprofessional behavior

Hmm.
I am not working "professionally" on FFmpeg, are you?

> and in particular stop outright harassing me into leaving.

Yes, I would very much prefer if I could stop;-)

Carl Eugen
Hendrik Leppkes Jan. 25, 2018, 11 p.m. UTC | #12
On Thu, Jan 25, 2018 at 11:35 PM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
>> and in particular stop outright harassing me into leaving.
>
> Yes, I would very much prefer if I could stop;-)
>

So you openly admit to harassing a fellow developer, and even more so
to your plans to not stop?
If this isn't grounds to remove someone from the project, I don't know what is.

- Hendrik
Carl Eugen Hoyos Jan. 25, 2018, 11:21 p.m. UTC | #13
2018-01-26 0:00 GMT+01:00 Hendrik Leppkes <h.leppkes@gmail.com>:
> On Thu, Jan 25, 2018 at 11:35 PM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
>>> and in particular stop outright harassing me into leaving.
>>
>> Yes, I would very much prefer if I could stop;-)
>
> So you openly admit to harassing a fellow developer, and
> even more so to your plans to not stop?

I had the feeling he was harassing Michael several times in the
last hours, do you disagree?

Carl Eugen
Hendrik Leppkes Jan. 25, 2018, 11:27 p.m. UTC | #14
On Fri, Jan 26, 2018 at 12:21 AM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
> 2018-01-26 0:00 GMT+01:00 Hendrik Leppkes <h.leppkes@gmail.com>:
>> On Thu, Jan 25, 2018 at 11:35 PM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
>>>> and in particular stop outright harassing me into leaving.
>>>
>>> Yes, I would very much prefer if I could stop;-)
>>
>> So you openly admit to harassing a fellow developer, and
>> even more so to your plans to not stop?
>
> I had the feeling he was harassing Michael several times in the
> last hours, do you disagree?
>

This is about your actions.

- Hendrik
wm4 Jan. 25, 2018, 11:52 p.m. UTC | #15
On Fri, 26 Jan 2018 00:21:14 +0100
Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:

> 2018-01-26 0:00 GMT+01:00 Hendrik Leppkes <h.leppkes@gmail.com>:
> > On Thu, Jan 25, 2018 at 11:35 PM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:  
> >>> and in particular stop outright harassing me into leaving.  
> >>
> >> Yes, I would very much prefer if I could stop;-)  
> >
> > So you openly admit to harassing a fellow developer, and
> > even more so to your plans to not stop?  
> 
> I had the feeling he was harassing Michael several times in the
> last hours, do you disagree?

Well, going by your standards, harassment is just OK?

Anyway, if my reply to Michael was too harsh/unfriendly, I hereby
apologize (and I already wrote that on IRC too, where he lurks as
michaelni). I'd still like to ask him to explicitly state whether his
reply is a change request in context of a patch review, or just general
advice when he's suggesting something that would be disproportionally
more work and not solve the problem the original patch tried to solve.
This is sort of irritating because you never know what you're supposed
to do to get the patch OK'ed.

I'd also like to point out that it _did_ happen in the past that
Michael went off tangents in patch reviews and asked for unreasonable
extra work because of minor issues. The AVFrame.opaque_ref fiasco
(whose non-sensical later solution which he demanded was finally
pushed, even though it still does not solve the issues he claimed his
approach would solve) comes to mind, and I considered his general
conduct on this issue harassment (not like I'd get an apology).

I don't know what the other case of supposed harassment Carl Eugen is
making up without providing any proof.
Carl Eugen Hoyos Jan. 25, 2018, 11:56 p.m. UTC | #16
2018-01-26 0:52 GMT+01:00 wm4 <nfxjfg@googlemail.com>:
> (and I already wrote that on IRC too, where he lurks as
> michaelni)

Could one of the native speakers please try to convince
me that this is not a disparaging term?

Thank you, Carl Eugen
wm4 Jan. 25, 2018, 11:59 p.m. UTC | #17
On Fri, 26 Jan 2018 00:56:16 +0100
Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:

> 2018-01-26 0:52 GMT+01:00 wm4 <nfxjfg@googlemail.com>:
> > (and I already wrote that on IRC too, where he lurks as
> > michaelni)  
> 
> Could one of the native speakers please try to convince
> me that this is not a disparaging term?

That's his IRC nick name. Did you not know this, and this is another
failed attempt of you trying to put me in a bad light?

Maybe you could just stop this behavior for general de-escalation?
Jan Ekström Jan. 26, 2018, 12:45 a.m. UTC | #18
On Fri, Jan 26, 2018 at 1:56 AM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
> 2018-01-26 0:52 GMT+01:00 wm4 <nfxjfg@googlemail.com>:
>> (and I already wrote that on IRC too, where he lurks as
>> michaelni)
>
> Could one of the native speakers please try to convince
> me that this is not a disparaging term?
>

Hi,

I am not an English native, but the verb "to lurk" is colloquially utilized as:

"read the postings in an Internet forum without actively contributing."
(quote from whatever dictionary Google utilizes)

...and this IMHO is applicable enough as Michael is busy just like
many of us are busy most of the day.

Now, please, all parties. Stop. We've had enough rudeness in this
thread. And now, back to on-topic:

* This change set offers to rename some options to make them similar
to how the TCP protocol does things
(http://git.videolan.org/?p=ffmpeg.git;a=blob;f=libavformat/tcp.c;h=8773493df1efebac33cff5d203c8c9ff17299d4b;hb=HEAD#l52).
UDP protocol seems to follow suite regarding the "timeout" parameter,
even matching the microsecond part?
* It's imperfect since it doesn't change any of the types of the
options (listen_timeout is now name-wise matched, but type-wise is
supposed to contain seconds for rtsp, milliseconds (! yet another time
scale) in tcp).
* But if we do not rename the current timeout parameter (which does a
different thing than timeout for both TCP and UDP - and name-wise
matches "listen_timeout" as far as it can be seen), we cannot have the
matching name for the matching type of - which should be under the
option "timeout".

So, as far as I can see for now applying this or a slightly modified
version of this as per James' comments doesn't seem to be a too bad
initial step. It brings at least one option in line. We can then start
the larger job of normalizing the timeout parameter across FFmpeg, as
it seems like mentioning this area made multiple people notice
possible improvements in this area. But this is just my 2 eurocents.


Best regards,
Jan
Michael Niedermayer Jan. 26, 2018, 3:44 p.m. UTC | #19
On Fri, Jan 26, 2018 at 02:45:47AM +0200, Jan Ekström wrote:
> On Fri, Jan 26, 2018 at 1:56 AM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
> > 2018-01-26 0:52 GMT+01:00 wm4 <nfxjfg@googlemail.com>:
> >> (and I already wrote that on IRC too, where he lurks as
> >> michaelni)
> >
> > Could one of the native speakers please try to convince
> > me that this is not a disparaging term?
> >
> 
> Hi,
> 
> I am not an English native, but the verb "to lurk" is colloquially utilized as:
> 
> "read the postings in an Internet forum without actively contributing."
> (quote from whatever dictionary Google utilizes)
> 
> ...and this IMHO is applicable enough as Michael is busy just like
> many of us are busy most of the day.
> 
> Now, please, all parties. Stop. We've had enough rudeness in this
> thread. And now, back to on-topic:
> 
> * This change set offers to rename some options to make them similar
> to how the TCP protocol does things
> (http://git.videolan.org/?p=ffmpeg.git;a=blob;f=libavformat/tcp.c;h=8773493df1efebac33cff5d203c8c9ff17299d4b;hb=HEAD#l52).
> UDP protocol seems to follow suite regarding the "timeout" parameter,
> even matching the microsecond part?
> * It's imperfect since it doesn't change any of the types of the
> options (listen_timeout is now name-wise matched, but type-wise is
> supposed to contain seconds for rtsp, milliseconds (! yet another time
> scale) in tcp).
> * But if we do not rename the current timeout parameter (which does a
> different thing than timeout for both TCP and UDP - and name-wise
> matches "listen_timeout" as far as it can be seen), we cannot have the
> matching name for the matching type of - which should be under the
> option "timeout".
> 
> So, as far as I can see for now applying this or a slightly modified
> version of this as per James' comments doesn't seem to be a too bad
> initial step. It brings at least one option in line. We can then start
> the larger job of normalizing the timeout parameter across FFmpeg, as
> it seems like mentioning this area made multiple people notice
> possible improvements in this area. But this is just my 2 eurocents.

I think we first should take a step back and decide how we want a
consistent API to look exactly.
Then make changes towards that.

This patch introduces a parameter that very possibly will have to
be deprecated when things are changed to consistent second based timeouts.

I mean this patch makes rtsp consistent with parameters that we may have to
deprecate.

Lets just for a moment assume we agree that we want to have consistent
second based timeouts throughout the codebase.
To achive this with minimal confusion it requires a new parameter name
that has not been used for micro or nano or milli seconds before.
If now a new parameter for timeouts is added in seconds then each
piece of code can independantly add support to it one by one never
conflicting with anything or depending on anything.
And all existing timeouts that are not in seconds would then become
deprecated.

I think thats a meaningfull way forward and i would suggest that we
go that direction. Its very easy to do too once there is agreement on
the name of the new parameter(s)

We can also apply this patch but its not really moving us closer to
consistent second based timeouts.

thanks

[...]
wm4 Jan. 26, 2018, 8:29 p.m. UTC | #20
On Thu, 25 Jan 2018 19:00:43 +0100
wm4 <nfxjfg@googlemail.com> 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.
> ---

I'll push this in a few days or so with James Almers change requests
applied. There was a lot of other bikeshedding and personal insults,
but no actual rejection or further mandatory change requests.
Michael Niedermayer Jan. 27, 2018, 1:25 a.m. UTC | #21
On Fri, Jan 26, 2018 at 12:52:19AM +0100, wm4 wrote:
> On Fri, 26 Jan 2018 00:21:14 +0100
> Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
> 
> > 2018-01-26 0:00 GMT+01:00 Hendrik Leppkes <h.leppkes@gmail.com>:
> > > On Thu, Jan 25, 2018 at 11:35 PM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:  
[...]

> 
> I'd also like to point out that it _did_ happen in the past that
> Michael went off tangents in patch reviews and asked for unreasonable
> extra work because of minor issues.

Ive written a huge number of reviews over the years, and i certainly
did what you claim above in one of these many reviews. Not in the case
you refer here to though. And if it happens, that a unreasonable request
is made, discussion is whats needed. 
In fact many developers have made requests that others have found
unreasonable. Many if not most of the more active developers have.
Its not a uncommon source of long discussions.



> The AVFrame.opaque_ref fiasco
> (whose non-sensical later solution which he demanded was finally
> pushed, even though it still does not solve the issues he claimed his
> approach would solve) comes to mind,

so much you say in here
ill skip over "non-sensical" and "demanded" these are not really true in
the way you present it here.
the solution choosen IIRC does indeed not achive everything. It was
in fact a compromise that resulted from the discussion of many developers.
IIRC a solution that would solve all teh listed issues was not liked by
multiple people


> and I considered his general
> conduct on this issue harassment (not like I'd get an apology).

If anything i said felt like harrasment, i appologize. That said
a review or comment or objection to a patch is not harrasment and
I will continue to review and when it is needed object to changes.

Maybe it is just my point of view here but you seem very sensitive
to anything that is not an approval of your changes.
While at the same time your replies are often aggressive or offensive.

Just as random examples, your single line objection here certainly came
over in a passive aggressive tone:
https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2018-January/224488.html

and your next reply, that basically told me farewell when i said that i
wouldnt continue to maintain or help maintain hevc when error messages
would have to be ommited (not one specific one but in general)
https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2018-January/224506.html

And that was just a day or 2 ago. 

The very mail here i reply to is then also accusing me of harrasment 
in patch review from last year. I dont exactly enjoy having to reply
to these kind of accusations. More so considering how long ago the
review is that refers to.

and on IRC 2 days ago

Jän 25 23:47:53 <jamrial>       <+wm4> also michaelni harassed me a lot in the past (like making me go through all that pointless crap when getting rid of the side data merging
Jän 25 23:48:02 <jamrial>       can you drop that already? it was not pointless
Jän 25 23:48:22 <jamrial>       i was careless and did a merge wrong, and google came out of the woodworks because i broke chromium
Jän 25 23:48:41 <jamrial>       the abi concerns had a reason

I would really like to see these accusations stop, this is not something
anyone else that i remember has done in FFmpeg or Libav, not at this scale
at least.

Thanks

[...]
wm4 Jan. 27, 2018, 2:23 a.m. UTC | #22
On Sat, 27 Jan 2018 02:25:49 +0100
Michael Niedermayer <michael@niedermayer.cc> wrote:

> On Fri, Jan 26, 2018 at 12:52:19AM +0100, wm4 wrote:
> > On Fri, 26 Jan 2018 00:21:14 +0100
> > Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
> >   
> > > 2018-01-26 0:00 GMT+01:00 Hendrik Leppkes <h.leppkes@gmail.com>:  
> > > > On Thu, Jan 25, 2018 at 11:35 PM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:    
> [...]
> 
> > 
> > I'd also like to point out that it _did_ happen in the past that
> > Michael went off tangents in patch reviews and asked for unreasonable
> > extra work because of minor issues.  
> 
> Ive written a huge number of reviews over the years, and i certainly
> did what you claim above in one of these many reviews. Not in the case
> you refer here to though. And if it happens, that a unreasonable request
> is made, discussion is whats needed. 
> In fact many developers have made requests that others have found
> unreasonable. Many if not most of the more active developers have.
> Its not a uncommon source of long discussions.

And so we're having a long discussion again. You could just have left
it alone.

> 
> 
> > The AVFrame.opaque_ref fiasco
> > (whose non-sensical later solution which he demanded was finally
> > pushed, even though it still does not solve the issues he claimed his
> > approach would solve) comes to mind,  
> 
> so much you say in here
> ill skip over "non-sensical" and "demanded" these are not really true in
> the way you present it here.

The "demanded" was definitely true. You blocked my patch, even though
it was just a Libav merge to begin with.

There were never big discussions about such tiny merges before, and
nobody would have given a shit had I pushed those merges directly to
git (like it's normally done with merges). But you started a big
discussion and eventually achieved that a sub-optimal solution was
added.

The current solution _is_ nonsense. There is now a semi-public field
(AVFrame.private_ref) in a libavutil struct that can be used only by
libavcodec, something which we always avoid, because the libraries are
supposed to use only the public API of each other (except avpriv_ and
headers).

It still doesn't solve the problem you claimed there was: if libavcodec
fails to "process" the private_ref contents, the returned frame to the
user will be broken and make the user application crash.

Oh, and of course the doxygen says AVFrame.private_ref can be used by
other ffmpeg libs too. So what happens if libavfilter's use of
private_ref clashes with libavcodec's? Please explain.

Absolutely NOTHING is helped by using a separate field instead of
opaque_ref, and you were the only one who insisted on doing this (the
others at best unaware of the details and vaguely trusting your claims).

You even ignored my solution, which was consistently checking all
entrypoints for opaque_ref (and wrapping the user use of it), which my
patches did. You didn't even care and somehow claimed there were holes
in it, which there were not, and even if there were, YOUR CURRENT CODE
WOULD FAIL TOO.

At this point I seriously had enough of your shit and left the project
for a while.

> the solution choosen IIRC does indeed not achive everything. It was
> in fact a compromise that resulted from the discussion of many developers.
> IIRC a solution that would solve all teh listed issues was not liked by
> multiple people

Because apparently they still wanted it to be merged, so they just did
whatever you wanted, whether it made sense or not. I on the other hand
was seeking a solution that was technically closer to ideal. But it was
impossible to get through you.

> > and I considered his general
> > conduct on this issue harassment (not like I'd get an apology).  
> 
> If anything i said felt like harrasment, i appologize. That said
> a review or comment or objection to a patch is not harrasment and
> I will continue to review and when it is needed object to changes.
> 
> Maybe it is just my point of view here but you seem very sensitive
> to anything that is not an approval of your changes.
> While at the same time your replies are often aggressive or offensive.
> 
> Just as random examples, your single line objection here certainly came
> over in a passive aggressive tone:
> https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2018-January/224488.html

That's not passive aggressive. You know certain devs (including me)
don't like excessive logging code for fuzzing fixes, and that was
extensively discussed in the past. But there you go again. (And sure, I
can be convinced that the logging is actually OK in this specific case,
but you preferred doing something else instead.)

However, your reply was passive aggressive to the max:

> 
> and your next reply, that basically told me farewell when i said that i
> wouldnt continue to maintain or help maintain hevc when error messages
> would have to be ommited (not one specific one but in general)
> https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2018-January/224506.html

You basically threatened that you'd stop doing important work on hevc
if you didn't get to add your error messages. Well, I ignored the
threat and just told you to do draw your consequences out of the
non-approval, if you really want to. I expect it was an empty threat
anyway.

Such "threats" are completely inappropriate in patch reviews.

You realize the irony of threatening something like this, while your
patch "reviews" made me leave the project for a while, right?

> And that was just a day or 2 ago. 
> 
> The very mail here i reply to is then also accusing me of harrasment 
> in patch review from last year. I dont exactly enjoy having to reply
> to these kind of accusations. More so considering how long ago the
> review is that refers to.
> 
> and on IRC 2 days ago
> 
> Jän 25 23:47:53 <jamrial>       <+wm4> also michaelni harassed me a lot in the past (like making me go through all that pointless crap when getting rid of the side data merging
> Jän 25 23:48:02 <jamrial>       can you drop that already? it was not pointless
> Jän 25 23:48:22 <jamrial>       i was careless and did a merge wrong, and google came out of the woodworks because i broke chromium
> Jän 25 23:48:41 <jamrial>       the abi concerns had a reason

Definitely the side data merging ABI claim was completely pointless. If
any application had a dependency on side data being merged, they would
have broken on the ABI bump. I don't know what jamrial is referring to.
Besides, packet contents were not considered ABI relevant before. My
own project was once broken by a packet format change - so what? I just
fixed it on my side.

Sure, these technical points can be fuzzy and I might be wrong too.
But it's not just about that. You fought pretty hard against my patches
and invented all sorts of arguments. You accused me of trolling too. I
didn't enjoy this interaction.

How come you find my claims illegitimate, but if I tell you something
as simple to drop the logging it's suddenly passive aggressive
harassment. Come on?

> I would really like to see these accusations stop, this is not something
> anyone else that i remember has done in FFmpeg or Libav, not at this scale
> at least.

Well, you remember how half of the project forked off years ago because
they were tired of your antics as project leader, right? That was pretty
big scale wasn't it? I'm not threatening to fork ffmpeg as a project or
anything close to it, nor do I want to unearth the Libav drama, but
your claim is certainly a bit of an exaggeration.

Anyway, I too would like if the harassment against me by you and CE
would stop. On a related note, I know no other project that would
tolerate CE's behavior at all.
James Almer Jan. 27, 2018, 2:26 a.m. UTC | #23
On 1/26/2018 11:23 PM, wm4 wrote:
> On Sat, 27 Jan 2018 02:25:49 +0100
> Michael Niedermayer <michael@niedermayer.cc> wrote:
> 
>> On Fri, Jan 26, 2018 at 12:52:19AM +0100, wm4 wrote:
>>> On Fri, 26 Jan 2018 00:21:14 +0100
>>> Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
>>>   
>>>> 2018-01-26 0:00 GMT+01:00 Hendrik Leppkes <h.leppkes@gmail.com>:  
>>>>> On Thu, Jan 25, 2018 at 11:35 PM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:    
>> [...]
>>
>>>
>>> I'd also like to point out that it _did_ happen in the past that
>>> Michael went off tangents in patch reviews and asked for unreasonable
>>> extra work because of minor issues.  
>>
>> Ive written a huge number of reviews over the years, and i certainly
>> did what you claim above in one of these many reviews. Not in the case
>> you refer here to though. And if it happens, that a unreasonable request
>> is made, discussion is whats needed. 
>> In fact many developers have made requests that others have found
>> unreasonable. Many if not most of the more active developers have.
>> Its not a uncommon source of long discussions.
> 
> And so we're having a long discussion again. You could just have left
> it alone.

Then drop it, the lot of you. Stop replying, throwing contradicting
accusations in every direction, and let this thread die already.

Nothing, absolutely nothing good will come out of this if you keep going.
wm4 Jan. 27, 2018, 2:38 a.m. UTC | #24
On Fri, 26 Jan 2018 23:26:20 -0300
James Almer <jamrial@gmail.com> wrote:

> On 1/26/2018 11:23 PM, wm4 wrote:
> > On Sat, 27 Jan 2018 02:25:49 +0100
> > Michael Niedermayer <michael@niedermayer.cc> wrote:
> >   
> >> On Fri, Jan 26, 2018 at 12:52:19AM +0100, wm4 wrote:  
> >>> On Fri, 26 Jan 2018 00:21:14 +0100
> >>> Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
> >>>     
> >>>> 2018-01-26 0:00 GMT+01:00 Hendrik Leppkes <h.leppkes@gmail.com>:    
> >>>>> On Thu, Jan 25, 2018 at 11:35 PM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:      
> >> [...]
> >>  
> >>>
> >>> I'd also like to point out that it _did_ happen in the past that
> >>> Michael went off tangents in patch reviews and asked for unreasonable
> >>> extra work because of minor issues.    
> >>
> >> Ive written a huge number of reviews over the years, and i certainly
> >> did what you claim above in one of these many reviews. Not in the case
> >> you refer here to though. And if it happens, that a unreasonable request
> >> is made, discussion is whats needed. 
> >> In fact many developers have made requests that others have found
> >> unreasonable. Many if not most of the more active developers have.
> >> Its not a uncommon source of long discussions.  
> > 
> > And so we're having a long discussion again. You could just have left
> > it alone.  
> 
> Then drop it, the lot of you. Stop replying, throwing contradicting
> accusations in every direction, and let this thread die already.
> 
> Nothing, absolutely nothing good will come out of this if you keep going.

I'm just defending myself. I don't want to be accused of throwing
around baseless accusations. That includes your post too.

There are _real_ issues and they will come up again and again if we
don't solve them. Pretending they don't exist isn't a solution.

But whatever.
wm4 Feb. 4, 2018, 2:24 p.m. UTC | #25
On Thu, 25 Jan 2018 19:00:43 +0100
wm4 <nfxjfg@googlemail.com> 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
>      { NULL },
>  };
>  
> diff --git a/libavformat/version.h b/libavformat/version.h
> index 5ff8a89ae0..4285e925cf 100644
> --- a/libavformat/version.h
> +++ b/libavformat/version.h
> @@ -32,7 +32,7 @@
>  // Major bumping may affect Ticket5467, 5421, 5451(compatibility with Chromium)
>  // Also please add any ticket numbers that you believe might be affected here
>  #define LIBAVFORMAT_VERSION_MAJOR  58
> -#define LIBAVFORMAT_VERSION_MINOR   6
> +#define LIBAVFORMAT_VERSION_MINOR   7
>  #define LIBAVFORMAT_VERSION_MICRO 100
>  
>  #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \
> @@ -85,6 +85,9 @@
>  #ifndef FF_API_LAVF_FFSERVER
>  #define FF_API_LAVF_FFSERVER            (LIBAVFORMAT_VERSION_MAJOR < 59)
>  #endif
> +#ifndef FF_API_OLD_RTSP_OPTIONS
> +#define FF_API_OLD_RTSP_OPTIONS         (LIBAVFORMAT_VERSION_MAJOR < 59)
> +#endif
>  
>  
>  #ifndef FF_API_R_FRAME_RATE

Pushed, with James Almer's change request added.

Anyone is free to add a fancy new timeout system on top of this, it
doesn't conflict.
Carl Eugen Hoyos Feb. 5, 2018, 2:42 a.m. UTC | #26
2018-02-04 15:24 GMT+01:00 wm4 <nfxjfg@googlemail.com>:
> On Thu, 25 Jan 2018 19:00:43 +0100
> wm4 <nfxjfg@googlemail.com> 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
>>      { NULL },
>>  };
>>
>> diff --git a/libavformat/version.h b/libavformat/version.h
>> index 5ff8a89ae0..4285e925cf 100644
>> --- a/libavformat/version.h
>> +++ b/libavformat/version.h
>> @@ -32,7 +32,7 @@
>>  // Major bumping may affect Ticket5467, 5421, 5451(compatibility with Chromium)
>>  // Also please add any ticket numbers that you believe might be affected here
>>  #define LIBAVFORMAT_VERSION_MAJOR  58
>> -#define LIBAVFORMAT_VERSION_MINOR   6
>> +#define LIBAVFORMAT_VERSION_MINOR   7
>>  #define LIBAVFORMAT_VERSION_MICRO 100
>>
>>  #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \
>> @@ -85,6 +85,9 @@
>>  #ifndef FF_API_LAVF_FFSERVER
>>  #define FF_API_LAVF_FFSERVER            (LIBAVFORMAT_VERSION_MAJOR < 59)
>>  #endif
>> +#ifndef FF_API_OLD_RTSP_OPTIONS
>> +#define FF_API_OLD_RTSP_OPTIONS         (LIBAVFORMAT_VERSION_MAJOR < 59)
>> +#endif
>>
>>
>>  #ifndef FF_API_R_FRAME_RATE
>
> Pushed, with James Almer's change request added.

This is completely unacceptable:
Why are you constantly breaking developer rules?

Carl Eugen
wm4 Feb. 5, 2018, 9:42 a.m. UTC | #27
On Mon, 5 Feb 2018 03:42:48 +0100
Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:

> 2018-02-04 15:24 GMT+01:00 wm4 <nfxjfg@googlemail.com>:
> > On Thu, 25 Jan 2018 19:00:43 +0100
> > wm4 <nfxjfg@googlemail.com> 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
> >>      { NULL },
> >>  };
> >>
> >> diff --git a/libavformat/version.h b/libavformat/version.h
> >> index 5ff8a89ae0..4285e925cf 100644
> >> --- a/libavformat/version.h
> >> +++ b/libavformat/version.h
> >> @@ -32,7 +32,7 @@
> >>  // Major bumping may affect Ticket5467, 5421, 5451(compatibility with Chromium)
> >>  // Also please add any ticket numbers that you believe might be affected here
> >>  #define LIBAVFORMAT_VERSION_MAJOR  58
> >> -#define LIBAVFORMAT_VERSION_MINOR   6
> >> +#define LIBAVFORMAT_VERSION_MINOR   7
> >>  #define LIBAVFORMAT_VERSION_MICRO 100
> >>
> >>  #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \
> >> @@ -85,6 +85,9 @@
> >>  #ifndef FF_API_LAVF_FFSERVER
> >>  #define FF_API_LAVF_FFSERVER            (LIBAVFORMAT_VERSION_MAJOR < 59)
> >>  #endif
> >> +#ifndef FF_API_OLD_RTSP_OPTIONS
> >> +#define FF_API_OLD_RTSP_OPTIONS         (LIBAVFORMAT_VERSION_MAJOR < 59)
> >> +#endif
> >>
> >>
> >>  #ifndef FF_API_R_FRAME_RATE  
> >
> > Pushed, with James Almer's change request added.  
> 
> This is completely unacceptable:
> Why are you constantly breaking developer rules?

I'm not. Stop attacking me, for no reason too.
diff mbox

Patch

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
     { NULL },
 };
 
diff --git a/libavformat/version.h b/libavformat/version.h
index 5ff8a89ae0..4285e925cf 100644
--- a/libavformat/version.h
+++ b/libavformat/version.h
@@ -32,7 +32,7 @@ 
 // Major bumping may affect Ticket5467, 5421, 5451(compatibility with Chromium)
 // Also please add any ticket numbers that you believe might be affected here
 #define LIBAVFORMAT_VERSION_MAJOR  58
-#define LIBAVFORMAT_VERSION_MINOR   6
+#define LIBAVFORMAT_VERSION_MINOR   7
 #define LIBAVFORMAT_VERSION_MICRO 100
 
 #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \
@@ -85,6 +85,9 @@ 
 #ifndef FF_API_LAVF_FFSERVER
 #define FF_API_LAVF_FFSERVER            (LIBAVFORMAT_VERSION_MAJOR < 59)
 #endif
+#ifndef FF_API_OLD_RTSP_OPTIONS
+#define FF_API_OLD_RTSP_OPTIONS         (LIBAVFORMAT_VERSION_MAJOR < 59)
+#endif
 
 
 #ifndef FF_API_R_FRAME_RATE