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

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Reply via email to