Re: [FFmpeg-devel] [PATCH] avformat/http: remove duplicate user-agent option

[Clément Bœsch]

On Fri, Sep 16, 2016 at 10:43:30AM +0800, Steven Liu wrote:
> From: Steven Liu 
> Date: Fri, 16 Sep 2016 10:42:34 +0800
> Subject: [PATCH] avformat/http: will remove option user_agent
> 
> add a FF_OPT_FLAG_DEPRECATED define for compile.
> add a new variable for user_agent to give a deprecated warning message
> ---
>  doc/protocols.texi|  4 +++-
>  libavformat/http.c| 13 -
>  libavformat/version.h |  3 +++
>  3 files changed, 18 insertions(+), 2 deletions(-)

Adjusted commit message and applied. Thanks.

-- 
Clément B.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avformat/http: remove duplicate user-agent option

[Steven Liu]

2016-09-16 19:07 GMT+08:00 Carl Eugen Hoyos :

> 2016-09-16 4:43 GMT+02:00 Steven Liu :
>
> > +@item user-agent
> > +This is a deprecated option, you can use user_agent instead it.
>
> Sorry for the nitpicking: Why do you move the first, unchanged line?
>
> Carl Eugen
>
>
Because it will be remove, so it's position is not important :-D
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avformat/http: remove duplicate user-agent option

[Carl Eugen Hoyos]

2016-09-16 4:43 GMT+02:00 Steven Liu :

> +@item user-agent
> +This is a deprecated option, you can use user_agent instead it.

Sorry for the nitpicking: Why do you move the first, unchanged line?

Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avformat/http: remove duplicate user-agent option

[Steven Liu]

2016-09-16 1:08 GMT+08:00 Clément Bœsch :

