[FFmpeg-devel] [PATCH 21/24] ffmpeg_mux: split of_write_packet()

[Andreas Rheinhardt]

Anton Khirnov:
> Quoting Andreas Rheinhardt (2021-12-17 00:42:25)
>> Anton Khirnov:
>>> diff --git a/fftools/ffmpeg_mux.c b/fftools/ffmpeg_mux.c
>>> index d4b674c9e2..e97ec8ab93 100644
>>> --- a/fftools/ffmpeg_mux.c
>>> +++ b/fftools/ffmpeg_mux.c
>>> @@ -102,39 +102,12 @@ static int queue_packet(OutputFile *of, OutputStream *ost, AVPacket *pkt)
>>>      return 0;
>>>  }
>>>  
>>> -void of_write_packet(OutputFile *of, AVPacket *pkt, OutputStream *ost,
>>> -                     int unqueue)
>>> +static void write_packet(OutputFile *of, OutputStream *ost, AVPacket *pkt)
>>>  {
>>>      AVFormatContext *s = of->ctx;
>>>      AVStream *st = ost->st;
>>>      int ret;
>>>  
>>> -    /*
>>> -     * Audio encoders may split the packets --  #frames in != #packets out.
>>> -     * But there is no reordering, so we can limit the number of output packets
>>> -     * by simply dropping them here.
>>> -     * Counting encoded video frames needs to be done separately because of
>>> -     * reordering, see do_video_out().
>>> -     * Do not count the packet when unqueued because it has been counted when queued.
>>> -     */
>>> -    if (!(st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO && ost->encoding_needed) && !unqueue) {
>>> -        if (ost->frame_number >= ost->max_frames) {
>>> -            av_packet_unref(pkt);
>>> -            return;
>>> -        }
>>> -        ost->frame_number++;
>>> -    }
>>
>> Factoring this chunk out of write_packet() (effectively inlining
>> unqueue) looks good (and I actually pondered it myself),
>>
>>> -
>>> -    /* the muxer is not initialized yet, buffer the packet */
>>> -    if (!of->mux->header_written) {
>>> -        ret = queue_packet(of, ost, pkt);
>>> -        if (ret < 0) {
>>> -            av_packet_unref(pkt);
>>> -            exit_program(1);
>>> -        }
>>> -        return;
>>> -    }
>>> -
>>
>> but I could not prove that the header has already been written in case
>> unqueue == 0. Can you guarantee this to be so and explain it to me?
> 
> I don't understand what you mean. unqueue == 0 in the current code tells
> you nothing about whether the header has been written.
> How does that relate to the patch?
> 

Wait, I misread this: You keep the header_written check for the two
callers in output_packet(), but remove it for the one caller for which
we know that initialization has already happened. This is good. Sorry
for the confusion.

- Andreas