[FFmpeg-devel] [PATCH 2/3] avformat: remove more unneeded avio_flush() calls

[Martin Storsjö]

On Fri, 3 Jan 2020, Marton Balint wrote:

> Throughout libavformat there are lots of avio_flush() calls which are unneeded:
> - Instances found at the end of write_header, write_packet and write_trailer
>  callbacks. These are handled by the generic code in libavformat/mux.c.

They are only handled by the generic code, if the flush_packets flag is 
set. If it isn't, and the flush was there for a reason (I'm sure not all 
of them are, but I'm also quite sure a few of them are there for very 
specific reasons), things will now break.

One such case that you're removing comes from 
8ad5124b7ecf7f727724e270a7b4bb8c7bcbf6a4 which was added for a specific 
reason.

> - Instances in the middle of write_header and write_trailer which are
>  redundant or are present becuase avio_flush() used to be required before
>  doing a seekback. That is no longer the case, aviobuf code does the flush
>  automatically on seek.

It's not necessarily about flushing before doing seekback, it's also about 
ensuring that seekback can be done within the current buffer.

For streaming output (where we can't seek back once data has been 
flushed), it's cruicial to flush _before_ a point you want to seek to. 
Consider if we have a 32k avio buffer, and it's filled up to 30k at the 
moment. We're going to write a 8k structure which requires seeks back and 
forth. If we remove the pre-write-flush, we'll write 2k of data, flush it 
out, and later seeks to the beginning of this block will fail.

If we explicitly flush before starting to write one block where we know 
we'll need to seek to, we maximize the chances of it working. (If we need 
to seek across a larger area than the avio buffer, then it still won't 
work of course.)

I didn't check yet all of the ones you are removing, but I'd say at least 
that movenc.c has cases of intentional flushing in this style.

> So this patch removes those cases. Removing explicit avio_flush() calls helps
> us to buffer more data and avoid flushing the IO context too often which causes
> reduced IO throughput for non-streamed file output.

So if you're arguing that some can be removed because the generic code in 
mux.c does the same (although it only does so if a nondefault flag is 
set?), this benefit can only be attributed to the other ones, that are 
removed from the middle of functions.

// Martin