diff mbox series

[FFmpeg-devel] avformat/mov: don't read duration from mvhd atom

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

Checks

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

Commit Message

James Almer Aug. 15, 2022, 11:14 p.m. UTC
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(-)

Comments

zhilizhao(赵志立) Aug. 16, 2022, 7:18 a.m. UTC | #1
> 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".
Michael Niedermayer Aug. 16, 2022, 7:22 p.m. UTC | #2
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

[...]
James Almer Aug. 27, 2022, 7:41 p.m. UTC | #3
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.
Michael Niedermayer Aug. 27, 2022, 10:33 p.m. UTC | #4
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 mbox series

Patch

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|