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

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

[James Almer]

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(-)

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".

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

["zhilizhao(赵志立)"]

> 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".

_______________________________________________
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".

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

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 1161 bytes --]

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

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

No human being will ever know the Truth, for even if they happen to say it
by chance, they would not even known they had done so. -- Xenophanes

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

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

[James Almer]

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.
_______________________________________________
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".

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

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 1545 bytes --]

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
[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I do not agree with what you have to say, but I'll defend to the death your
right to say it. -- Voltaire

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".