| Message ID | 20200331123745.6461-8-andreas.rheinhardt@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | libavformat/mux patches | expand |
| Context | Check | Description |
|---|---|---|
| andriy/ffmpeg-patchwork | success | Make fate finished |
On Tue, 31 Mar 2020, Andreas Rheinhardt wrote: > write_packet() currently saves the original timestamps of the packet it > got and restores them in case writing fails. This is unnecessary as none > of write_packet()'s callers make any use of these timestamps at all. So > remove this and add a general comment to the function that timestamps > may be modified; also remove a long outdated comment about side data. > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> > --- > Actually, doing the timestamp backups in write_packet() never made much > sense: Timestamps may also be modified before the packet reaches > write_packet(). Furthermore, backups for av_interleaved_write_frame() > don't make sense at all, so these backups should have been performed in > av_write_frame(). > > libavformat/mux.c | 12 ++---------- > 1 file changed, 2 insertions(+), 10 deletions(-) > > diff --git a/libavformat/mux.c b/libavformat/mux.c > index be7652c97b..acb71b30c9 100644 > --- a/libavformat/mux.c > +++ b/libavformat/mux.c > @@ -670,8 +670,7 @@ FF_ENABLE_DEPRECATION_WARNINGS > #endif > > /** > - * Make timestamps non negative, move side data from payload to internal struct, call muxer, and restore > - * sidedata. > + * Make timestamps non negative and call muxer; the original pts/dts are not kept. > * > * FIXME: this function should NEVER get undefined pts/dts beside when the > * AVFMT_NOTIMESTAMPS is set. > @@ -681,10 +680,6 @@ FF_ENABLE_DEPRECATION_WARNINGS > static int write_packet(AVFormatContext *s, AVPacket *pkt) > { > int ret; > - int64_t pts_backup, dts_backup; > - > - pts_backup = pkt->pts; > - dts_backup = pkt->dts; > > // If the timestamp offsetting below is adjusted, adjust > // ff_interleaved_peek similarly. > @@ -760,10 +755,7 @@ static int write_packet(AVFormatContext *s, AVPacket *pkt) > ret = s->pb->error; > } > > - if (ret < 0) { > - pkt->pts = pts_backup; > - pkt->dts = dts_backup; > - } else > + if (ret >= 0) > s->streams[pkt->stream_index]->nb_frames++; > > return ret; LGTM, thanks. Marton
diff --git a/libavformat/mux.c b/libavformat/mux.c index be7652c97b..acb71b30c9 100644 --- a/libavformat/mux.c +++ b/libavformat/mux.c @@ -670,8 +670,7 @@ FF_ENABLE_DEPRECATION_WARNINGS #endif /** - * Make timestamps non negative, move side data from payload to internal struct, call muxer, and restore - * sidedata. + * Make timestamps non negative and call muxer; the original pts/dts are not kept. * * FIXME: this function should NEVER get undefined pts/dts beside when the * AVFMT_NOTIMESTAMPS is set. @@ -681,10 +680,6 @@ FF_ENABLE_DEPRECATION_WARNINGS static int write_packet(AVFormatContext *s, AVPacket *pkt) { int ret; - int64_t pts_backup, dts_backup; - - pts_backup = pkt->pts; - dts_backup = pkt->dts; // If the timestamp offsetting below is adjusted, adjust // ff_interleaved_peek similarly. @@ -760,10 +755,7 @@ static int write_packet(AVFormatContext *s, AVPacket *pkt) ret = s->pb->error; } - if (ret < 0) { - pkt->pts = pts_backup; - pkt->dts = dts_backup; - } else + if (ret >= 0) s->streams[pkt->stream_index]->nb_frames++; return ret;
write_packet() currently saves the original timestamps of the packet it got and restores them in case writing fails. This is unnecessary as none of write_packet()'s callers make any use of these timestamps at all. So remove this and add a general comment to the function that timestamps may be modified; also remove a long outdated comment about side data. Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> --- Actually, doing the timestamp backups in write_packet() never made much sense: Timestamps may also be modified before the packet reaches write_packet(). Furthermore, backups for av_interleaved_write_frame() don't make sense at all, so these backups should have been performed in av_write_frame(). libavformat/mux.c | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-)