[FFmpeg-devel] [PATCH] Revert "ffmpeg: force 128k default audio bitrate if nothing is specified and there is no specific default"

[Andreas Rheinhardt]

Andreas Rheinhardt:
> This reverts commit 628a73f8f3768513fa6152c98d54842cf2ae1aad.
> 
> At the time of said commit there was talk of removing the audio bitrate
> "ab" option to bring FFmpeg in line with what Libav has done in 2012 in
> commit 041cd5a0c55e02dd3b9a2419644b03103819c3d3. By having different
> option flags for the "ab" and the ordinay bitrate "b" option is is
> possible to have different default bitrates for audio and video. In
> order to maintain this behaviour and not break user scripts the commit
> to be reverted added code to ffmpeg.c that set the bitrate value to the
> audio default for audio codecs, but only if AVCodec.defaults didn't
> exist (as in this case the default would be codec-default and not
> affected by the "ab" removal).
> 
> This had the downside of being an API violation, because
> AVCodec.defaults is not a public field. Given that the "ab" option
> and its audio-specific default value have never been removed,
> said API violation can be simply fixed by reverting said commit.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at outlook.com>
> ---
> There are a few more instances of fftools violating the API besides
> this one and the one in [1]:
> a) ffprobe accesses AVProgram.start_time and AVProgram.end_time.
> I only see two options here: Don't report these values or make
> them public.
> b) cmdutils accesses AVFilter.process_command. This could be fixed
> by adding an AVFILTER_FLAG_SUPPORT_COMMANDS and checking for that
> instead.
> c) ffplay checks for AVInputFormat.read_seek and the seeking flags
> to determine whether seeking is available. The way it does this seems
> wrong: It checks for whether any (not all!) of the default seeking methods
> (binary, generic, byte seek) is disabled and for whether there is
> a dedicated read_seek function. This could be fixed by adding a flag
> for AVInputFormats that tells whether seeking is supported
> (we could even distinguish the cases of "files of this format are
> usually well seekable" (e.g. because they usually contain an index)
> and "seeking is not that good, but works" (because e.g. it is just
> the generic seeking code that reads all packets linearly).
> 
> Given that the seeking flags (with the exception of AVFMT_NO_BYTE_SEEK)
> are actually implementation details that are probably only public
> because there were no internal flags for AVInputFormat until recently
> this should be done at the same time as deprecating these fields.
> 
> [1]: https://ffmpeg.org/pipermail/ffmpeg-devel/2021-September/284647.html
> 
>  fftools/ffmpeg.c | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
> index b0ce7c7c32..ecff156529 100644
> --- a/fftools/ffmpeg.c
> +++ b/fftools/ffmpeg.c
> @@ -3571,11 +3571,6 @@ static int init_output_stream(OutputStream *ost, AVFrame *frame,
>          }
>          if (!av_dict_get(ost->encoder_opts, "threads", NULL, 0))
>              av_dict_set(&ost->encoder_opts, "threads", "auto", 0);
> -        if (ost->enc->type == AVMEDIA_TYPE_AUDIO &&
> -            !codec->defaults &&
> -            !av_dict_get(ost->encoder_opts, "b", NULL, 0) &&
> -            !av_dict_get(ost->encoder_opts, "ab", NULL, 0))
> -            av_dict_set(&ost->encoder_opts, "b", "128000", 0);
>  
>          ret = hw_device_setup_for_encode(ost);
>          if (ret < 0) {
> 
Will apply this tomorrow unless there are objections. I am also still
seeking opinions on the above mentioned issues.

- Andreas