[FFmpeg-devel] [PATCH 17/17] fftools/ffmpeg_opt: Apply copyinkf for all stream types

[Jan Ekström]

On Tue, Nov 9, 2021 at 8:05 PM Andreas Rheinhardt
<andreas.rheinhardt at outlook.com> wrote:
>
> The earlier code has ignored it for all stream types except
> video and subtitles, probably because audio was presumed
> to only consist of keyframes. Yet this assumption is not true
> for e.g. TrueHD.
>
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at outlook.com>
> ---
>  fftools/ffmpeg_opt.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
> index ab4c63a362..60ee6b16b5 100644
> --- a/fftools/ffmpeg_opt.c
> +++ b/fftools/ffmpeg_opt.c
> @@ -1655,6 +1655,9 @@ static OutputStream *new_output_stream(OptionsContext *o, AVFormatContext *oc, e
>      ost->muxing_queue = av_fifo_alloc(8 * sizeof(AVPacket));
>      if (!ost->muxing_queue)
>          exit_program(1);
> +    if (ost->stream_copy)
> +        MATCH_PER_STREAM_OPT(copy_initial_nonkeyframes, i,
> +                             ost->copy_initial_nonkeyframes, oc, st);

Could use an extra empty line after the previous if that leads to
exit_program since the two are not related/grouped.

I think we can follow the way it was done for subtitle streams
previously, and just skip the ost->stream_copy check? There is no
extra initialization done for this logic, and having the correct value
always set in "ost" should not be a bad thing (unless I've missed
something with my quick check-around)

Otherwise LGTM for me.

Jan