> On Fri, Sep 16, 2016 at 12:41:41AM +0800, Steven Liu wrote:
> [...]
> > update:
> >
>
> Please attach a git-format-patch or use git-send-email.
> Also, your mailer is mangling the inlined patch.
>
> >
> > add a FF_OPT_FLAG_DEPRECATED define for compile.
> > add a new variable for user_agent to give a deprecated warning message
> > ---
> >  doc/protocols.texi|  4 +++-
> >  libavformat/http.c| 14 --
> >  libavformat/version.h |  3 +++
> >  3 files changed, 18 insertions(+), 3 deletions(-)
> >
> > diff --git a/doc/protocols.texi b/doc/protocols.texi
> > index 470c99c..3acdc78 100644
> > --- a/doc/protocols.texi
> > +++ b/doc/protocols.texi
> > @@ -292,10 +292,12 @@ Use persistent connections if set to 1, default is
> 0.
> >  Set custom HTTP post data.
> >
> >  @item user-agent
> > -@item user_agent
> >  Override the User-Agent header. If not specified the protocol will use a
> >  string describing the libavformat build. ("Lavf/")
> >
> > +@item user_agent
> > +This is a deprecated option, you can use user-agent instead it.
> > +
>
> Again, you don't want to deprecate "user_agent", that's the one you want
> to keep (because it's consistent with the other options).
>
> >  @item timeout
> >  Set timeout in microseconds of socket I/O operations used by the
> > underlying low level
> >  operation. By default it is set to -1, which means that the timeout is
> > diff --git a/libavformat/http.c b/libavformat/http.c
> > index adb3d92..0b8a86c 100644
> > --- a/libavformat/http.c
> > +++ b/libavformat/http.c
> > @@ -71,6 +71,9 @@ typedef struct HTTPContext {
> >  char *headers;
> >  char *mime_type;
> >  char *user_agent;
>
> > +#ifdef FF_OPT_FLAG_DEPRECATED
>
> The FF_OPT_FLAG_DEPRECATED name is related to a generic approach for
> deprecating AVOption (by adding an AVOption flag, and patching
> lavu/opt.c). In your case, you want to use FF_API_HTTP_USER_AGENT.
>
> > +attribute_deprecated char *user_agent_deprecated;
>
> again, you don't want to use attribute_deprecated.
>
> > +#endif
> >  char *content_type;
> >  /* Set if the server correctly handles Connection: close and will
> close
> >   * the connection after feeding us the content. */
> > @@ -130,7 +133,9 @@ static const AVOption options[] = {
> >  { "http_proxy", "set HTTP proxy to tunnel through",
> > OFFSET(http_proxy), AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, D | E },
> >  { "headers", "set custom HTTP headers, can override built in default
> > headers", OFFSET(headers), AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, D
> | E
> > },
> >  { "content_type", "set a specific content type for the POST
> messages",
> > OFFSET(content_type), AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, D | E },
> > -{ "user_agent", "override User-Agent header", OFFSET(user_agent),
> > AV_OPT_TYPE_STRING, { .str = DEFAULT_USER_AGENT }, 0, 0, D },
> > +#ifdef FF_OPT_FLAG_DEPRECATED
> > +{ "user_agent", "override User-Agent header",
> > OFFSET(user_agent_deprecated), AV_OPT_TYPE_STRING, { .str =
> > DEFAULT_USER_AGENT }, 0, 0, D },
> > +#endif
> >  { "user-agent", "override User-Agent header", OFFSET(user_agent),
> > AV_OPT_TYPE_STRING, { .str = DEFAULT_USER_AGENT }, 0, 0, D },
> >  { "multiple_requests", "use persistent connections",
> > OFFSET(multiple_requests), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, D | E },
> >  { "post_data", "set custom HTTP post data", OFFSET(post_data),
> > AV_OPT_TYPE_BINARY, .flags = D | E },
> > @@ -1036,7 +1041,12 @@ static int http_connect(URLContext *h, const char
> > *path, const char *local_path,
> >  s->http_code != 401)
> >  send_expect_100 = 1;
> >  }
>
> > -
>
> You should keep that empty line
>
> [...]
>
> --
> Clément B.
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>


0001-avformat-http-will-remove-option-user_agent.patch
Description: Binary data
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avformat/http: remove duplicate user-agent option

[Clément Bœsch]

On Fri, Sep 16, 2016 at 12:41:41AM +0800, Steven Liu wrote:
[...]
> update:
> 

Please attach a git-format-patch or use git-send-email.
Also, your mailer is mangling the inlined patch.

> 
> add a FF_OPT_FLAG_DEPRECATED define for compile.
> add a new variable for user_agent to give a deprecated warning message
> ---
>  doc/protocols.texi|  4 +++-
>  libavformat/http.c| 14 --
>  libavformat/version.h |  3 +++
>  3 files changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/doc/protocols.texi b/doc/protocols.texi
> index 470c99c..3acdc78 100644
> --- a/doc/protocols.texi
> +++ b/doc/protocols.texi
> @@ -292,10 +292,12 @@ Use persistent connections if set to 1, default is 0.
>  Set custom HTTP post data.
> 
>  @item user-agent
> -@item user_agent
>  Override the User-Agent header. If not specified the protocol will use a
>  string describing the libavformat build. ("Lavf/")
> 
> +@item user_agent
> +This is a deprecated option, you can use user-agent instead it.
> +

Again, you don't want to deprecate "user_agent", that's the one you want
to keep (because it's consistent with the other options).

>  @item timeout
>  Set timeout in microseconds of socket I/O operations used by the
> underlying low level
>  operation. By default it is set to -1, which means that the timeout is
> diff --git a/libavformat/http.c b/libavformat/http.c
> index adb3d92..0b8a86c 100644
> --- a/libavformat/http.c
> +++ b/libavformat/http.c
> @@ -71,6 +71,9 @@ typedef struct HTTPContext {
>  char *headers;
>  char *mime_type;
>  char *user_agent;

> +#ifdef FF_OPT_FLAG_DEPRECATED

The FF_OPT_FLAG_DEPRECATED name is related to a generic approach for
deprecating AVOption (by adding an AVOption flag, and patching
lavu/opt.c). In your case, you want to use FF_API_HTTP_USER_AGENT.

> +attribute_deprecated char *user_agent_deprecated;

again, you don't want to use attribute_deprecated.

> +#endif
>  char *content_type;
>  /* Set if the server correctly handles Connection: close and will close
>   * the connection after feeding us the content. */
> @@ -130,7 +133,9 @@ static const AVOption options[] = {
>  { "http_proxy", "set HTTP proxy to tunnel through",
> OFFSET(http_proxy), AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, D | E },
>  { "headers", "set custom HTTP headers, can override built in default
> headers", OFFSET(headers), AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, D | E
> },
>  { "content_type", "set a specific content type for the POST messages",
> OFFSET(content_type), AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, D | E },
> -{ "user_agent", "override User-Agent header", OFFSET(user_agent),
> AV_OPT_TYPE_STRING, { .str = DEFAULT_USER_AGENT }, 0, 0, D },
> +#ifdef FF_OPT_FLAG_DEPRECATED
> +{ "user_agent", "override User-Agent header",
> OFFSET(user_agent_deprecated), AV_OPT_TYPE_STRING, { .str =
> DEFAULT_USER_AGENT }, 0, 0, D },
> +#endif
>  { "user-agent", "override User-Agent header", OFFSET(user_agent),
> AV_OPT_TYPE_STRING, { .str = DEFAULT_USER_AGENT }, 0, 0, D },
>  { "multiple_requests", "use persistent connections",
> OFFSET(multiple_requests), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, D | E },
>  { "post_data", "set custom HTTP post data", OFFSET(post_data),
> AV_OPT_TYPE_BINARY, .flags = D | E },
> @@ -1036,7 +1041,12 @@ static int http_connect(URLContext *h, const char
> *path, const char *local_path,
>  s->http_code != 401)
>  send_expect_100 = 1;
>  }

> -

You should keep that empty line

[...]

-- 
Clément B.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avformat/http: remove duplicate user-agent option

[Steven Liu]

2016-09-16 0:35 GMT+08:00 Steven Liu :

> ignore it please, i will fix some problem
>
> 2016-09-16 0:30 GMT+08:00 Steven Liu :
>
>>
>>
>> 2016-09-16 0:21 GMT+08:00 Clément Bœsch :
>>
>>> On Fri, Sep 16, 2016 at 12:04:34AM +0800, Steven Liu wrote:
>>> > 2016-09-15 23:42 GMT+08:00 Clément Bœsch :
>>> >
>>> > > On Thu, Sep 15, 2016 at 05:39:25PM +0200, Nicolas George wrote:
>>> > > > Le decadi 30 fructidor, an CCXXIV, Clement Boesch a écrit :
>>> > > > > Yes, surrounded by the #ifdefery so it will go away with the
>>> option.
>>> > > > >
>>> > > > > Note: use av_strdup() to transfer from one field to the other
>>> > > >
>>> > > > Another possibility would be to implement AV_OPT_FLAG_DEPRECATED
>>> and let
>>> > > the
>>> > > > options system itself print the warning.
>>> > > >
>>> > >
>>> > > That's not a bad idea; you might need to add a field to redirect to
>>> > > the new option though.
>>> > >
>>> > > --
>>> > > Clément B.
>>> > > ___
>>> > >
>>> >
>>> >
>>> > Hi guys,
>>> >
>>> > are you mean like this?
>>> >
>>> > localhost:xxx StevenLiu$ make
>>> > CC libavformat/http.o
>>> > src/libavformat/http.c:1041:27: warning: 'user_agent_deprecated' is
>>> > deprecated [-Wdeprecated-declarations]
>>> > if (av_strncasecmp(s->user_agent_deprecated, DEFAULT_USER_AGENT,
>>> > strlen(DEFAULT_USER_AGENT))) {
>>> >   ^
>>> > src/libavformat/http.c:74:32: note: 'user_agent_deprecated' has been
>>> > explicitly marked deprecated here
>>> > attribute_deprecated char *user_agent_deprecated;
>>> >^
>>> > src/libavformat/http.c:1043:38: warning: 'user_agent_deprecated' is
>>> > deprecated [-Wdeprecated-declarations]
>>> > s->user_agent = av_strdup(s->user_agent_deprecated);
>>> >  ^
>>> > src/libavformat/http.c:74:32: note: 'user_agent_deprecated' has been
>>> > explicitly marked deprecated here
>>> > attribute_deprecated char *user_agent_deprecated;
>>> >^
>>> > 2 warnings generated.
>>> > AR libavformat/libavformat.a
>>> >
>>> >
>>> > and the modify like this?
>>> >
>>> >
>>> > localhost:xxx StevenLiu$ git diff
>>> > diff --git a/libavformat/http.c b/libavformat/http.c
>>> > index adb3d92..df9cf1a 100644
>>> > --- a/libavformat/http.c
>>> > +++ b/libavformat/http.c
>>> > @@ -71,6 +71,7 @@ typedef struct HTTPContext {
>>> >  char *headers;
>>> >  char *mime_type;
>>> >  char *user_agent;
>>> > +attribute_deprecated char *user_agent_deprecated;
>>>
>>> not attribute_deprecated
>>>
>>> #if FF_API_HTTP_USER_AGENT
>>> char *user_agent_deprecated;
>>> #endif
>>>
>>> >  char *content_type;
>>> >  /* Set if the server correctly handles Connection: close and will
>>> close
>>> >   * the connection after feeding us the content. */
>>> > @@ -130,7 +131,7 @@ static const AVOption options[] = {
>>> >  { "http_proxy", "set HTTP proxy to tunnel through",
>>> > OFFSET(http_proxy), AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, D | E },
>>> >  { "headers", "set custom HTTP headers, can override built in
>>> default
>>> > headers", OFFSET(headers), AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0,
>>> D | E
>>> > },
>>> >  { "content_type", "set a specific content type for the POST
>>> messages",
>>> > OFFSET(content_type), AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, D | E
>>> },
>>> > -{ "user_agent", "override User-Agent header", OFFSET(user_agent),
>>> > AV_OPT_TYPE_STRING, { .str = DEFAULT_USER_AGENT }, 0, 0, D },
>>> > +{ "user_agent", "override User-Agent header",
>>>
>>> You want to deprecate user-agent, not user_agent.
>>>
>>> #if FF_API_HTTP_USER_AGENT
>>> { "user-agent", "override User-Agent header", ...
>>> #endif
>>>
>>> > OFFSET(user_agent_deprecated), AV_OPT_TYPE_STRING, { .str =
>>> > DEFAULT_USER_AGENT }, 0, 0, D },
>>> >  { "user-agent", "override User-Agent header", OFFSET(user_agent),
>>> > AV_OPT_TYPE_STRING, { .str = DEFAULT_USER_AGENT }, 0, 0, D },
>>> >  { "multiple_requests", "use persistent connections",
>>> > OFFSET(multiple_requests), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, D | E
>>> },
>>> >  { "post_data", "set custom HTTP post data", OFFSET(post_data),
>>> > AV_OPT_TYPE_BINARY, .flags = D | E },
>>> > @@ -1037,6 +1038,10 @@ static int http_connect(URLContext *h, const
>>> char
>>> > *path, const char *local_path,
>>> >  send_expect_100 = 1;
>>> >  }
>>> >
>>> > +if (av_strncasecmp(s->user_agent_deprecated, DEFAULT_USER_AGENT,
>>> > strlen(DEFAULT_USER_AGENT))) {
>>>
>>> - you need to check s->user_agent, not s->user_agent_deprecated
>>> - strcmp() is enough
>>>
>>> > +av_log(s, AV_LOG_WARNING, "the user_agent option is
>>> deprecated,
>>> > please use user-agent option\n");
>>>
>>> the other way around
>>>
>>> > +s->user_agent = 

Re: [FFmpeg-devel] [PATCH] avformat/http: remove duplicate user-agent option

[Steven Liu]

ignore it please, i will fix some problem

2016-09-16 0:30 GMT+08:00 Steven Liu :

>
>
> 2016-09-16 0:21 GMT+08:00 Clément Bœsch :
>
>> On Fri, Sep 16, 2016 at 12:04:34AM +0800, Steven Liu wrote:
>> > 2016-09-15 23:42 GMT+08:00 Clément Bœsch :
>> >
>> > > On Thu, Sep 15, 2016 at 05:39:25PM +0200, Nicolas George wrote:
>> > > > Le decadi 30 fructidor, an CCXXIV, Clement Boesch a écrit :
>> > > > > Yes, surrounded by the #ifdefery so it will go away with the
>> option.
>> > > > >
>> > > > > Note: use av_strdup() to transfer from one field to the other
>> > > >
>> > > > Another possibility would be to implement AV_OPT_FLAG_DEPRECATED
>> and let
>> > > the
>> > > > options system itself print the warning.
>> > > >
>> > >
>> > > That's not a bad idea; you might need to add a field to redirect to
>> > > the new option though.
>> > >
>> > > --
>> > > Clément B.
>> > > ___
>> > >
>> >
>> >
>> > Hi guys,
>> >
>> > are you mean like this?
>> >
>> > localhost:xxx StevenLiu$ make
>> > CC libavformat/http.o
>> > src/libavformat/http.c:1041:27: warning: 'user_agent_deprecated' is
>> > deprecated [-Wdeprecated-declarations]
>> > if (av_strncasecmp(s->user_agent_deprecated, DEFAULT_USER_AGENT,
>> > strlen(DEFAULT_USER_AGENT))) {
>> >   ^
>> > src/libavformat/http.c:74:32: note: 'user_agent_deprecated' has been
>> > explicitly marked deprecated here
>> > attribute_deprecated char *user_agent_deprecated;
>> >^
>> > src/libavformat/http.c:1043:38: warning: 'user_agent_deprecated' is
>> > deprecated [-Wdeprecated-declarations]
>> > s->user_agent = av_strdup(s->user_agent_deprecated);
>> >  ^
>> > src/libavformat/http.c:74:32: note: 'user_agent_deprecated' has been
>> > explicitly marked deprecated here
>> > attribute_deprecated char *user_agent_deprecated;
>> >^
>> > 2 warnings generated.
>> > AR libavformat/libavformat.a
>> >
>> >
>> > and the modify like this?
>> >
>> >
>> > localhost:xxx StevenLiu$ git diff
>> > diff --git a/libavformat/http.c b/libavformat/http.c
>> > index adb3d92..df9cf1a 100644
>> > --- a/libavformat/http.c
>> > +++ b/libavformat/http.c
>> > @@ -71,6 +71,7 @@ typedef struct HTTPContext {
>> >  char *headers;
>> >  char *mime_type;
>> >  char *user_agent;
>> > +attribute_deprecated char *user_agent_deprecated;
>>
>> not attribute_deprecated
>>
>> #if FF_API_HTTP_USER_AGENT
>> char *user_agent_deprecated;
>> #endif
>>
>> >  char *content_type;
>> >  /* Set if the server correctly handles Connection: close and will
>> close
>> >   * the connection after feeding us the content. */
>> > @@ -130,7 +131,7 @@ static const AVOption options[] = {
>> >  { "http_proxy", "set HTTP proxy to tunnel through",
>> > OFFSET(http_proxy), AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, D | E },
>> >  { "headers", "set custom HTTP headers, can override built in
>> default
>> > headers", OFFSET(headers), AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, D
>> | E
>> > },
>> >  { "content_type", "set a specific content type for the POST
>> messages",
>> > OFFSET(content_type), AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, D | E
>> },
>> > -{ "user_agent", "override User-Agent header", OFFSET(user_agent),
>> > AV_OPT_TYPE_STRING, { .str = DEFAULT_USER_AGENT }, 0, 0, D },
>> > +{ "user_agent", "override User-Agent header",
>>
>> You want to deprecate user-agent, not user_agent.
>>
>> #if FF_API_HTTP_USER_AGENT
>> { "user-agent", "override User-Agent header", ...
>> #endif
>>
>> > OFFSET(user_agent_deprecated), AV_OPT_TYPE_STRING, { .str =
>> > DEFAULT_USER_AGENT }, 0, 0, D },
>> >  { "user-agent", "override User-Agent header", OFFSET(user_agent),
>> > AV_OPT_TYPE_STRING, { .str = DEFAULT_USER_AGENT }, 0, 0, D },
>> >  { "multiple_requests", "use persistent connections",
>> > OFFSET(multiple_requests), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, D | E
>> },
>> >  { "post_data", "set custom HTTP post data", OFFSET(post_data),
>> > AV_OPT_TYPE_BINARY, .flags = D | E },
>> > @@ -1037,6 +1038,10 @@ static int http_connect(URLContext *h, const char
>> > *path, const char *local_path,
>> >  send_expect_100 = 1;
>> >  }
>> >
>> > +if (av_strncasecmp(s->user_agent_deprecated, DEFAULT_USER_AGENT,
>> > strlen(DEFAULT_USER_AGENT))) {
>>
>> - you need to check s->user_agent, not s->user_agent_deprecated
>> - strcmp() is enough
>>
>> > +av_log(s, AV_LOG_WARNING, "the user_agent option is deprecated,
>> > please use user-agent option\n");
>>
>> the other way around
>>
>> > +s->user_agent = av_strdup(s->user_agent_deprecated);
>> > +}
>>
>> --
>> Clément B.
>> ___
>>
>
>
>
>
>
> patch update
>
>
>
>
> add a FF_OPT_FLAG_DEPRECATED define for compile.
> add a new variable for 

Re: [FFmpeg-devel] [PATCH] avformat/http: remove duplicate user-agent option

[Steven Liu]

2016-09-16 0:21 GMT+08:00 Clément Bœsch :

> On Fri, Sep 16, 2016 at 12:04:34AM +0800, Steven Liu wrote:
> > 2016-09-15 23:42 GMT+08:00 Clément Bœsch :
> >
> > > On Thu, Sep 15, 2016 at 05:39:25PM +0200, Nicolas George wrote:
> > > > Le decadi 30 fructidor, an CCXXIV, Clement Boesch a écrit :
> > > > > Yes, surrounded by the #ifdefery so it will go away with the
> option.
> > > > >
> > > > > Note: use av_strdup() to transfer from one field to the other
> > > >
> > > > Another possibility would be to implement AV_OPT_FLAG_DEPRECATED and
> let
> > > the
> > > > options system itself print the warning.
> > > >
> > >
> > > That's not a bad idea; you might need to add a field to redirect to
> > > the new option though.
> > >
> > > --
> > > Clément B.
> > > ___
> > >
> >
> >
> > Hi guys,
> >
> > are you mean like this?
> >
> > localhost:xxx StevenLiu$ make
> > CC libavformat/http.o
> > src/libavformat/http.c:1041:27: warning: 'user_agent_deprecated' is
> > deprecated [-Wdeprecated-declarations]
> > if (av_strncasecmp(s->user_agent_deprecated, DEFAULT_USER_AGENT,
> > strlen(DEFAULT_USER_AGENT))) {
> >   ^
> > src/libavformat/http.c:74:32: note: 'user_agent_deprecated' has been
> > explicitly marked deprecated here
> > attribute_deprecated char *user_agent_deprecated;
> >^
> > src/libavformat/http.c:1043:38: warning: 'user_agent_deprecated' is
> > deprecated [-Wdeprecated-declarations]
> > s->user_agent = av_strdup(s->user_agent_deprecated);
> >  ^
> > src/libavformat/http.c:74:32: note: 'user_agent_deprecated' has been
> > explicitly marked deprecated here
> > attribute_deprecated char *user_agent_deprecated;
> >^
> > 2 warnings generated.
> > AR libavformat/libavformat.a
> >
> >
> > and the modify like this?
> >
> >
> > localhost:xxx StevenLiu$ git diff
> > diff --git a/libavformat/http.c b/libavformat/http.c
> > index adb3d92..df9cf1a 100644
> > --- a/libavformat/http.c
> > +++ b/libavformat/http.c
> > @@ -71,6 +71,7 @@ typedef struct HTTPContext {
> >  char *headers;
> >  char *mime_type;
> >  char *user_agent;
> > +attribute_deprecated char *user_agent_deprecated;
>
> not attribute_deprecated
>
> #if FF_API_HTTP_USER_AGENT
> char *user_agent_deprecated;
> #endif
>
> >  char *content_type;
> >  /* Set if the server correctly handles Connection: close and will
> close
> >   * the connection after feeding us the content. */
> > @@ -130,7 +131,7 @@ static const AVOption options[] = {
> >  { "http_proxy", "set HTTP proxy to tunnel through",
> > OFFSET(http_proxy), AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, D | E },
> >  { "headers", "set custom HTTP headers, can override built in default
> > headers", OFFSET(headers), AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, D
> | E
> > },
> >  { "content_type", "set a specific content type for the POST
> messages",
> > OFFSET(content_type), AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, D | E },
> > -{ "user_agent", "override User-Agent header", OFFSET(user_agent),
> > AV_OPT_TYPE_STRING, { .str = DEFAULT_USER_AGENT }, 0, 0, D },
> > +{ "user_agent", "override User-Agent header",
>
> You want to deprecate user-agent, not user_agent.
>
> #if FF_API_HTTP_USER_AGENT
> { "user-agent", "override User-Agent header", ...
> #endif
>
> > OFFSET(user_agent_deprecated), AV_OPT_TYPE_STRING, { .str =
> > DEFAULT_USER_AGENT }, 0, 0, D },
> >  { "user-agent", "override User-Agent header", OFFSET(user_agent),
> > AV_OPT_TYPE_STRING, { .str = DEFAULT_USER_AGENT }, 0, 0, D },
> >  { "multiple_requests", "use persistent connections",
> > OFFSET(multiple_requests), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, D | E },
> >  { "post_data", "set custom HTTP post data", OFFSET(post_data),
> > AV_OPT_TYPE_BINARY, .flags = D | E },
> > @@ -1037,6 +1038,10 @@ static int http_connect(URLContext *h, const char
> > *path, const char *local_path,
> >  send_expect_100 = 1;
> >  }
> >
> > +if (av_strncasecmp(s->user_agent_deprecated, DEFAULT_USER_AGENT,
> > strlen(DEFAULT_USER_AGENT))) {
>
> - you need to check s->user_agent, not s->user_agent_deprecated
> - strcmp() is enough
>
> > +av_log(s, AV_LOG_WARNING, "the user_agent option is deprecated,
> > please use user-agent option\n");
>
> the other way around
>
> > +s->user_agent = av_strdup(s->user_agent_deprecated);
> > +}
>
> --
> Clément B.
> ___
>





patch update




add a FF_OPT_FLAG_DEPRECATED define for compile.
add a new variable for user_agent to give a deprecated warning message
---
 doc/protocols.texi|  4 +++-
 libavformat/http.c| 14 --
 libavformat/version.h |  3 +++
 3 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/doc/protocols.texi b/doc/protocols.texi
index 

Re: [FFmpeg-devel] [PATCH] avformat/http: remove duplicate user-agent option

[Clément Bœsch]

On Fri, Sep 16, 2016 at 12:04:34AM +0800, Steven Liu wrote:
> 2016-09-15 23:42 GMT+08:00 Clément Bœsch :
> 
> > On Thu, Sep 15, 2016 at 05:39:25PM +0200, Nicolas George wrote:
> > > Le decadi 30 fructidor, an CCXXIV, Clement Boesch a écrit :
> > > > Yes, surrounded by the #ifdefery so it will go away with the option.
> > > >
> > > > Note: use av_strdup() to transfer from one field to the other
> > >
> > > Another possibility would be to implement AV_OPT_FLAG_DEPRECATED and let
> > the
> > > options system itself print the warning.
> > >
> >
> > That's not a bad idea; you might need to add a field to redirect to
> > the new option though.
> >
> > --
> > Clément B.
> > ___
> >
> 
> 
> Hi guys,
> 
> are you mean like this?
> 
> localhost:xxx StevenLiu$ make
> CC libavformat/http.o
> src/libavformat/http.c:1041:27: warning: 'user_agent_deprecated' is
> deprecated [-Wdeprecated-declarations]
> if (av_strncasecmp(s->user_agent_deprecated, DEFAULT_USER_AGENT,
> strlen(DEFAULT_USER_AGENT))) {
>   ^
> src/libavformat/http.c:74:32: note: 'user_agent_deprecated' has been
> explicitly marked deprecated here
> attribute_deprecated char *user_agent_deprecated;
>^
> src/libavformat/http.c:1043:38: warning: 'user_agent_deprecated' is
> deprecated [-Wdeprecated-declarations]
> s->user_agent = av_strdup(s->user_agent_deprecated);
>  ^
> src/libavformat/http.c:74:32: note: 'user_agent_deprecated' has been
> explicitly marked deprecated here
> attribute_deprecated char *user_agent_deprecated;
>^
> 2 warnings generated.
> AR libavformat/libavformat.a
> 
> 
> and the modify like this?
> 
> 
> localhost:xxx StevenLiu$ git diff
> diff --git a/libavformat/http.c b/libavformat/http.c
> index adb3d92..df9cf1a 100644
> --- a/libavformat/http.c
> +++ b/libavformat/http.c
> @@ -71,6 +71,7 @@ typedef struct HTTPContext {
>  char *headers;
>  char *mime_type;
>  char *user_agent;
> +attribute_deprecated char *user_agent_deprecated;

not attribute_deprecated

#if FF_API_HTTP_USER_AGENT
char *user_agent_deprecated;
#endif

>  char *content_type;
>  /* Set if the server correctly handles Connection: close and will close
>   * the connection after feeding us the content. */
> @@ -130,7 +131,7 @@ static const AVOption options[] = {
>  { "http_proxy", "set HTTP proxy to tunnel through",
> OFFSET(http_proxy), AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, D | E },
>  { "headers", "set custom HTTP headers, can override built in default
> headers", OFFSET(headers), AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, D | E
> },
>  { "content_type", "set a specific content type for the POST messages",
> OFFSET(content_type), AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, D | E },
> -{ "user_agent", "override User-Agent header", OFFSET(user_agent),
> AV_OPT_TYPE_STRING, { .str = DEFAULT_USER_AGENT }, 0, 0, D },
> +{ "user_agent", "override User-Agent header",

You want to deprecate user-agent, not user_agent.

#if FF_API_HTTP_USER_AGENT
{ "user-agent", "override User-Agent header", ...
#endif

> OFFSET(user_agent_deprecated), AV_OPT_TYPE_STRING, { .str =
> DEFAULT_USER_AGENT }, 0, 0, D },
>  { "user-agent", "override User-Agent header", OFFSET(user_agent),
> AV_OPT_TYPE_STRING, { .str = DEFAULT_USER_AGENT }, 0, 0, D },
>  { "multiple_requests", "use persistent connections",
> OFFSET(multiple_requests), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, D | E },
>  { "post_data", "set custom HTTP post data", OFFSET(post_data),
> AV_OPT_TYPE_BINARY, .flags = D | E },
> @@ -1037,6 +1038,10 @@ static int http_connect(URLContext *h, const char
> *path, const char *local_path,
>  send_expect_100 = 1;
>  }
> 
> +if (av_strncasecmp(s->user_agent_deprecated, DEFAULT_USER_AGENT,
> strlen(DEFAULT_USER_AGENT))) {

- you need to check s->user_agent, not s->user_agent_deprecated
- strcmp() is enough

> +av_log(s, AV_LOG_WARNING, "the user_agent option is deprecated,
> please use user-agent option\n");

the other way around

> +s->user_agent = av_strdup(s->user_agent_deprecated);
> +}

-- 
Clément B.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avformat/http: remove duplicate user-agent option

[Carl Eugen Hoyos]

2016-09-15 18:04 GMT+02:00 Steven Liu :

> are you mean like this?

Imo, this is missing the (only relevant) change to doc/protocols.texi.

Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avformat/http: remove duplicate user-agent option

[Steven Liu]

2016-09-15 23:42 GMT+08:00 Clément Bœsch :

> On Thu, Sep 15, 2016 at 05:39:25PM +0200, Nicolas George wrote:
> > Le decadi 30 fructidor, an CCXXIV, Clement Boesch a écrit :
> > > Yes, surrounded by the #ifdefery so it will go away with the option.
> > >
> > > Note: use av_strdup() to transfer from one field to the other
> >
> > Another possibility would be to implement AV_OPT_FLAG_DEPRECATED and let
> the
> > options system itself print the warning.
> >
>
> That's not a bad idea; you might need to add a field to redirect to
> the new option though.
>
> --
> Clément B.
> ___
>


Hi guys,

are you mean like this?

localhost:xxx StevenLiu$ make
CC libavformat/http.o
src/libavformat/http.c:1041:27: warning: 'user_agent_deprecated' is
deprecated [-Wdeprecated-declarations]
if (av_strncasecmp(s->user_agent_deprecated, DEFAULT_USER_AGENT,
strlen(DEFAULT_USER_AGENT))) {
  ^
src/libavformat/http.c:74:32: note: 'user_agent_deprecated' has been
explicitly marked deprecated here
attribute_deprecated char *user_agent_deprecated;
   ^
src/libavformat/http.c:1043:38: warning: 'user_agent_deprecated' is
deprecated [-Wdeprecated-declarations]
s->user_agent = av_strdup(s->user_agent_deprecated);
 ^
src/libavformat/http.c:74:32: note: 'user_agent_deprecated' has been
explicitly marked deprecated here
attribute_deprecated char *user_agent_deprecated;
   ^
2 warnings generated.
AR libavformat/libavformat.a


and the modify like this?


localhost:xxx StevenLiu$ git diff
diff --git a/libavformat/http.c b/libavformat/http.c
index adb3d92..df9cf1a 100644
--- a/libavformat/http.c
+++ b/libavformat/http.c
@@ -71,6 +71,7 @@ typedef struct HTTPContext {
 char *headers;
 char *mime_type;
 char *user_agent;
+attribute_deprecated char *user_agent_deprecated;
 char *content_type;
 /* Set if the server correctly handles Connection: close and will close
  * the connection after feeding us the content. */
@@ -130,7 +131,7 @@ static const AVOption options[] = {
 { "http_proxy", "set HTTP proxy to tunnel through",
OFFSET(http_proxy), AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, D | E },
 { "headers", "set custom HTTP headers, can override built in default
headers", OFFSET(headers), AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, D | E
},
 { "content_type", "set a specific content type for the POST messages",
OFFSET(content_type), AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, D | E },
-{ "user_agent", "override User-Agent header", OFFSET(user_agent),
AV_OPT_TYPE_STRING, { .str = DEFAULT_USER_AGENT }, 0, 0, D },
+{ "user_agent", "override User-Agent header",
OFFSET(user_agent_deprecated), AV_OPT_TYPE_STRING, { .str =
DEFAULT_USER_AGENT }, 0, 0, D },
 { "user-agent", "override User-Agent header", OFFSET(user_agent),
AV_OPT_TYPE_STRING, { .str = DEFAULT_USER_AGENT }, 0, 0, D },
 { "multiple_requests", "use persistent connections",
OFFSET(multiple_requests), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, D | E },
 { "post_data", "set custom HTTP post data", OFFSET(post_data),
AV_OPT_TYPE_BINARY, .flags = D | E },
@@ -1037,6 +1038,10 @@ static int http_connect(URLContext *h, const char
*path, const char *local_path,
 send_expect_100 = 1;
 }

+if (av_strncasecmp(s->user_agent_deprecated, DEFAULT_USER_AGENT,
strlen(DEFAULT_USER_AGENT))) {
+av_log(s, AV_LOG_WARNING, "the user_agent option is deprecated,
please use user-agent option\n");
+s->user_agent = av_strdup(s->user_agent_deprecated);
+}
 /* set default headers if needed */
 if (!has_header(s->headers, "\r\nUser-Agent: "))
 len += av_strlcatf(headers + len, sizeof(headers) - len,
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avformat/http: remove duplicate user-agent option

[Clément Bœsch]

On Thu, Sep 15, 2016 at 05:39:25PM +0200, Nicolas George wrote:
> Le decadi 30 fructidor, an CCXXIV, Clement Boesch a écrit :
> > Yes, surrounded by the #ifdefery so it will go away with the option.
> > 
> > Note: use av_strdup() to transfer from one field to the other
> 
> Another possibility would be to implement AV_OPT_FLAG_DEPRECATED and let the
> options system itself print the warning.
> 

That's not a bad idea; you might need to add a field to redirect to
the new option though.

-- 
Clément B.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avformat/http: remove duplicate user-agent option

[Nicolas George]

Le decadi 30 fructidor, an CCXXIV, Clement Boesch a écrit :
> Yes, surrounded by the #ifdefery so it will go away with the option.
> 
> Note: use av_strdup() to transfer from one field to the other

Another possibility would be to implement AV_OPT_FLAG_DEPRECATED and let the
options system itself print the warning.

Regards,

-- 
  Nicolas George


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avformat/http: remove duplicate user-agent option

[Clément Bœsch]

On Thu, Sep 15, 2016 at 11:27:02PM +0800, Steven Liu wrote:
> 2016-09-15 23:03 GMT+08:00 Carl Eugen Hoyos :
> 
> > 2016-09-15 16:40 GMT+02:00 Clément Bœsch :
> >
> > >> Carl Eugen's mean is apply this patch after the next release?
> > >
> > > You need to deprecate the option with at least the classic #ifdefery
> > dance
> > > (git grep FF_API for examples), and ideally by also printing a warning
> > > when using the wrong option.
> >
> > Imo, it is completely sufficient to add a line to the filter documentation
> > (and print a warning if this is easily possible).
> >
> > Carl Eugen
> >
> >
> user_agent and user-agent use the same variable 'user_agent', if print a
> warning, maybe add a user variable for  user_agent option is a good way?

Yes, surrounded by the #ifdefery so it will go away with the option.

Note: use av_strdup() to transfer from one field to the other

-- 
Clément B.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avformat/http: remove duplicate user-agent option

[Steven Liu]

2016-09-15 23:03 GMT+08:00 Carl Eugen Hoyos :

> 2016-09-15 16:40 GMT+02:00 Clément Bœsch :
>
> >> Carl Eugen's mean is apply this patch after the next release?
> >
> > You need to deprecate the option with at least the classic #ifdefery
> dance
> > (git grep FF_API for examples), and ideally by also printing a warning
> > when using the wrong option.
>
> Imo, it is completely sufficient to add a line to the filter documentation
> (and print a warning if this is easily possible).
>
> Carl Eugen
>
>
user_agent and user-agent use the same variable 'user_agent', if print a
warning, maybe add a user variable for  user_agent option is a good way?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avformat/http: remove duplicate user-agent option

[Carl Eugen Hoyos]

2016-09-15 16:40 GMT+02:00 Clément Bœsch :

>> Carl Eugen's mean is apply this patch after the next release?
>
> You need to deprecate the option with at least the classic #ifdefery dance
> (git grep FF_API for examples), and ideally by also printing a warning
> when using the wrong option.

Imo, it is completely sufficient to add a line to the filter documentation
(and print a warning if this is easily possible).

Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avformat/http: remove duplicate user-agent option

[Clément Bœsch]

On Thu, Sep 15, 2016 at 10:34:37PM +0800, Steven Liu wrote:
> 2016-09-15 22:22 GMT+08:00 Carl Eugen Hoyos :
> 
> > 2016-09-15 16:04 GMT+02:00 Steven Liu :
> > > double user-agent option, same option, remove one.
> >
> > Looking at the documentation, the patch is not ok as-is.
> >
> > I suggest to deprecate the option now, and to remove it after
> > the next release.
> >
> > Carl Eugen
> >
> >
> Carl Eugen's mean is apply this patch after the next release?

You need to deprecate the option with at least the classic #ifdefery dance
(git grep FF_API for examples), and ideally by also printing a warning
when using the wrong option.

-- 
Clément B.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avformat/http: remove duplicate user-agent option

[Steven Liu]

2016-09-15 22:22 GMT+08:00 Carl Eugen Hoyos :

> 2016-09-15 16:04 GMT+02:00 Steven Liu :
> > double user-agent option, same option, remove one.
>
> Looking at the documentation, the patch is not ok as-is.
>
> I suggest to deprecate the option now, and to remove it after
> the next release.
>
> Carl Eugen
>
>
Carl Eugen's mean is apply this patch after the next release?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avformat/http: remove duplicate user-agent option

[Carl Eugen Hoyos]

2016-09-15 16:04 GMT+02:00 Steven Liu :
> double user-agent option, same option, remove one.

Looking at the documentation, the patch is not ok as-is.

I suggest to deprecate the option now, and to remove it after
the next release.

Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avformat/http: remove duplicate user-agent option

[Clément Bœsch]

On Thu, Sep 15, 2016 at 10:04:48PM +0800, Steven Liu wrote:
> double user-agent option, same option, remove one.
> 
> Signed-off-by: Steven Liu 
> ---
>  libavformat/http.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/libavformat/http.c b/libavformat/http.c
> index adb3d92..f2bfb17 100644
> --- a/libavformat/http.c
> +++ b/libavformat/http.c
> @@ -131,7 +131,6 @@ static const AVOption options[] = {
>  { "headers", "set custom HTTP headers, can override built in default
> headers", OFFSET(headers), AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, D | E
> },
>  { "content_type", "set a specific content type for the POST messages",
> OFFSET(content_type), AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, D | E },
>  { "user_agent", "override User-Agent header", OFFSET(user_agent),
> AV_OPT_TYPE_STRING, { .str = DEFAULT_USER_AGENT }, 0, 0, D },
> -{ "user-agent", "override User-Agent header", OFFSET(user_agent),

one has '_', the other '-'

-- 
Clément B.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] avformat/http: remove duplicate user-agent option

[Steven Liu]

double user-agent option, same option, remove one.

Signed-off-by: Steven Liu 
---
 libavformat/http.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/libavformat/http.c b/libavformat/http.c
index adb3d92..f2bfb17 100644
--- a/libavformat/http.c
+++ b/libavformat/http.c
@@ -131,7 +131,6 @@ static const AVOption options[] = {
 { "headers", "set custom HTTP headers, can override built in default
headers", OFFSET(headers), AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, D | E
},
 { "content_type", "set a specific content type for the POST messages",
OFFSET(content_type), AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, D | E },
 { "user_agent", "override User-Agent header", OFFSET(user_agent),
AV_OPT_TYPE_STRING, { .str = DEFAULT_USER_AGENT }, 0, 0, D },
-{ "user-agent", "override User-Agent header", OFFSET(user_agent),
AV_OPT_TYPE_STRING, { .str = DEFAULT_USER_AGENT }, 0, 0, D },
 { "multiple_requests", "use persistent connections",
OFFSET(multiple_requests), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, D | E },
 { "post_data", "set custom HTTP post data", OFFSET(post_data),
AV_OPT_TYPE_BINARY, .flags = D | E },
 { "mime_type", "export the MIME type", OFFSET(mime_type),
AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, AV_OPT_FLAG_EXPORT |
AV_OPT_FLAG_READONLY },
-- 
2.7.4 (Apple Git-66)


0001-avformat-http-remove-duplicate-user-agent-option.patch
Description: Binary data
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel