diff mbox series

[FFmpeg-devel] ffmpeg_opt: inform user about -b:a and -b:v instead of legacy form

Message ID 20231209191002.680650-1-marth64@proxyid.net
State New
Headers show
Series [FFmpeg-devel] ffmpeg_opt: inform user about -b:a and -b:v instead of legacy form | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Marth64 Dec. 9, 2023, 7:10 p.m. UTC 
---
 fftools/ffmpeg_opt.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

James Almer Dec. 9, 2023, 7:24 p.m. UTC | #1 
On 12/9/2023 4:10 PM, Marth64 wrote:
> ---
>   fftools/ffmpeg_opt.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
> index 304471dd03..5e50e5efa2 100644
> --- a/fftools/ffmpeg_opt.c
> +++ b/fftools/ffmpeg_opt.c
> @@ -1721,8 +1721,8 @@ const OptionDef options[] = {
>       { "force_key_frames", OPT_VIDEO | OPT_STRING | HAS_ARG | OPT_EXPERT |
>                             OPT_SPEC | OPT_OUTPUT,                                 { .off = OFFSET(forced_key_frames) },
>           "force key frames at specified timestamps", "timestamps" },
> -    { "b",            OPT_VIDEO | HAS_ARG | OPT_PERFILE | OPT_OUTPUT,            { .func_arg = opt_bitrate },
> -        "video bitrate (please use -b:v)", "bitrate" },
> +    { "b:v",          OPT_VIDEO | HAS_ARG | OPT_PERFILE | OPT_OUTPUT,            { .func_arg = opt_bitrate },
> +        "video bitrate", "bitrate" },

"b" is for bitrate regardless of media type. The specifier that comes 
after it decides the type. You're probably breaking that by making "b" 
no longer be a valid option.

IMO, change the description to "Stream bitrate".

>       { "hwaccel",          OPT_VIDEO | OPT_STRING | HAS_ARG | OPT_EXPERT |
>                             OPT_SPEC | OPT_INPUT,                                  { .off = OFFSET(hwaccels) },
>           "use HW accelerated decoding", "hwaccel name" },
> @@ -1762,8 +1762,8 @@ const OptionDef options[] = {
>       { "acodec",         OPT_AUDIO | HAS_ARG  | OPT_PERFILE |
>                           OPT_INPUT | OPT_OUTPUT,                                    { .func_arg = opt_audio_codec },
>           "force audio codec ('copy' to copy stream)", "codec" },
> -    { "ab",             OPT_AUDIO | HAS_ARG | OPT_PERFILE | OPT_OUTPUT,            { .func_arg = opt_bitrate },
> -        "audio bitrate (please use -b:a)", "bitrate" },
> +    { "b:a",            OPT_AUDIO | HAS_ARG | OPT_PERFILE | OPT_OUTPUT,            { .func_arg = opt_bitrate },
> +        "audio bitrate", "bitrate" },

And this just removes "ab" altogether, which is a break with no previous 
warning. b:a already works thanks to "b" above.

Just flag this option as deprecated.

>       { "atag",           OPT_AUDIO | HAS_ARG  | OPT_EXPERT | OPT_PERFILE |
>                           OPT_OUTPUT,                                                { .func_arg = opt_old2new },
>           "force audio tag/fourcc", "fourcc/tag" },
Marth64 Dec. 9, 2023, 7:32 p.m. UTC | #2 
Roger, will update. Thanks!

On Sat, Dec 9, 2023 at 1:25 PM James Almer <jamrial@gmail.com> wrote:

> On 12/9/2023 4:10 PM, Marth64 wrote:
> > ---
> >   fftools/ffmpeg_opt.c | 8 ++++----
> >   1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
> > index 304471dd03..5e50e5efa2 100644
> > --- a/fftools/ffmpeg_opt.c
> > +++ b/fftools/ffmpeg_opt.c
> > @@ -1721,8 +1721,8 @@ const OptionDef options[] = {
> >       { "force_key_frames", OPT_VIDEO | OPT_STRING | HAS_ARG |
> OPT_EXPERT |
> >                             OPT_SPEC | OPT_OUTPUT,
>            { .off = OFFSET(forced_key_frames) },
> >           "force key frames at specified timestamps", "timestamps" },
> > -    { "b",            OPT_VIDEO | HAS_ARG | OPT_PERFILE | OPT_OUTPUT,
>           { .func_arg = opt_bitrate },
> > -        "video bitrate (please use -b:v)", "bitrate" },
> > +    { "b:v",          OPT_VIDEO | HAS_ARG | OPT_PERFILE | OPT_OUTPUT,
>           { .func_arg = opt_bitrate },
> > +        "video bitrate", "bitrate" },
>
> "b" is for bitrate regardless of media type. The specifier that comes
> after it decides the type. You're probably breaking that by making "b"
> no longer be a valid option.
>
> IMO, change the description to "Stream bitrate".
>
> >       { "hwaccel",          OPT_VIDEO | OPT_STRING | HAS_ARG |
> OPT_EXPERT |
> >                             OPT_SPEC | OPT_INPUT,
>           { .off = OFFSET(hwaccels) },
> >           "use HW accelerated decoding", "hwaccel name" },
> > @@ -1762,8 +1762,8 @@ const OptionDef options[] = {
> >       { "acodec",         OPT_AUDIO | HAS_ARG  | OPT_PERFILE |
> >                           OPT_INPUT | OPT_OUTPUT,
>             { .func_arg = opt_audio_codec },
> >           "force audio codec ('copy' to copy stream)", "codec" },
> > -    { "ab",             OPT_AUDIO | HAS_ARG | OPT_PERFILE |
> OPT_OUTPUT,            { .func_arg = opt_bitrate },
> > -        "audio bitrate (please use -b:a)", "bitrate" },
> > +    { "b:a",            OPT_AUDIO | HAS_ARG | OPT_PERFILE |
> OPT_OUTPUT,            { .func_arg = opt_bitrate },
> > +        "audio bitrate", "bitrate" },
>
> And this just removes "ab" altogether, which is a break with no previous
> warning. b:a already works thanks to "b" above.
>
> Just flag this option as deprecated.
>
> >       { "atag",           OPT_AUDIO | HAS_ARG  | OPT_EXPERT |
> OPT_PERFILE |
> >                           OPT_OUTPUT,
>             { .func_arg = opt_old2new },
> >           "force audio tag/fourcc", "fourcc/tag" },
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
diff mbox series

Patch

diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
index 304471dd03..5e50e5efa2 100644
--- a/fftools/ffmpeg_opt.c
+++ b/fftools/ffmpeg_opt.c
@@ -1721,8 +1721,8 @@  const OptionDef options[] = {
     { "force_key_frames", OPT_VIDEO | OPT_STRING | HAS_ARG | OPT_EXPERT |
                           OPT_SPEC | OPT_OUTPUT,                                 { .off = OFFSET(forced_key_frames) },
         "force key frames at specified timestamps", "timestamps" },
-    { "b",            OPT_VIDEO | HAS_ARG | OPT_PERFILE | OPT_OUTPUT,            { .func_arg = opt_bitrate },
-        "video bitrate (please use -b:v)", "bitrate" },
+    { "b:v",          OPT_VIDEO | HAS_ARG | OPT_PERFILE | OPT_OUTPUT,            { .func_arg = opt_bitrate },
+        "video bitrate", "bitrate" },
     { "hwaccel",          OPT_VIDEO | OPT_STRING | HAS_ARG | OPT_EXPERT |
                           OPT_SPEC | OPT_INPUT,                                  { .off = OFFSET(hwaccels) },
         "use HW accelerated decoding", "hwaccel name" },
@@ -1762,8 +1762,8 @@  const OptionDef options[] = {
     { "acodec",         OPT_AUDIO | HAS_ARG  | OPT_PERFILE |
                         OPT_INPUT | OPT_OUTPUT,                                    { .func_arg = opt_audio_codec },
         "force audio codec ('copy' to copy stream)", "codec" },
-    { "ab",             OPT_AUDIO | HAS_ARG | OPT_PERFILE | OPT_OUTPUT,            { .func_arg = opt_bitrate },
-        "audio bitrate (please use -b:a)", "bitrate" },
+    { "b:a",            OPT_AUDIO | HAS_ARG | OPT_PERFILE | OPT_OUTPUT,            { .func_arg = opt_bitrate },
+        "audio bitrate", "bitrate" },
     { "atag",           OPT_AUDIO | HAS_ARG  | OPT_EXPERT | OPT_PERFILE |
                         OPT_OUTPUT,                                                { .func_arg = opt_old2new },
         "force audio tag/fourcc", "fourcc/tag" },