diff mbox series

[FFmpeg-devel,04/18] fftools/ffmpeg_mux: stop rescaling timestamps in of_streamcopy()

Message ID 20230826151144.24858-4-anton@khirnov.net
State New
Headers show
Series [FFmpeg-devel,01/18] fftools/ffmpeg: stop explicitly closing output streams on input EOF | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Anton Khirnov Aug. 26, 2023, 3:11 p.m. UTC 
This function converts packet timestamps from the input stream timebase
to OutputStream.mux_timebase, which may or may not be equal to the
actual output AVStream timebase (and even when it is, this may not
always be the optimal choice due to bitstream filtering).

Just keep the timestamps in input stream timebase, they will be rescaled
as needed before bitstream filtering and/or sending the packet to the
muxer.

Drop now-unused OutputStream.mux_timebase.
---
 fftools/ffmpeg.h          |  2 --
 fftools/ffmpeg_enc.c      |  2 --
 fftools/ffmpeg_mux.c      | 21 +++++++--------------
 fftools/ffmpeg_mux_init.c |  2 --
 4 files changed, 7 insertions(+), 20 deletions(-)

Comments

Michael Niedermayer Aug. 26, 2023, 8:20 p.m. UTC | #1 
On Sat, Aug 26, 2023 at 05:11:30PM +0200, Anton Khirnov wrote:
> This function converts packet timestamps from the input stream timebase
> to OutputStream.mux_timebase, which may or may not be equal to the
> actual output AVStream timebase (and even when it is, this may not
> always be the optimal choice due to bitstream filtering).
> 
> Just keep the timestamps in input stream timebase, they will be rescaled
> as needed before bitstream filtering and/or sending the packet to the
> muxer.
> 
> Drop now-unused OutputStream.mux_timebase.
> ---
>  fftools/ffmpeg.h          |  2 --
>  fftools/ffmpeg_enc.c      |  2 --
>  fftools/ffmpeg_mux.c      | 21 +++++++--------------
>  fftools/ffmpeg_mux_init.c |  2 --
>  4 files changed, 7 insertions(+), 20 deletions(-)

./ffmpeg -i in.flv  -t 0.2 -bitexact -acodec copy -y /tmp/copy.mov  (this ffmpeg version matters)

./ffmpeg -i /tmp/copy.mov -bitexact -f framecrc -                   (this ffmpeg version doesnt matter)

0,          0,          0,     1152,     4608, 0xa25d01fe
0,       1152,       1152,     1152,     4608, 0x3b9ff171
0,       2304,       2304,     1152,     4608, 0x10ad7d49
0,       3456,       3456,     1152,     4608, 0xadae4ea6
0,       4608,       4608,     1152,     4608, 0xaba106e7
0,       5760,       5760,     1152,     4608, 0x1beee538
0,       6912,       6912,     1152,     4608, 0x7dbe78ef
0,       8064,       8064,     1152,     4608, 0xaf41b247

after the commit:
0,          0,          0,     1152,     4608, 0xa25d01fe
0,       1147,       1147,     1152,     4608, 0x3b9ff171
0,       2293,       2293,     1152,     4608, 0x10ad7d49
0,       3440,       3440,     1152,     4608, 0xadae4ea6
0,       4586,       4586,     1152,     4608, 0xaba106e7
0,       5777,       5777,     1152,     4608, 0x1beee538
0,       6924,       6924,     1152,     4608, 0x7dbe78ef
0,       8070,       8070,     1152,     4608, 0xaf41b247

Here the timestamps stored in a newly muxed file seems bad
could be missing something but that seems bad, even if the
input is only 1ms precisse we need to generate valid timestamps
if we remux

I suspect every mp3 in flv will show this effect but if not i can share in.flv

thx

[...]
diff mbox series

Patch

diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index d53181e427..ef5bb13908 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -523,8 +523,6 @@  typedef struct OutputStream {
     /* dts of the last packet sent to the muxing queue, in AV_TIME_BASE_Q */
     int64_t last_mux_dts;
 
-    // the timebase of the packets sent to the muxer
-    AVRational mux_timebase;
     AVRational enc_timebase;
 
     Encoder *enc;
diff --git a/fftools/ffmpeg_enc.c b/fftools/ffmpeg_enc.c
index 96424272bf..3c9fdd3237 100644
--- a/fftools/ffmpeg_enc.c
+++ b/fftools/ffmpeg_enc.c
@@ -463,8 +463,6 @@  int enc_open(OutputStream *ost, AVFrame *frame)
     if (ost->st->time_base.num <= 0 || ost->st->time_base.den <= 0)
         ost->st->time_base = av_add_q(ost->enc_ctx->time_base, (AVRational){0, 1});
 
-    ost->mux_timebase = enc_ctx->time_base;
-
     ret = of_stream_init(of, ost);
     if (ret < 0)
         return ret;
diff --git a/fftools/ffmpeg_mux.c b/fftools/ffmpeg_mux.c
index ab9bb398c1..076c6afabd 100644
--- a/fftools/ffmpeg_mux.c
+++ b/fftools/ffmpeg_mux.c
@@ -392,7 +392,7 @@  int of_streamcopy(OutputStream *ost, const AVPacket *pkt, int64_t dts)
     OutputFile *of = output_files[ost->file_index];
     MuxStream  *ms = ms_from_ost(ost);
     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);
+    int64_t ts_offset;
     AVPacket *opkt = ms->pkt;
     int ret;
 
@@ -425,10 +425,10 @@  int of_streamcopy(OutputStream *ost, const AVPacket *pkt, int64_t dts)
     if (ret < 0)
         return ret;
 
-    opkt->time_base = ost->mux_timebase;
+    ts_offset = av_rescale_q(start_time, AV_TIME_BASE_Q, opkt->time_base);
 
     if (pkt->pts != AV_NOPTS_VALUE)
-        opkt->pts = av_rescale_q(pkt->pts, pkt->time_base, opkt->time_base) - ost_tb_start_time;
+        opkt->pts -= ts_offset;
 
     if (pkt->dts == AV_NOPTS_VALUE) {
         opkt->dts = av_rescale_q(dts, AV_TIME_BASE_Q, opkt->time_base);
@@ -436,16 +436,13 @@  int of_streamcopy(OutputStream *ost, const AVPacket *pkt, int64_t dts)
         int duration = av_get_audio_frame_duration2(ost->par_in, pkt->size);
         if(!duration)
             duration = ost->par_in->frame_size;
-        opkt->dts = av_rescale_delta(pkt->time_base, pkt->dts,
+        opkt->dts = av_rescale_delta(opkt->time_base, pkt->dts,
                                     (AVRational){1, ost->par_in->sample_rate}, duration,
                                     &ms->ts_rescale_delta_last, opkt->time_base);
         /* 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, pkt->time_base, opkt->time_base);
-    opkt->dts -= ost_tb_start_time;
-
-    opkt->duration = av_rescale_q(pkt->duration, pkt->time_base, opkt->time_base);
+        opkt->pts = opkt->dts - ts_offset;
+    }
+    opkt->dts -= ts_offset;
 
     {
         int ret = trigger_fix_sub_duration_heartbeat(ost, pkt);
@@ -511,10 +508,6 @@  static int thread_start(Muxer *mux)
         MuxStream     *ms = ms_from_ost(ost);
         AVPacket *pkt;
 
-        /* try to improve muxing time_base (only possible if nothing has been written yet) */
-        if (!av_fifo_can_read(ms->muxing_queue))
-            ost->mux_timebase = ost->st->time_base;
-
         while (av_fifo_read(ms->muxing_queue, &pkt, 1) >= 0) {
             ret = thread_submit_packet(mux, ost, pkt);
             if (pkt) {
diff --git a/fftools/ffmpeg_mux_init.c b/fftools/ffmpeg_mux_init.c
index 0289cdabad..cf4cd2d5b7 100644
--- a/fftools/ffmpeg_mux_init.c
+++ b/fftools/ffmpeg_mux_init.c
@@ -1090,8 +1090,6 @@  static int streamcopy_init(const Muxer *mux, OutputStream *ost)
         }
     }
 
-    ost->mux_timebase = ist->st->time_base;
-
 fail:
     avcodec_free_context(&codec_ctx);
     av_dict_free(&codec_opts);