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

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

[Andreas Rheinhardt]

Jan Ekström:
> On Tue, Nov 9, 2021 at 8:05 PM Andreas Rheinhardt
> <andreas.rheinhardt@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@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.
> 

Applied with these changes.

- Andreas

_______________________________________________
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".