| Message ID | 20200103181548.20535-2-cus@passwd.hu |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/3] avformat: remove unneded avio_flush() calls before calling avio_close_dyn_buf() | expand |
| Context | Check | Description |
|---|---|---|
| andriy/ffmpeg-patchwork | success | Make fate finished |
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
On Fri, 3 Jan 2020, Martin Storsjö wrote: > 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. For write_packet, you are right, it removes the explicit flush and fall backs to the default behaviour of the flush_packets flag, which is to flush if the output is streamed, and not flush otherwise. Can you a think of a case which breaks when the output is not streamed? For write_header there is an explicit flush in avio_write_marker if the marker type is AVIO_DATA_MARKER_HEADER and if the type is different to the existing marker, so it should not make a difference. > > One such case that you're removing comes from > 8ad5124b7ecf7f727724e270a7b4bb8c7bcbf6a4 which was added for a specific > reason. I guess this predates your avio_write_marker addition. > >> - 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. Ok, this can be a problem indeed. A proper solution would be to use a dynbuf in these cases if streaming is of importance because it is wrong to assume any particular output buffer size in a muxer and rely on this behaviour. >> 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. Yes. I will rework the patchset, sepearte the cases better and maybe keep the ones in the middle write_header/write_trailer for now. Thanks, Marton
On Fri, 3 Jan 2020, Marton Balint wrote: > On Fri, 3 Jan 2020, Martin Storsjö wrote: > >> 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. > > For write_packet, you are right, it removes the explicit flush and fall > backs to the default behaviour of the flush_packets flag, which is to > flush if the output is streamed, and not flush otherwise. Can you a think > of a case which breaks when the output is not streamed? Hmm, not really > For write_header there is an explicit flush in avio_write_marker if the > marker type is AVIO_DATA_MARKER_HEADER and if the type is different to the > existing marker, so it should not make a difference. Ah, I overlooked that - then it might be fine. >> >> One such case that you're removing comes from >> 8ad5124b7ecf7f727724e270a7b4bb8c7bcbf6a4 which was added for a specific >> reason. > > I guess this predates your avio_write_marker addition. Yeah, I guess so. >> >>> - 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. > > Ok, this can be a problem indeed. A proper solution would be to use a > dynbuf in these cases if streaming is of importance because it is wrong to > assume any particular output buffer size in a muxer and rely on this > behaviour. Yes, that would be better indeed. For the case with movenc, I think I think I might have taken this route in order to reduce the amount of needed refactoring as this method felt good enough for the cases at hand at the time. >>> 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. > > Yes. I will rework the patchset, sepearte the cases better and maybe keep > the ones in the middle write_header/write_trailer for now. Awesome, that'd make me more comfortable with it! // Martin
diff --git a/libavformat/aiffenc.c b/libavformat/aiffenc.c index e25794d185..0145596bec 100644 --- a/libavformat/aiffenc.c +++ b/libavformat/aiffenc.c @@ -199,9 +199,6 @@ static int aiff_write_header(AVFormatContext *s) avpriv_set_pts_info(s->streams[aiff->audio_stream_idx], 64, 1, s->streams[aiff->audio_stream_idx]->codecpar->sample_rate); - /* Data is starting here */ - avio_flush(pb); - return 0; } @@ -266,8 +263,6 @@ static int aiff_write_trailer(AVFormatContext *s) file_size = avio_tell(pb); avio_seek(pb, aiff->form, SEEK_SET); avio_wb32(pb, file_size - aiff->form - 4); - - avio_flush(pb); } return ret; diff --git a/libavformat/amr.c b/libavformat/amr.c index 42840a50a3..650b565b1b 100644 --- a/libavformat/amr.c +++ b/libavformat/amr.c @@ -60,7 +60,6 @@ static int amr_write_header(AVFormatContext *s) } else { return -1; } - avio_flush(pb); return 0; } diff --git a/libavformat/asfenc.c b/libavformat/asfenc.c index 44e11fc763..8eaa9d40ce 100644 --- a/libavformat/asfenc.c +++ b/libavformat/asfenc.c @@ -801,8 +801,6 @@ static int asf_write_header(AVFormatContext *s) return -1; } - avio_flush(s->pb); - asf->packet_nb_payloads = 0; asf->packet_timestamp_start = -1; asf->packet_timestamp_end = -1; @@ -1132,7 +1130,6 @@ static int asf_write_trailer(AVFormatContext *s) return ret; asf_write_index(s, asf->index_ptr, asf->maximum_packet, asf->next_start_sec); } - avio_flush(s->pb); if (asf->is_streamed || !(s->pb->seekable & AVIO_SEEKABLE_NORMAL)) { put_chunk(s, 0x4524, 0, 0); /* end of stream */ diff --git a/libavformat/assenc.c b/libavformat/assenc.c index 12aadca171..68c3396e5a 100644 --- a/libavformat/assenc.c +++ b/libavformat/assenc.c @@ -77,7 +77,6 @@ static int write_header(AVFormatContext *s) avio_printf(s->pb, "[Events]\r\nFormat: %s, Start, End, Style, Name, MarginL, MarginR, MarginV, Effect, Text\r\n", ass->ssa_mode ? "Marked" : "Layer"); } - avio_flush(s->pb); return 0; } diff --git a/libavformat/astenc.c b/libavformat/astenc.c index 578e658891..e0b94b8b63 100644 --- a/libavformat/astenc.c +++ b/libavformat/astenc.c @@ -101,8 +101,6 @@ static int ast_write_header(AVFormatContext *s) avio_wb64(pb, 0); avio_wb32(pb, 0); - avio_flush(pb); - return 0; } @@ -180,7 +178,6 @@ static int ast_write_trailer(AVFormatContext *s) } avio_seek(pb, file_size, SEEK_SET); - avio_flush(pb); } return 0; } diff --git a/libavformat/au.c b/libavformat/au.c index cb48e67feb..4afee85a94 100644 --- a/libavformat/au.c +++ b/libavformat/au.c @@ -311,7 +311,6 @@ static int au_write_header(AVFormatContext *s) } else { avio_wb64(pb, 0); /* annotation field */ } - avio_flush(pb); return 0; } @@ -327,7 +326,6 @@ static int au_write_trailer(AVFormatContext *s) avio_seek(pb, 8, SEEK_SET); avio_wb32(pb, (uint32_t)(file_size - au->header_size)); avio_seek(pb, file_size, SEEK_SET); - avio_flush(pb); } return 0; diff --git a/libavformat/avienc.c b/libavformat/avienc.c index 940ea33510..d6cfb1b783 100644 --- a/libavformat/avienc.c +++ b/libavformat/avienc.c @@ -580,8 +580,6 @@ static int avi_write_header(AVFormatContext *s) avi->movi_list = ff_start_tag(pb, "LIST"); ffio_wfourcc(pb, "movi"); - avio_flush(pb); - return 0; } @@ -593,7 +591,6 @@ static void update_odml_entry(AVFormatContext *s, int stream_index, int64_t ix, int64_t pos; int au_byterate, au_ssize, au_scale; - avio_flush(pb); pos = avio_tell(pb); /* Updating one entry in the AVI OpenDML master index */ diff --git a/libavformat/cafenc.c b/libavformat/cafenc.c index 0f7c4ebbb3..98d4d9212f 100644 --- a/libavformat/cafenc.c +++ b/libavformat/cafenc.c @@ -203,7 +203,6 @@ static int caf_write_header(AVFormatContext *s) avio_wb64(pb, -1); //< mChunkSize avio_wb32(pb, 0); //< mEditCount - avio_flush(pb); return 0; } @@ -259,7 +258,6 @@ static int caf_write_trailer(AVFormatContext *s) avio_write(pb, caf->pkt_sizes, caf->size_entries_used); caf->size_buffer_size = 0; } - avio_flush(pb); } av_freep(&caf->pkt_sizes); return 0; diff --git a/libavformat/ffmetaenc.c b/libavformat/ffmetaenc.c index a9adbb1d19..800fb1887c 100644 --- a/libavformat/ffmetaenc.c +++ b/libavformat/ffmetaenc.c @@ -54,7 +54,6 @@ static int write_header(AVFormatContext *s) avio_write(s->pb, ID_STRING, sizeof(ID_STRING) - 1); avio_w8(s->pb, '1'); // version avio_w8(s->pb, '\n'); - avio_flush(s->pb); return 0; } diff --git a/libavformat/flacenc.c b/libavformat/flacenc.c index abbed38f89..1aae0c97e0 100644 --- a/libavformat/flacenc.c +++ b/libavformat/flacenc.c @@ -349,7 +349,6 @@ static int flac_write_trailer(struct AVFormatContext *s) avio_seek(pb, 8, SEEK_SET); avio_write(pb, streaminfo, FLAC_STREAMINFO_SIZE); avio_seek(pb, file_size, SEEK_SET); - avio_flush(pb); } else { av_log(s, AV_LOG_WARNING, "unable to rewrite FLAC header.\n"); } diff --git a/libavformat/framehash.c b/libavformat/framehash.c index 3ae9092c61..8d90793d7c 100644 --- a/libavformat/framehash.c +++ b/libavformat/framehash.c @@ -45,7 +45,6 @@ int ff_framehash_write_header(AVFormatContext *s) avio_printf(s->pb, "#sar %d: %d/%d\n", i, st->sample_aspect_ratio.num, st->sample_aspect_ratio.den); break; } - avio_flush(s->pb); } return 0; } diff --git a/libavformat/gxfenc.c b/libavformat/gxfenc.c index ad9ddea887..e09b8d7625 100644 --- a/libavformat/gxfenc.c +++ b/libavformat/gxfenc.c @@ -834,7 +834,6 @@ static int gxf_write_header(AVFormatContext *s) gxf->packet_count = 3; - avio_flush(pb); return 0; } @@ -864,13 +863,11 @@ static int gxf_write_trailer(AVFormatContext *s) return ret; gxf_write_flt_packet(s); gxf_write_umf_packet(s); - avio_flush(pb); /* update duration in all map packets */ for (i = 1; i < gxf->map_offsets_nb; i++) { avio_seek(pb, gxf->map_offsets[i], SEEK_SET); if ((ret = gxf_write_map_packet(s, 1)) < 0) return ret; - avio_flush(pb); } avio_seek(pb, end, SEEK_SET); diff --git a/libavformat/hashenc.c b/libavformat/hashenc.c index 34a8fd1f50..ce609f6efa 100644 --- a/libavformat/hashenc.c +++ b/libavformat/hashenc.c @@ -152,7 +152,6 @@ static int hash_write_trailer(struct AVFormatContext *s) av_hash_final_hex(c->hashes[i], buf + strlen(buf), sizeof(buf) - strlen(buf)); av_strlcatf(buf, sizeof(buf), "\n"); avio_write(s->pb, buf, strlen(buf)); - avio_flush(s->pb); } return 0; @@ -325,7 +324,6 @@ static int framehash_write_packet(struct AVFormatContext *s, AVPacket *pkt) } avio_printf(s->pb, "\n"); - avio_flush(s->pb); return 0; } diff --git a/libavformat/icoenc.c b/libavformat/icoenc.c index 975c3466bf..a7df8b72bc 100644 --- a/libavformat/icoenc.c +++ b/libavformat/icoenc.c @@ -106,8 +106,6 @@ static int ico_write_header(AVFormatContext *s) if (!ico->images) return AVERROR(ENOMEM); - avio_flush(pb); - return 0; } diff --git a/libavformat/idroqenc.c b/libavformat/idroqenc.c index 8122efef83..261f21939c 100644 --- a/libavformat/idroqenc.c +++ b/libavformat/idroqenc.c @@ -55,7 +55,6 @@ static int roq_write_header(struct AVFormatContext *s) } avio_write(s->pb, header, 8); - avio_flush(s->pb); return 0; } diff --git a/libavformat/ilbc.c b/libavformat/ilbc.c index ba11953b59..01c7112ad1 100644 --- a/libavformat/ilbc.c +++ b/libavformat/ilbc.c @@ -49,7 +49,6 @@ static int ilbc_write_header(AVFormatContext *s) av_log(s, AV_LOG_ERROR, "Unsupported mode\n"); return AVERROR(EINVAL); } - avio_flush(pb); return 0; } diff --git a/libavformat/img2enc.c b/libavformat/img2enc.c index 39398f37a3..5ee99890ce 100644 --- a/libavformat/img2enc.c +++ b/libavformat/img2enc.c @@ -118,7 +118,6 @@ static int write_packet_pipe(AVFormatContext *s, AVPacket *pkt) return ret; } else { avio_write(s->pb, pkt->data, pkt->size); - avio_flush(s->pb); } img->img_number++; return 0; diff --git a/libavformat/jacosubenc.c b/libavformat/jacosubenc.c index 0954f5f058..77575c6b3c 100644 --- a/libavformat/jacosubenc.c +++ b/libavformat/jacosubenc.c @@ -25,7 +25,6 @@ static int jacosub_write_header(AVFormatContext *s) if (par->extradata_size) { avio_write(s->pb, par->extradata, par->extradata_size - 1); - avio_flush(s->pb); } return 0; } diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c index 9dcd1946e8..4b6d14b435 100644 --- a/libavformat/matroskaenc.c +++ b/libavformat/matroskaenc.c @@ -2000,8 +2000,6 @@ static int mkv_write_header(AVFormatContext *s) mkv->cur_audio_pkt.size = 0; mkv->cluster_pos = -1; - avio_flush(pb); - // start a new cluster every 5 MB or 5 sec, or 32k / 1 sec for streaming or // after 4k and on a keyframe if (pb->seekable & AVIO_SEEKABLE_NORMAL) { diff --git a/libavformat/microdvdenc.c b/libavformat/microdvdenc.c index 80e6fa8674..1cd215d8de 100644 --- a/libavformat/microdvdenc.c +++ b/libavformat/microdvdenc.c @@ -37,7 +37,6 @@ static int microdvd_write_header(struct AVFormatContext *s) avio_write(s->pb, "{DEFAULT}{}", 11); avio_write(s->pb, par->extradata, par->extradata_size); avio_w8(s->pb, '\n'); - avio_flush(s->pb); } avpriv_set_pts_info(s->streams[0], 64, framerate.num, framerate.den); diff --git a/libavformat/mmf.c b/libavformat/mmf.c index 917113066a..e4768db064 100644 --- a/libavformat/mmf.c +++ b/libavformat/mmf.c @@ -123,8 +123,6 @@ static int mmf_write_header(AVFormatContext *s) avpriv_set_pts_info(s->streams[0], 64, 1, s->streams[0]->codecpar->sample_rate); - avio_flush(pb); - return 0; } @@ -173,8 +171,6 @@ static int mmf_write_trailer(AVFormatContext *s) avio_write(pb, "\x00\x00\x00\x00", 4); avio_seek(pb, pos, SEEK_SET); - - avio_flush(pb); } return 0; } diff --git a/libavformat/movenc.c b/libavformat/movenc.c index e422bdd071..84d73a36d1 100644 --- a/libavformat/movenc.c +++ b/libavformat/movenc.c @@ -6570,8 +6570,6 @@ static int mov_write_header(AVFormatContext *s) } } - avio_flush(pb); - if (mov->flags & FF_MOV_FLAG_ISML) mov_write_isml_manifest(pb, mov, s); @@ -6579,7 +6577,6 @@ static int mov_write_header(AVFormatContext *s) !(mov->flags & FF_MOV_FLAG_DELAY_MOOV)) { if ((ret = mov_write_moov_tag(pb, mov, s)) < 0) return ret; - avio_flush(pb); mov->moov_written = 1; if (mov->flags & FF_MOV_FLAG_GLOBAL_SIDX) mov->reserved_header_pos = avio_tell(pb); diff --git a/libavformat/mpegenc.c b/libavformat/mpegenc.c index f6980231a2..f8039a42c0 100644 --- a/libavformat/mpegenc.c +++ b/libavformat/mpegenc.c @@ -1244,7 +1244,6 @@ static int mpeg_mux_end(AVFormatContext *ctx) * it as it is usually not needed by decoders and because it * complicates MPEG stream concatenation. */ // avio_wb32(ctx->pb, ISO_11172_END_CODE); - // avio_flush(ctx->pb); for (i = 0; i < ctx->nb_streams; i++) { stream = ctx->streams[i]->priv_data; diff --git a/libavformat/mpjpeg.c b/libavformat/mpjpeg.c index 80f83c5871..e410e84801 100644 --- a/libavformat/mpjpeg.c +++ b/libavformat/mpjpeg.c @@ -34,7 +34,6 @@ static int mpjpeg_write_header(AVFormatContext *s) { MPJPEGContext *mpj = s->priv_data; avio_printf(s->pb, "--%s\r\n", mpj->boundary_tag); - avio_flush(s->pb); return 0; } diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c index e187b3845d..396768d51f 100644 --- a/libavformat/mxfenc.c +++ b/libavformat/mxfenc.c @@ -2799,7 +2799,6 @@ static int mxf_write_opatom_packet(AVFormatContext *s, AVPacket *pkt, MXFIndexEn mxf->edit_units_count++; avio_write(pb, pkt->data, pkt->size); mxf->body_offset += pkt->size; - avio_flush(pb); return 0; } @@ -2937,8 +2936,6 @@ static int mxf_write_packet(AVFormatContext *s, AVPacket *pkt) mxf->body_offset += 16+4+pkt->size + klv_fill_size(16+4+pkt->size); } - avio_flush(pb); - return 0; } diff --git a/libavformat/nutenc.c b/libavformat/nutenc.c index 46dce7722d..44773924dd 100644 --- a/libavformat/nutenc.c +++ b/libavformat/nutenc.c @@ -768,8 +768,6 @@ static int nut_write_header(AVFormatContext *s) if (s->avoid_negative_ts < 0) s->avoid_negative_ts = 1; - avio_flush(bc); - return 0; } diff --git a/libavformat/rmenc.c b/libavformat/rmenc.c index 3bff4daf0a..e137dbc44f 100644 --- a/libavformat/rmenc.c +++ b/libavformat/rmenc.c @@ -360,7 +360,6 @@ static int rm_write_header(AVFormatContext *s) if (rv10_write_header(s, 0, 0)) return AVERROR_INVALIDDATA; - avio_flush(s->pb); return 0; } diff --git a/libavformat/rsoenc.c b/libavformat/rsoenc.c index e34e2c64e2..3159f0c427 100644 --- a/libavformat/rsoenc.c +++ b/libavformat/rsoenc.c @@ -60,8 +60,6 @@ static int rso_write_header(AVFormatContext *s) avio_wb16(pb, par->sample_rate); avio_wb16(pb, 0x0000); /* play mode ? (0x0000 = don't loop) */ - avio_flush(pb); - return 0; } diff --git a/libavformat/segafilmenc.c b/libavformat/segafilmenc.c index f1048ff808..812d0ad64e 100644 --- a/libavformat/segafilmenc.c +++ b/libavformat/segafilmenc.c @@ -362,8 +362,6 @@ static int film_write_header(AVFormatContext *format_context) avio_wb32(pb, film->packet_count); - avio_flush(pb); - /* Finally, write out each packet's data to the header */ packet = film->start; while (packet != NULL) { diff --git a/libavformat/smjpegenc.c b/libavformat/smjpegenc.c index 68a128647e..c3c1a6346c 100644 --- a/libavformat/smjpegenc.c +++ b/libavformat/smjpegenc.c @@ -88,7 +88,6 @@ static int smjpeg_write_header(AVFormatContext *s) } avio_wl32(pb, SMJPEG_HEND); - avio_flush(pb); return 0; } diff --git a/libavformat/soxenc.c b/libavformat/soxenc.c index 7b37bd4848..ce276f88b5 100644 --- a/libavformat/soxenc.c +++ b/libavformat/soxenc.c @@ -80,8 +80,6 @@ static int sox_write_header(AVFormatContext *s) ffio_fill(pb, 0, comment_size - comment_len); - avio_flush(pb); - return 0; } @@ -101,8 +99,6 @@ static int sox_write_trailer(AVFormatContext *s) } else avio_wb64(pb, num_samples); avio_seek(pb, file_size, SEEK_SET); - - avio_flush(pb); } return 0; diff --git a/libavformat/swfenc.c b/libavformat/swfenc.c index f53db0fb2b..84f924eda5 100644 --- a/libavformat/swfenc.c +++ b/libavformat/swfenc.c @@ -337,7 +337,6 @@ static int swf_write_header(AVFormatContext *s) put_swf_end_tag(s); } - avio_flush(s->pb); return 0; } diff --git a/libavformat/ttaenc.c b/libavformat/ttaenc.c index 73c29ae936..4860aab4c1 100644 --- a/libavformat/ttaenc.c +++ b/libavformat/ttaenc.c @@ -154,7 +154,6 @@ static int tta_write_trailer(AVFormatContext *s) tta_queue_flush(s); ff_ape_write_tag(s); - avio_flush(s->pb); return 0; } diff --git a/libavformat/vc1testenc.c b/libavformat/vc1testenc.c index cf95d1d80d..1365bdd660 100644 --- a/libavformat/vc1testenc.c +++ b/libavformat/vc1testenc.c @@ -76,7 +76,6 @@ static int vc1test_write_trailer(AVFormatContext *s) if (s->pb->seekable & AVIO_SEEKABLE_NORMAL) { avio_seek(pb, 0, SEEK_SET); avio_wl24(pb, ctx->frames); - avio_flush(pb); } return 0; } diff --git a/libavformat/wavenc.c b/libavformat/wavenc.c index 2b28149056..f6f5710802 100644 --- a/libavformat/wavenc.c +++ b/libavformat/wavenc.c @@ -361,8 +361,6 @@ static int wav_write_header(AVFormatContext *s) wav->data = ff_start_tag(pb, "data"); } - avio_flush(pb); - return 0; } @@ -413,17 +411,13 @@ static int wav_write_trailer(AVFormatContext *s) int rf64 = 0; int ret = 0; - avio_flush(pb); - if (s->pb->seekable & AVIO_SEEKABLE_NORMAL) { if (wav->write_peak != PEAK_ONLY && avio_tell(pb) - wav->data < UINT32_MAX) { ff_end_tag(pb, wav->data); - avio_flush(pb); } if (wav->write_peak && wav->peak_output) { ret = peak_write_chunk(s); - avio_flush(pb); } /* update file size */ @@ -435,8 +429,6 @@ static int wav_write_trailer(AVFormatContext *s) avio_seek(pb, 4, SEEK_SET); avio_wl32(pb, (uint32_t)(file_size - 8)); avio_seek(pb, file_size, SEEK_SET); - - avio_flush(pb); } else { av_log(s, AV_LOG_ERROR, "Filesize %"PRId64" invalid for wav, output file will be broken\n", @@ -456,7 +448,6 @@ static int wav_write_trailer(AVFormatContext *s) } else { avio_wl32(pb, number_of_samples); avio_seek(pb, file_size, SEEK_SET); - avio_flush(pb); } } @@ -480,7 +471,6 @@ static int wav_write_trailer(AVFormatContext *s) avio_wl32(pb, -1); avio_seek(pb, file_size, SEEK_SET); - avio_flush(pb); } } @@ -607,7 +597,6 @@ static int w64_write_trailer(AVFormatContext *s) } avio_seek(pb, file_size, SEEK_SET); - avio_flush(pb); } return 0; diff --git a/libavformat/webvttenc.c b/libavformat/webvttenc.c index 61b7f54622..cbd989dcb6 100644 --- a/libavformat/webvttenc.c +++ b/libavformat/webvttenc.c @@ -57,7 +57,6 @@ static int webvtt_write_header(AVFormatContext *ctx) avpriv_set_pts_info(s, 64, 1, 1000); avio_printf(pb, "WEBVTT\n"); - avio_flush(pb); return 0; } diff --git a/libavformat/wtvenc.c b/libavformat/wtvenc.c index 4a68b8133f..498bc64019 100644 --- a/libavformat/wtvenc.c +++ b/libavformat/wtvenc.c @@ -823,8 +823,6 @@ static int write_trailer(AVFormatContext *s) avio_seek(pb, 0x5c, SEEK_SET); avio_wl32(pb, file_end_pos >> WTV_SECTOR_BITS); - avio_flush(pb); - av_free(wctx->sp_pairs); av_free(wctx->st_pairs); av_packet_unref(&wctx->thumbnail); diff --git a/tests/ref/fate/movenc b/tests/ref/fate/movenc index 5c6a054c1a..d5ef89124a 100644 --- a/tests/ref/fate/movenc +++ b/tests/ref/fate/movenc @@ -13,28 +13,24 @@ write_data len 2629, time nopts, type header atom - write_data len 908, time 1000000, type sync atom moof write_data len 110, time nopts, type trailer atom - e9f6fa032d6d8265d67aef5de81a48bf 3683 non-empty-moov-no-elst -write_data len 24, time nopts, type header atom ftyp -write_data len 1171, time nopts, type header atom - +write_data len 1195, time nopts, type header atom ftyp write_data len 728, time 0, type sync atom moof write_data len 828, time nopts, type unknown atom - write_data len 728, time 1046439, type sync atom moof write_data len 812, time nopts, type unknown atom - write_data len 148, time nopts, type trailer atom - a7c93f998e88fee1159580b6ca7e3d2b 4439 ismv -write_data len 36, time nopts, type header atom ftyp -write_data len 1123, time nopts, type header atom - +write_data len 1159, time nopts, type header atom ftyp write_data len 796, time 0, type sync atom moof write_data len 788, time 1000000, type sync atom moof write_data len 148, time nopts, type trailer atom - e6a4b15443d006efd727a80f6624b7db 2891 empty-moov -write_data len 36, time nopts, type header atom ftyp -write_data len 1123, time nopts, type header atom - +write_data len 1159, time nopts, type header atom ftyp write_data len 1068, time 0, type sync atom moof write_data len 908, time 1000000, type sync atom moof write_data len 148, time nopts, type trailer atom - 800f854aff2ac76dfaddebd0562c75b9 3283 empty-moov-no-elst -write_data len 36, time nopts, type header atom ftyp -write_data len 1123, time nopts, type header atom - +write_data len 1159, time nopts, type header atom ftyp write_data len 900, time -33333, type sync atom moof write_data len 908, time 966667, type sync atom moof write_data len 148, time nopts, type trailer atom - @@ -58,8 +54,7 @@ write_data len 360, time 0, type sync atom moof write_data len 360, time 1000000, type sync atom moof write_data len 86, time nopts, type trailer atom - 8805d72a27b340ea229c16edde78f974 2001 delay-moov-empty-track-flush -write_data len 36, time nopts, type header atom ftyp -write_data len 1123, time nopts, type header atom - +write_data len 1159, time nopts, type header atom ftyp 351ae2c8b6d35d98b4848c309cce6704 1159 empty-moov-header write_data len 796, time 0, type sync atom moof write_data len 788, time 1000000, type sync atom moof @@ -71,14 +66,12 @@ write_data len 796, time 0, type sync atom moof write_data len 788, time 1000000, type sync atom moof a0165f4a26a409212b0946e981bdefb9 1584 delay-moov-content write_data len 148, time nopts, type trailer atom - -write_data len 28, time nopts, type header atom - -write_data len 1123, time nopts, type header atom - +write_data len 1151, time nopts, type header atom - write_data len 884, time 0, type sync atom sidx write_data len 876, time 1000000, type sync atom sidx 272a474cfd2a68cc5f05b426b14a2b7d 876 empty-moov-second-frag write_data len 148, time nopts, type trailer atom - -write_data len 28, time nopts, type header atom - -write_data len 1123, time nopts, type header atom - +write_data len 1151, time nopts, type header atom - write_data len 876, time 1000000, type sync atom sidx 272a474cfd2a68cc5f05b426b14a2b7d 876 empty-moov-second-frag-discont write_data len 110, time nopts, type trailer atom - @@ -145,8 +138,7 @@ write_data len 916, time 0, type sync atom moof write_data len 908, time 1000000, type sync atom moof write_data len 148, time nopts, type trailer atom - c200a345c365dd35a31e7e62a9ae6c10 3203 delay-moov-elst-neg-cts -write_data len 36, time nopts, type header atom ftyp -write_data len 1123, time nopts, type header atom - +write_data len 1159, time nopts, type header atom ftyp write_data len 1188, time 0, type sync atom moof write_data len 908, time 1033333, type sync atom moof write_data len 148, time nopts, type trailer atom -
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. - 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. 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. The user can still control flushing behaviour using the -flush_packets option, the default typically means to flush unless a non-streamed file output is used. Signed-off-by: Marton Balint <cus@passwd.hu> --- libavformat/aiffenc.c | 5 ----- libavformat/amr.c | 1 - libavformat/asfenc.c | 3 --- libavformat/assenc.c | 1 - libavformat/astenc.c | 3 --- libavformat/au.c | 2 -- libavformat/avienc.c | 3 --- libavformat/cafenc.c | 2 -- libavformat/ffmetaenc.c | 1 - libavformat/flacenc.c | 1 - libavformat/framehash.c | 1 - libavformat/gxfenc.c | 3 --- libavformat/hashenc.c | 2 -- libavformat/icoenc.c | 2 -- libavformat/idroqenc.c | 1 - libavformat/ilbc.c | 1 - libavformat/img2enc.c | 1 - libavformat/jacosubenc.c | 1 - libavformat/matroskaenc.c | 2 -- libavformat/microdvdenc.c | 1 - libavformat/mmf.c | 4 ---- libavformat/movenc.c | 3 --- libavformat/mpegenc.c | 1 - libavformat/mpjpeg.c | 1 - libavformat/mxfenc.c | 3 --- libavformat/nutenc.c | 2 -- libavformat/rmenc.c | 1 - libavformat/rsoenc.c | 2 -- libavformat/segafilmenc.c | 2 -- libavformat/smjpegenc.c | 1 - libavformat/soxenc.c | 4 ---- libavformat/swfenc.c | 1 - libavformat/ttaenc.c | 1 - libavformat/vc1testenc.c | 1 - libavformat/wavenc.c | 11 ----------- libavformat/webvttenc.c | 1 - libavformat/wtvenc.c | 2 -- tests/ref/fate/movenc | 24 ++++++++---------------- 38 files changed, 8 insertions(+), 94 deletions(-)