| Message ID | CADxeRw=WrUsCfBEs_htfTn+tOhVnEARNLGmumjwUAjSv7YwR0A@mail.gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
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 <lingjiujianke@gmail.com> > --- > 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 '-'
2016-09-15 16:04 GMT+02:00 Steven Liu <lingjiujianke@gmail.com>:
> 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
2016-09-15 22:21 GMT+08:00 Clément Bœsch <u@pkh.me>: > 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 <lingjiujianke@gmail.com> > > --- > > 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. > all right, i ignore the '_' . ignore this patch please.
2016-09-15 22:22 GMT+08:00 Carl Eugen Hoyos <ceffmpeg@gmail.com>: > 2016-09-15 16:04 GMT+02:00 Steven Liu <lingjiujianke@gmail.com>: > > 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?
On Thu, Sep 15, 2016 at 10:34:37PM +0800, Steven Liu wrote: > 2016-09-15 22:22 GMT+08:00 Carl Eugen Hoyos <ceffmpeg@gmail.com>: > > > 2016-09-15 16:04 GMT+02:00 Steven Liu <lingjiujianke@gmail.com>: > > > 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.
2016-09-15 16:40 GMT+02:00 Clément Bœsch <u@pkh.me>: >> 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
2016-09-15 23:03 GMT+08:00 Carl Eugen Hoyos <ceffmpeg@gmail.com>: > 2016-09-15 16:40 GMT+02:00 Clément Bœsch <u@pkh.me>: > > >> 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?
On Thu, Sep 15, 2016 at 11:27:02PM +0800, Steven Liu wrote: > 2016-09-15 23:03 GMT+08:00 Carl Eugen Hoyos <ceffmpeg@gmail.com>: > > > 2016-09-15 16:40 GMT+02:00 Clément Bœsch <u@pkh.me>: > > > > >> 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
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,
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.
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 },
double user-agent option, same option, remove one. Signed-off-by: Steven Liu <lingjiujianke@gmail.com> --- libavformat/http.c | 1 - 1 file changed, 1 deletion(-) - { "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 },