| Message ID | 20191011040658.31714-1-andreas.rheinhardt@gmail.com |
|---|---|
| State | Accepted |
| Commit | 223a2c2a6079a8a5a8bbaf4a7671f959a8dd8bc2 |
| Headers | show |
On Fri, Oct 11, 2019 at 06:06:58AM +0200, Andreas Rheinhardt wrote: > do_streamcopy() has a packet that gets zero-initialized first, then gets > initialized via av_init_packet() after which some of its fields are > oerwritten again with the actually desired values (unless it's EOF): The > side data is copied into the packet with av_copy_packet_side_data() and > if the source packet is refcounted, the packet will get a new reference > to the source packet's data. Furthermore, the flags are copied and the > timestamp related fields are overwritten with new values. > > This commit replaces this by using av_packet_ref() to both initialize > the packet as well as populate its fields with the right values (unless > it's EOF again in which case the packet will still be initialized). The > differences to the current approach are as follows: > a) There is no call to a deprecated function (av_copy_packet_side_data()) > any more. > b) Several fields that weren't copied before are now copied from the source > packet to the new packet (e.g. pos). Some of them (the timestamp related > fields) may be immediately overwritten again and some don't seem to be > used at all (e.g. pos), but in return using av_packet_ref() allows to forgo > the initializations. > c) There was no check for whether copying side data fails or not. This > has been changed: Now the program is exited in this case. > > Using av_packet_ref() does not lead to unnecessary copying of data, > because the source packets are already always refcounted (they originate > from av_read_frame()). > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> > --- > After this patch there will be only call to av_copy_packet_side_data() > left: In libavformat/movenc.c. > Btw: Initializing the packet for flushing is actually unnecessary. > > fftools/ffmpeg.c | 24 +++++++----------------- > 1 file changed, 7 insertions(+), 17 deletions(-) will apply thx [...]
On Sat, Oct 12, 2019 at 5:18 AM Andreas Rheinhardt < andreas.rheinhardt@gmail.com> wrote: > For audio packets with dts != AV_NOPTS_VALUEs the dts was converted > twice to the muxer's timebase during streamcopy, once as a normal > packet and once specifically as an audio packet. This has been changed. > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> > --- > fftools/ffmpeg.c | 20 ++++++++++---------- > 1 file changed, 10 insertions(+), 10 deletions(-) > > diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c > index 8e408c808a..314d590734 100644 > --- a/fftools/ffmpeg.c > +++ b/fftools/ffmpeg.c > @@ -2046,20 +2046,20 @@ static void do_streamcopy(InputStream *ist, > OutputStream *ost, const AVPacket *p > if (pkt->pts != AV_NOPTS_VALUE) > opkt.pts = av_rescale_q(pkt->pts, ist->st->time_base, > ost->mux_timebase) - ost_tb_start_time; > > - if (pkt->dts == AV_NOPTS_VALUE) > + if (pkt->dts == AV_NOPTS_VALUE) { > opkt.dts = av_rescale_q(ist->dts, AV_TIME_BASE_Q, > ost->mux_timebase); > - else > - opkt.dts = av_rescale_q(pkt->dts, ist->st->time_base, > ost->mux_timebase); > - opkt.dts -= ost_tb_start_time; > - > - if (ost->st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO && pkt->dts > != AV_NOPTS_VALUE) { > + } else if (ost->st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO) { > int duration = av_get_audio_frame_duration(ist->dec_ctx, > pkt->size); > if(!duration) > duration = ist->dec_ctx->frame_size; > - opkt.dts = opkt.pts = av_rescale_delta(ist->st->time_base, > pkt->dts, > - (AVRational){1, > ist->dec_ctx->sample_rate}, duration, &ist->filter_in_rescale_delta_last, > - ost->mux_timebase) - > ost_tb_start_time; > - } > + opkt.dts = av_rescale_delta(ist->st->time_base, pkt->dts, > + (AVRational){1, > ist->dec_ctx->sample_rate}, duration, > + &ist->filter_in_rescale_delta_last, > ost->mux_timebase); > + /* dts will be set immediately afterwards to what pts is now */ > + opkt.pts = opkt.dts - ost_tb_start_time; > + } else > + opkt.dts = av_rescale_q(pkt->dts, ist->st->time_base, > ost->mux_timebase); > + opkt.dts -= ost_tb_start_time; > > opkt.duration = av_rescale_q(pkt->duration, ist->st->time_base, > ost->mux_timebase); > > -- > 2.21.0 > > Ping. - Andreas
On Sat, Oct 12, 2019 at 05:18:39AM +0200, Andreas Rheinhardt wrote: > For audio packets with dts != AV_NOPTS_VALUEs the dts was converted > twice to the muxer's timebase during streamcopy, once as a normal > packet and once specifically as an audio packet. This has been changed. > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> > --- > fftools/ffmpeg.c | 20 ++++++++++---------- > 1 file changed, 10 insertions(+), 10 deletions(-) will apply thx [...]
diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c index b6ecb89893..8e408c808a 100644 --- a/fftools/ffmpeg.c +++ b/fftools/ffmpeg.c @@ -1995,12 +1995,13 @@ static void do_streamcopy(InputStream *ist, OutputStream *ost, const AVPacket *p InputFile *f = input_files [ist->file_index]; int64_t start_time = (of->start_time == AV_NOPTS_VALUE) ? 0 : of->start_time; int64_t ost_tb_start_time = av_rescale_q(start_time, AV_TIME_BASE_Q, ost->mux_timebase); - AVPacket opkt = { 0 }; - - av_init_packet(&opkt); + AVPacket opkt; // EOF: flush output bitstream filters. if (!pkt) { + av_init_packet(&opkt); + opkt.data = NULL; + opkt.size = 0; output_packet(of, &opkt, ost, 1); return; } @@ -2039,10 +2040,11 @@ static void do_streamcopy(InputStream *ist, OutputStream *ost, const AVPacket *p if (ost->enc_ctx->codec_type == AVMEDIA_TYPE_VIDEO) ost->sync_opts++; + if (av_packet_ref(&opkt, pkt) < 0) + exit_program(1); + if (pkt->pts != AV_NOPTS_VALUE) opkt.pts = av_rescale_q(pkt->pts, ist->st->time_base, ost->mux_timebase) - ost_tb_start_time; - else - opkt.pts = AV_NOPTS_VALUE; if (pkt->dts == AV_NOPTS_VALUE) opkt.dts = av_rescale_q(ist->dts, AV_TIME_BASE_Q, ost->mux_timebase); @@ -2061,18 +2063,6 @@ static void do_streamcopy(InputStream *ist, OutputStream *ost, const AVPacket *p opkt.duration = av_rescale_q(pkt->duration, ist->st->time_base, ost->mux_timebase); - opkt.flags = pkt->flags; - - if (pkt->buf) { - opkt.buf = av_buffer_ref(pkt->buf); - if (!opkt.buf) - exit_program(1); - } - opkt.data = pkt->data; - opkt.size = pkt->size; - - av_copy_packet_side_data(&opkt, pkt); - output_packet(of, &opkt, ost, 0); }
do_streamcopy() has a packet that gets zero-initialized first, then gets initialized via av_init_packet() after which some of its fields are oerwritten again with the actually desired values (unless it's EOF): The side data is copied into the packet with av_copy_packet_side_data() and if the source packet is refcounted, the packet will get a new reference to the source packet's data. Furthermore, the flags are copied and the timestamp related fields are overwritten with new values. This commit replaces this by using av_packet_ref() to both initialize the packet as well as populate its fields with the right values (unless it's EOF again in which case the packet will still be initialized). The differences to the current approach are as follows: a) There is no call to a deprecated function (av_copy_packet_side_data()) any more. b) Several fields that weren't copied before are now copied from the source packet to the new packet (e.g. pos). Some of them (the timestamp related fields) may be immediately overwritten again and some don't seem to be used at all (e.g. pos), but in return using av_packet_ref() allows to forgo the initializations. c) There was no check for whether copying side data fails or not. This has been changed: Now the program is exited in this case. Using av_packet_ref() does not lead to unnecessary copying of data, because the source packets are already always refcounted (they originate from av_read_frame()). Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> --- After this patch there will be only call to av_copy_packet_side_data() left: In libavformat/movenc.c. Btw: Initializing the packet for flushing is actually unnecessary. fftools/ffmpeg.c | 24 +++++++----------------- 1 file changed, 7 insertions(+), 17 deletions(-)