| Message ID | 20220815231442.1976-1-jamrial@gmail.com |
|---|---|
| State | Accepted |
| Commit | 836b8001c924deb9263b0f5b7c74ccfaab1f4fdc |
| Headers | show |
| Series | [FFmpeg-devel] avformat/mov: don't read duration from mvhd atom | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/commit_msg_loongarch64 | warning | Please wrap lines in the body of the commit message between 60 and 72 characters. |
| andriy/commit_msg_x86 | warning | Please wrap lines in the body of the commit message between 60 and 72 characters. |
| 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 |
> On Aug 16, 2022, at 7:14 AM, James Almer <jamrial@gmail.com> wrote: > > This duration is equal to the longest duration in all track's tkhd atoms, which > may be comprised of the sum of all edit lists in each track. Empty edit lists > in tracks represent start_time, and the actual media duration is stored in the > mdhd atom. > This change lets the generic demux code derive the longest track duration taken > from mdhd atoms, so the correct duration and start_time combination will be > reported. > > Should fix ticket #9775. The patch LGTM. However, does the ticket indicts some issue related to the mov muxer, which may be hidden by the patch? > > Signed-off-by: James Almer <jamrial@gmail.com> > --- > libavformat/mov.c | 4 ---- > tests/ref/fate/gaplessenc-itunes-to-ipod-aac | 2 +- > 2 files changed, 1 insertion(+), 5 deletions(-) > > diff --git a/libavformat/mov.c b/libavformat/mov.c > index 6ee6ed0950..fee9c39f39 100644 > --- a/libavformat/mov.c > +++ b/libavformat/mov.c > @@ -1516,10 +1516,6 @@ static int mov_read_mvhd(MOVContext *c, AVIOContext *pb, MOVAtom atom) > av_log(c->fc, AV_LOG_TRACE, "time scale = %i\n", c->time_scale); > > c->duration = (version == 1) ? avio_rb64(pb) : avio_rb32(pb); /* duration */ > - // set the AVFormatContext duration because the duration of individual tracks > - // may be inaccurate > - if (!c->trex_data) > - c->fc->duration = av_rescale(c->duration, AV_TIME_BASE, c->time_scale); > avio_rb32(pb); /* preferred scale */ > > avio_rb16(pb); /* preferred volume */ > diff --git a/tests/ref/fate/gaplessenc-itunes-to-ipod-aac b/tests/ref/fate/gaplessenc-itunes-to-ipod-aac > index f967ac05bc..1f89e9af85 100644 > --- a/tests/ref/fate/gaplessenc-itunes-to-ipod-aac > +++ b/tests/ref/fate/gaplessenc-itunes-to-ipod-aac > @@ -5,7 +5,7 @@ duration_ts=103326 > [/STREAM] > [FORMAT] > start_time=0.000000 > -duration=2.344000 > +duration=2.342993 > [/FORMAT] > packet|pts=-1024|dts=-1024|duration=1024|flags=KD|side_data| > > -- > 2.37.1 > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
On Mon, Aug 15, 2022 at 08:14:42PM -0300, James Almer wrote: > This duration is equal to the longest duration in all track's tkhd atoms, which > may be comprised of the sum of all edit lists in each track. Empty edit lists > in tracks represent start_time, and the actual media duration is stored in the > mdhd atom. > This change lets the generic demux code derive the longest track duration taken > from mdhd atoms, so the correct duration and start_time combination will be > reported. > > Should fix ticket #9775. > > Signed-off-by: James Almer <jamrial@gmail.com> > --- > libavformat/mov.c | 4 ---- > tests/ref/fate/gaplessenc-itunes-to-ipod-aac | 2 +- > 2 files changed, 1 insertion(+), 5 deletions(-) No idea if its a bad or good change but this changes the output for: ./ffmpeg -f concat -i ~/tickets/3108/concatfile.txt -codec copy -bitexact /tmp/3108.avi thx [...]
On 8/16/2022 4:22 PM, Michael Niedermayer wrote: > On Mon, Aug 15, 2022 at 08:14:42PM -0300, James Almer wrote: >> This duration is equal to the longest duration in all track's tkhd atoms, which >> may be comprised of the sum of all edit lists in each track. Empty edit lists >> in tracks represent start_time, and the actual media duration is stored in the >> mdhd atom. >> This change lets the generic demux code derive the longest track duration taken >> from mdhd atoms, so the correct duration and start_time combination will be >> reported. >> >> Should fix ticket #9775. >> >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> libavformat/mov.c | 4 ---- >> tests/ref/fate/gaplessenc-itunes-to-ipod-aac | 2 +- >> 2 files changed, 1 insertion(+), 5 deletions(-) > > No idea if its a bad or good change but this changes the output for: > ./ffmpeg -f concat -i ~/tickets/3108/concatfile.txt -codec copy -bitexact /tmp/3108.avi > > thx I don't see an avi file in that ticket, but for the mov in the zip file, the video track duration is used after this patch.
On Sat, Aug 27, 2022 at 04:41:39PM -0300, James Almer wrote: > On 8/16/2022 4:22 PM, Michael Niedermayer wrote: > > On Mon, Aug 15, 2022 at 08:14:42PM -0300, James Almer wrote: > > > This duration is equal to the longest duration in all track's tkhd atoms, which > > > may be comprised of the sum of all edit lists in each track. Empty edit lists > > > in tracks represent start_time, and the actual media duration is stored in the > > > mdhd atom. > > > This change lets the generic demux code derive the longest track duration taken > > > from mdhd atoms, so the correct duration and start_time combination will be > > > reported. > > > > > > Should fix ticket #9775. > > > > > > Signed-off-by: James Almer <jamrial@gmail.com> > > > --- > > > libavformat/mov.c | 4 ---- > > > tests/ref/fate/gaplessenc-itunes-to-ipod-aac | 2 +- > > > 2 files changed, 1 insertion(+), 5 deletions(-) > > > > No idea if its a bad or good change but this changes the output for: > > ./ffmpeg -f concat -i ~/tickets/3108/concatfile.txt -codec copy -bitexact /tmp/3108.avi > > > > thx > > I don't see an avi file in that ticket, but for the mov in the zip file, the > video track duration is used after this patch. the avi is generated in that command from the concatfile.txt from the tickets zip thx [...]
diff --git a/libavformat/mov.c b/libavformat/mov.c index 6ee6ed0950..fee9c39f39 100644 --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -1516,10 +1516,6 @@ static int mov_read_mvhd(MOVContext *c, AVIOContext *pb, MOVAtom atom) av_log(c->fc, AV_LOG_TRACE, "time scale = %i\n", c->time_scale); c->duration = (version == 1) ? avio_rb64(pb) : avio_rb32(pb); /* duration */ - // set the AVFormatContext duration because the duration of individual tracks - // may be inaccurate - if (!c->trex_data) - c->fc->duration = av_rescale(c->duration, AV_TIME_BASE, c->time_scale); avio_rb32(pb); /* preferred scale */ avio_rb16(pb); /* preferred volume */ diff --git a/tests/ref/fate/gaplessenc-itunes-to-ipod-aac b/tests/ref/fate/gaplessenc-itunes-to-ipod-aac index f967ac05bc..1f89e9af85 100644 --- a/tests/ref/fate/gaplessenc-itunes-to-ipod-aac +++ b/tests/ref/fate/gaplessenc-itunes-to-ipod-aac @@ -5,7 +5,7 @@ duration_ts=103326 [/STREAM] [FORMAT] start_time=0.000000 -duration=2.344000 +duration=2.342993 [/FORMAT] packet|pts=-1024|dts=-1024|duration=1024|flags=KD|side_data|
This duration is equal to the longest duration in all track's tkhd atoms, which may be comprised of the sum of all edit lists in each track. Empty edit lists in tracks represent start_time, and the actual media duration is stored in the mdhd atom. This change lets the generic demux code derive the longest track duration taken from mdhd atoms, so the correct duration and start_time combination will be reported. Should fix ticket #9775. Signed-off-by: James Almer <jamrial@gmail.com> --- libavformat/mov.c | 4 ---- tests/ref/fate/gaplessenc-itunes-to-ipod-aac | 2 +- 2 files changed, 1 insertion(+), 5 deletions(-)