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

2016-09-15 23:42 GMT+08:00 Clément Bœsch <u@pkh.me>:

> 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
-    { "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,

2016-09-15 18:04 GMT+02:00 Steven Liu <lingjiujianke@gmail.com>:

>     are you mean like this?

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

Carl Eugen

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 <u@pkh.me>:
> 
> > 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);
> +    }