diff mbox series

[FFmpeg-devel] avformat/mov: remove hack breaking creation time parsing

Message ID 20230408183724.12479-1-cus@passwd.hu
State New
Headers show
Series [FFmpeg-devel] avformat/mov: remove hack breaking creation time parsing | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Marton Balint April 8, 2023, 6:37 p.m. UTC
Commit 23eeffcd48a15e73fb2649b712870b6d101c5471 added a hack to support invalid
files where the creation date was encoded as a classic unix timestamp. This
broke however valid files having creation dates before the unix epoch.

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 libavformat/mov.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Michael Niedermayer April 8, 2023, 9:14 p.m. UTC | #1
On Sat, Apr 08, 2023 at 08:37:24PM +0200, Marton Balint wrote:
> Commit 23eeffcd48a15e73fb2649b712870b6d101c5471 added a hack to support invalid
> files where the creation date was encoded as a classic unix timestamp. This
> broke however valid files having creation dates before the unix epoch.
> 
> Signed-off-by: Marton Balint <cus@passwd.hu>
> ---
>  libavformat/mov.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

This results in:
@@ -1,11 +1,11 @@
-    creation_time   : 2012-06-20T20:58:31.000000Z
-      creation_time   : 2012-06-20T20:58:31.000000Z
-      creation_time   : 2012-06-20T20:58:31.000000Z
+    creation_time   : 1946-06-20T20:58:31.000000Z
+      creation_time   : 1946-06-20T20:58:31.000000Z
+      creation_time   : 1946-06-20T20:58:31.000000Z

Are you sure that 1946 is the correct creation date and not 2012 ?

Thx

[...]
Marton Balint April 9, 2023, 1:49 p.m. UTC | #2
On Sat, 8 Apr 2023, Michael Niedermayer wrote:

> On Sat, Apr 08, 2023 at 08:37:24PM +0200, Marton Balint wrote:
>> Commit 23eeffcd48a15e73fb2649b712870b6d101c5471 added a hack to support invalid
>> files where the creation date was encoded as a classic unix timestamp. This
>> broke however valid files having creation dates before the unix epoch.
>>
>> Signed-off-by: Marton Balint <cus@passwd.hu>
>> ---
>>  libavformat/mov.c | 3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> This results in:
> @@ -1,11 +1,11 @@
> -    creation_time   : 2012-06-20T20:58:31.000000Z
> -      creation_time   : 2012-06-20T20:58:31.000000Z
> -      creation_time   : 2012-06-20T20:58:31.000000Z
> +    creation_time   : 1946-06-20T20:58:31.000000Z
> +      creation_time   : 1946-06-20T20:58:31.000000Z
> +      creation_time   : 1946-06-20T20:58:31.000000Z
>
> Are you sure that 1946 is the correct creation date and not 2012 ?

If you are referring to the file in ticket #1471, yes, 1946 is consistent 
with what mediainfo shows for creation time. Obviously 1946 was not the 
intended creation time, but that does not warrant us to break 
files where 1946 is the *intended* creation time. Proper way to fix the 
original issue would be to detect the device and software version which 
produces the invalid files, and only apply the hack there. But I don't 
think that is doable here, the file does not seem to contain any device or 
software information.

Regards,
Marton
Anton Khirnov April 9, 2023, 2:06 p.m. UTC | #3
Quoting Marton Balint (2023-04-09 15:49:33)
> 
> 
> On Sat, 8 Apr 2023, Michael Niedermayer wrote:
> 
> > On Sat, Apr 08, 2023 at 08:37:24PM +0200, Marton Balint wrote:
> >> Commit 23eeffcd48a15e73fb2649b712870b6d101c5471 added a hack to support invalid
> >> files where the creation date was encoded as a classic unix timestamp. This
> >> broke however valid files having creation dates before the unix epoch.
> >>
> >> Signed-off-by: Marton Balint <cus@passwd.hu>
> >> ---
> >>  libavformat/mov.c | 3 +--
> >>  1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > This results in:
> > @@ -1,11 +1,11 @@
> > -    creation_time   : 2012-06-20T20:58:31.000000Z
> > -      creation_time   : 2012-06-20T20:58:31.000000Z
> > -      creation_time   : 2012-06-20T20:58:31.000000Z
> > +    creation_time   : 1946-06-20T20:58:31.000000Z
> > +      creation_time   : 1946-06-20T20:58:31.000000Z
> > +      creation_time   : 1946-06-20T20:58:31.000000Z
> >
> > Are you sure that 1946 is the correct creation date and not 2012 ?
> 
> If you are referring to the file in ticket #1471, yes, 1946 is consistent 
> with what mediainfo shows for creation time. Obviously 1946 was not the 
> intended creation time, but that does not warrant us to break 
> files where 1946 is the *intended* creation time. Proper way to fix the 
> original issue would be to detect the device and software version which 
> produces the invalid files, and only apply the hack there. But I don't 
> think that is doable here, the file does not seem to contain any device or 
> software information.

How about adding an option for it, like -unix_time?
Michael Niedermayer April 9, 2023, 5:20 p.m. UTC | #4
On Sun, Apr 09, 2023 at 03:49:33PM +0200, Marton Balint wrote:
> 
> 
> On Sat, 8 Apr 2023, Michael Niedermayer wrote:
> 
> > On Sat, Apr 08, 2023 at 08:37:24PM +0200, Marton Balint wrote:
> > > Commit 23eeffcd48a15e73fb2649b712870b6d101c5471 added a hack to support invalid
> > > files where the creation date was encoded as a classic unix timestamp. This
> > > broke however valid files having creation dates before the unix epoch.
> > > 
> > > Signed-off-by: Marton Balint <cus@passwd.hu>
> > > ---
> > >  libavformat/mov.c | 3 +--
> > >  1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > This results in:
> > @@ -1,11 +1,11 @@
> > -    creation_time   : 2012-06-20T20:58:31.000000Z
> > -      creation_time   : 2012-06-20T20:58:31.000000Z
> > -      creation_time   : 2012-06-20T20:58:31.000000Z
> > +    creation_time   : 1946-06-20T20:58:31.000000Z
> > +      creation_time   : 1946-06-20T20:58:31.000000Z
> > +      creation_time   : 1946-06-20T20:58:31.000000Z
> > 
> > Are you sure that 1946 is the correct creation date and not 2012 ?
> 
> If you are referring to the file in ticket #1471, yes, 1946 is consistent
> with what mediainfo shows for creation time. Obviously 1946 was not the
> intended creation time, but that does not warrant us to break files where
> 1946 is the *intended* creation time. Proper way to fix the original issue
> would be to detect the device and software version which produces the
> invalid files, and only apply the hack there. But I don't think that is
> doable here, the file does not seem to contain any device or software
> information.

what do you mean by intended creation time?
the file format did not exist in 1946. and all the codecs also didnt exist
so when you encounter a file that says its from that time it must be crafted
later and backdated or that bug.
we know the bug is a real thing
do you want to support crafted and backdatred files? if so can you explain
the usecase for that ?
maybe iam missing something

thx

[...]
Marton Balint April 9, 2023, 5:52 p.m. UTC | #5
On Sun, 9 Apr 2023, Michael Niedermayer wrote:

> On Sun, Apr 09, 2023 at 03:49:33PM +0200, Marton Balint wrote:
>>
>>
>> On Sat, 8 Apr 2023, Michael Niedermayer wrote:
>>
>>> On Sat, Apr 08, 2023 at 08:37:24PM +0200, Marton Balint wrote:
>>>> Commit 23eeffcd48a15e73fb2649b712870b6d101c5471 added a hack to support invalid
>>>> files where the creation date was encoded as a classic unix timestamp. This
>>>> broke however valid files having creation dates before the unix epoch.
>>>>
>>>> Signed-off-by: Marton Balint <cus@passwd.hu>
>>>> ---
>>>>  libavformat/mov.c | 3 +--
>>>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>>
>>> This results in:
>>> @@ -1,11 +1,11 @@
>>> -    creation_time   : 2012-06-20T20:58:31.000000Z
>>> -      creation_time   : 2012-06-20T20:58:31.000000Z
>>> -      creation_time   : 2012-06-20T20:58:31.000000Z
>>> +    creation_time   : 1946-06-20T20:58:31.000000Z
>>> +      creation_time   : 1946-06-20T20:58:31.000000Z
>>> +      creation_time   : 1946-06-20T20:58:31.000000Z
>>>
>>> Are you sure that 1946 is the correct creation date and not 2012 ?
>>
>> If you are referring to the file in ticket #1471, yes, 1946 is consistent
>> with what mediainfo shows for creation time. Obviously 1946 was not the
>> intended creation time, but that does not warrant us to break files where
>> 1946 is the *intended* creation time. Proper way to fix the original issue
>> would be to detect the device and software version which produces the
>> invalid files, and only apply the hack there. But I don't think that is
>> doable here, the file does not seem to contain any device or software
>> information.
>
> what do you mean by intended creation time?
> the file format did not exist in 1946. and all the codecs also didnt exist
> so when you encounter a file that says its from that time it must be crafted
> later and backdated or that bug.
> we know the bug is a real thing
> do you want to support crafted and backdatred files? if so can you explain
> the usecase for that ?

http://ffmpeg.org/pipermail/ffmpeg-user/2023-April/056265.html

Alternatives I can think of:

1) A -unix_time switch what Anton proposed
2) doing strict compliant parsing only if mdat version is 1 so creation 
time is 64bit. And change our muxer to write mdat version 1 by default, so 
ffmpeg will be able to read back what it has written...

Regards,
Marton
Michael Niedermayer April 9, 2023, 9:27 p.m. UTC | #6
On Sun, Apr 09, 2023 at 07:52:12PM +0200, Marton Balint wrote:
> 
> 
> On Sun, 9 Apr 2023, Michael Niedermayer wrote:
> 
> > On Sun, Apr 09, 2023 at 03:49:33PM +0200, Marton Balint wrote:
> > > 
> > > 
> > > On Sat, 8 Apr 2023, Michael Niedermayer wrote:
> > > 
> > > > On Sat, Apr 08, 2023 at 08:37:24PM +0200, Marton Balint wrote:
> > > > > Commit 23eeffcd48a15e73fb2649b712870b6d101c5471 added a hack to support invalid
> > > > > files where the creation date was encoded as a classic unix timestamp. This
> > > > > broke however valid files having creation dates before the unix epoch.
> > > > > 
> > > > > Signed-off-by: Marton Balint <cus@passwd.hu>
> > > > > ---
> > > > >  libavformat/mov.c | 3 +--
> > > > >  1 file changed, 1 insertion(+), 2 deletions(-)
> > > > 
> > > > This results in:
> > > > @@ -1,11 +1,11 @@
> > > > -    creation_time   : 2012-06-20T20:58:31.000000Z
> > > > -      creation_time   : 2012-06-20T20:58:31.000000Z
> > > > -      creation_time   : 2012-06-20T20:58:31.000000Z
> > > > +    creation_time   : 1946-06-20T20:58:31.000000Z
> > > > +      creation_time   : 1946-06-20T20:58:31.000000Z
> > > > +      creation_time   : 1946-06-20T20:58:31.000000Z
> > > > 
> > > > Are you sure that 1946 is the correct creation date and not 2012 ?
> > > 
> > > If you are referring to the file in ticket #1471, yes, 1946 is consistent
> > > with what mediainfo shows for creation time. Obviously 1946 was not the
> > > intended creation time, but that does not warrant us to break files where
> > > 1946 is the *intended* creation time. Proper way to fix the original issue
> > > would be to detect the device and software version which produces the
> > > invalid files, and only apply the hack there. But I don't think that is
> > > doable here, the file does not seem to contain any device or software
> > > information.
> > 
> > what do you mean by intended creation time?
> > the file format did not exist in 1946. and all the codecs also didnt exist
> > so when you encounter a file that says its from that time it must be crafted
> > later and backdated or that bug.
> > we know the bug is a real thing
> > do you want to support crafted and backdatred files? if so can you explain
> > the usecase for that ?
> 
> http://ffmpeg.org/pipermail/ffmpeg-user/2023-April/056265.html
> 
> Alternatives I can think of:
> 
> 1) A -unix_time switch what Anton proposed

> 2) doing strict compliant parsing only if mdat version is 1 so creation time
> is 64bit. And change our muxer to write mdat version 1 by default, so ffmpeg
> will be able to read back what it has written...

What do we know about the buggy files that need this correction ?
Is there any hint/metadata that identifies the muxer/encoder/version ?

Limiting the correction to the cases that need it is a good idea
Iam not sure i feel positive about changing the muxer

thx


[...]
Zhao Zhili April 10, 2023, 2:44 a.m. UTC | #7
> On Apr 10, 2023, at 01:20, Michael Niedermayer <michael@niedermayer.cc> wrote:
> 
> On Sun, Apr 09, 2023 at 03:49:33PM +0200, Marton Balint wrote:
>> 
>> 
>> On Sat, 8 Apr 2023, Michael Niedermayer wrote:
>> 
>>> On Sat, Apr 08, 2023 at 08:37:24PM +0200, Marton Balint wrote:
>>>> Commit 23eeffcd48a15e73fb2649b712870b6d101c5471 added a hack to support invalid
>>>> files where the creation date was encoded as a classic unix timestamp. This
>>>> broke however valid files having creation dates before the unix epoch.
>>>> 
>>>> Signed-off-by: Marton Balint <cus@passwd.hu>
>>>> ---
>>>> libavformat/mov.c | 3 +--
>>>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>> 
>>> This results in:
>>> @@ -1,11 +1,11 @@
>>> -    creation_time   : 2012-06-20T20:58:31.000000Z
>>> -      creation_time   : 2012-06-20T20:58:31.000000Z
>>> -      creation_time   : 2012-06-20T20:58:31.000000Z
>>> +    creation_time   : 1946-06-20T20:58:31.000000Z
>>> +      creation_time   : 1946-06-20T20:58:31.000000Z
>>> +      creation_time   : 1946-06-20T20:58:31.000000Z
>>> 
>>> Are you sure that 1946 is the correct creation date and not 2012 ?
>> 
>> If you are referring to the file in ticket #1471, yes, 1946 is consistent
>> with what mediainfo shows for creation time. Obviously 1946 was not the
>> intended creation time, but that does not warrant us to break files where
>> 1946 is the *intended* creation time. Proper way to fix the original issue
>> would be to detect the device and software version which produces the
>> invalid files, and only apply the hack there. But I don't think that is
>> doable here, the file does not seem to contain any device or software
>> information.
> 
> what do you mean by intended creation time?
> the file format did not exist in 1946. and all the codecs also didnt exist
> so when you encounter a file that says its from that time it must be crafted
> later and backdated or that bug.
> we know the bug is a real thing
> do you want to support crafted and backdatred files? if so can you explain
> the usecase for that ?
> maybe iam missing something

The workaround can be a positive feedback to create more broken files:

1. Once FFmpeg demuxer supports it, other demuxers may be required to
do the same workaround, because FFmpeg is (kind of) the standard software.

2. Someone write a new muxer can make the same mistake and doesn’t notice,
since FFmpeg don’t complain on the broken files.

3. Old muxers which create those files don’t fix the bug, because FFmpeg
doesn't complain.

We need to workaround bugs which break the process of demux or decoding.
Metadata with invalid values doesn't has the same priority. Make a big
noise at least to reduce the positive feedback effect, so user can report
bugs to the right place.

> 
> thx
> 
> [...]
> -- 
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> 
> Does the universe only have a finite lifespan? No, its going to go on
> forever, its just that you wont like living in it. -- Hiranya Peiri
> _______________________________________________
> 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 April 10, 2023, 2:31 p.m. UTC | #8
On Mon, Apr 10, 2023 at 10:44:32AM +0800, "zhilizhao(赵志立)" wrote:
> 
> 
> > On Apr 10, 2023, at 01:20, Michael Niedermayer <michael@niedermayer.cc> wrote:
> > 
> > On Sun, Apr 09, 2023 at 03:49:33PM +0200, Marton Balint wrote:
> >> 
> >> 
> >> On Sat, 8 Apr 2023, Michael Niedermayer wrote:
> >> 
> >>> On Sat, Apr 08, 2023 at 08:37:24PM +0200, Marton Balint wrote:
> >>>> Commit 23eeffcd48a15e73fb2649b712870b6d101c5471 added a hack to support invalid
> >>>> files where the creation date was encoded as a classic unix timestamp. This
> >>>> broke however valid files having creation dates before the unix epoch.
> >>>> 
> >>>> Signed-off-by: Marton Balint <cus@passwd.hu>
> >>>> ---
> >>>> libavformat/mov.c | 3 +--
> >>>> 1 file changed, 1 insertion(+), 2 deletions(-)
> >>> 
> >>> This results in:
> >>> @@ -1,11 +1,11 @@
> >>> -    creation_time   : 2012-06-20T20:58:31.000000Z
> >>> -      creation_time   : 2012-06-20T20:58:31.000000Z
> >>> -      creation_time   : 2012-06-20T20:58:31.000000Z
> >>> +    creation_time   : 1946-06-20T20:58:31.000000Z
> >>> +      creation_time   : 1946-06-20T20:58:31.000000Z
> >>> +      creation_time   : 1946-06-20T20:58:31.000000Z
> >>> 
> >>> Are you sure that 1946 is the correct creation date and not 2012 ?
> >> 
> >> If you are referring to the file in ticket #1471, yes, 1946 is consistent
> >> with what mediainfo shows for creation time. Obviously 1946 was not the
> >> intended creation time, but that does not warrant us to break files where
> >> 1946 is the *intended* creation time. Proper way to fix the original issue
> >> would be to detect the device and software version which produces the
> >> invalid files, and only apply the hack there. But I don't think that is
> >> doable here, the file does not seem to contain any device or software
> >> information.
> > 
> > what do you mean by intended creation time?
> > the file format did not exist in 1946. and all the codecs also didnt exist
> > so when you encounter a file that says its from that time it must be crafted
> > later and backdated or that bug.
> > we know the bug is a real thing
> > do you want to support crafted and backdatred files? if so can you explain
> > the usecase for that ?
> > maybe iam missing something
> 
> The workaround can be a positive feedback to create more broken files:
> 
> 1. Once FFmpeg demuxer supports it, other demuxers may be required to
> do the same workaround, because FFmpeg is (kind of) the standard software.

In this case iam not sure, its just a fairly irrelevant piece of metadata
Also why would we care that a feature causes work to the competition 


> 
> 2. Someone write a new muxer can make the same mistake and doesn’t notice,
> since FFmpeg don’t complain on the broken files.
> 
> 3. Old muxers which create those files don’t fix the bug, because FFmpeg
> doesn't complain.
> 
> We need to workaround bugs which break the process of demux or decoding.
> Metadata with invalid values doesn't has the same priority. Make a big
> noise at least to reduce the positive feedback effect, so user can report
> bugs to the right place.

I agree we should be more noisy about it when we encounter broken files.

About causing more broken files with bug workarounds, we have worked
around bugs alot so one would expect that there have to be many stories
of that causing new bugs and unfixed bugs not just hypothethial cases.
Still every time i read about it its a hypothethial case not a real
example that somewhere a bug was caused by a workaround. So maybe
that direction is relatively rare

thx

[...]
Marton Balint April 10, 2023, 7:11 p.m. UTC | #9
On Sun, 9 Apr 2023, Michael Niedermayer wrote:

> On Sun, Apr 09, 2023 at 07:52:12PM +0200, Marton Balint wrote:
>>
>>
>> On Sun, 9 Apr 2023, Michael Niedermayer wrote:
>>
>>> On Sun, Apr 09, 2023 at 03:49:33PM +0200, Marton Balint wrote:
>>>>
>>>>
>>>> On Sat, 8 Apr 2023, Michael Niedermayer wrote:
>>>>
>>>>> On Sat, Apr 08, 2023 at 08:37:24PM +0200, Marton Balint wrote:
>>>>>> Commit 23eeffcd48a15e73fb2649b712870b6d101c5471 added a hack to support invalid
>>>>>> files where the creation date was encoded as a classic unix timestamp. This
>>>>>> broke however valid files having creation dates before the unix epoch.
>>>>>>
>>>>>> Signed-off-by: Marton Balint <cus@passwd.hu>
>>>>>> ---
>>>>>>  libavformat/mov.c | 3 +--
>>>>>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>>>>
>>>>> This results in:
>>>>> @@ -1,11 +1,11 @@
>>>>> -    creation_time   : 2012-06-20T20:58:31.000000Z
>>>>> -      creation_time   : 2012-06-20T20:58:31.000000Z
>>>>> -      creation_time   : 2012-06-20T20:58:31.000000Z
>>>>> +    creation_time   : 1946-06-20T20:58:31.000000Z
>>>>> +      creation_time   : 1946-06-20T20:58:31.000000Z
>>>>> +      creation_time   : 1946-06-20T20:58:31.000000Z
>>>>>
>>>>> Are you sure that 1946 is the correct creation date and not 2012 ?
>>>>
>>>> If you are referring to the file in ticket #1471, yes, 1946 is consistent
>>>> with what mediainfo shows for creation time. Obviously 1946 was not the
>>>> intended creation time, but that does not warrant us to break files where
>>>> 1946 is the *intended* creation time. Proper way to fix the original issue
>>>> would be to detect the device and software version which produces the
>>>> invalid files, and only apply the hack there. But I don't think that is
>>>> doable here, the file does not seem to contain any device or software
>>>> information.
>>>
>>> what do you mean by intended creation time?
>>> the file format did not exist in 1946. and all the codecs also didnt exist
>>> so when you encounter a file that says its from that time it must be crafted
>>> later and backdated or that bug.
>>> we know the bug is a real thing
>>> do you want to support crafted and backdatred files? if so can you explain
>>> the usecase for that ?
>>
>> http://ffmpeg.org/pipermail/ffmpeg-user/2023-April/056265.html
>>
>> Alternatives I can think of:
>>
>> 1) A -unix_time switch what Anton proposed
>
>> 2) doing strict compliant parsing only if mdat version is 1 so creation time
>> is 64bit. And change our muxer to write mdat version 1 by default, so ffmpeg
>> will be able to read back what it has written...
>
> What do we know about the buggy files that need this correction ?

Not much. Samsung Galaxy Nexus phone and Samsung HMX-S16BP camcorder seems 
affected at least. But not exclusively. I found these files among the 
samples:

Samsung HMX-S16BP:
samples/ffmpeg-bugs/roundup/issue2517/HDV_0112.MP4
samples/ffmpeg-bugs/roundup/issue2517_HDV_0113.MP4

Galaxy Nexus:
Sample in ticket 1471.

Unknown:
samples/ffmpeg-bugs/trac/ticket2095_385 Deadlist Form 2-7-13.mp4
samples/ffmpeg-bugs/trac/ticket3399_VID_20130619_161750_449.mp4
user/aac-input-buffer-exhausted-up_1434137794_VID_20120604_172442.mp4

> Is there any hint/metadata that identifies the muxer/encoder/version ?

In case of HMX-S16BP yes, in other cases no.

>
> Limiting the correction to the cases that need it is a good idea

I guess I will start with limiting workaround for version 1 as a start.

> Iam not sure i feel positive about changing the muxer

Eventually we have to bump the mdhd version, because after 2040 creation 
time will overflow. We should not wait until 2040 to do that, neither we 
should start writing version 1 only after 2040, that would be Y2K problem 
waiting to happen.

Regards,
Marton
Michael Niedermayer April 10, 2023, 10:34 p.m. UTC | #10
On Mon, Apr 10, 2023 at 09:11:04PM +0200, Marton Balint wrote:
> 
> 
> On Sun, 9 Apr 2023, Michael Niedermayer wrote:
> 
> > On Sun, Apr 09, 2023 at 07:52:12PM +0200, Marton Balint wrote:
> > > 
> > > 
> > > On Sun, 9 Apr 2023, Michael Niedermayer wrote:
> > > 
> > > > On Sun, Apr 09, 2023 at 03:49:33PM +0200, Marton Balint wrote:
> > > > > 
> > > > > 
> > > > > On Sat, 8 Apr 2023, Michael Niedermayer wrote:
> > > > > 
> > > > > > On Sat, Apr 08, 2023 at 08:37:24PM +0200, Marton Balint wrote:
> > > > > > > Commit 23eeffcd48a15e73fb2649b712870b6d101c5471 added a hack to support invalid
> > > > > > > files where the creation date was encoded as a classic unix timestamp. This
> > > > > > > broke however valid files having creation dates before the unix epoch.
> > > > > > > 
> > > > > > > Signed-off-by: Marton Balint <cus@passwd.hu>
> > > > > > > ---
> > > > > > >  libavformat/mov.c | 3 +--
> > > > > > >  1 file changed, 1 insertion(+), 2 deletions(-)
> > > > > > 
> > > > > > This results in:
> > > > > > @@ -1,11 +1,11 @@
> > > > > > -    creation_time   : 2012-06-20T20:58:31.000000Z
> > > > > > -      creation_time   : 2012-06-20T20:58:31.000000Z
> > > > > > -      creation_time   : 2012-06-20T20:58:31.000000Z
> > > > > > +    creation_time   : 1946-06-20T20:58:31.000000Z
> > > > > > +      creation_time   : 1946-06-20T20:58:31.000000Z
> > > > > > +      creation_time   : 1946-06-20T20:58:31.000000Z
> > > > > > 
> > > > > > Are you sure that 1946 is the correct creation date and not 2012 ?
> > > > > 
> > > > > If you are referring to the file in ticket #1471, yes, 1946 is consistent
> > > > > with what mediainfo shows for creation time. Obviously 1946 was not the
> > > > > intended creation time, but that does not warrant us to break files where
> > > > > 1946 is the *intended* creation time. Proper way to fix the original issue
> > > > > would be to detect the device and software version which produces the
> > > > > invalid files, and only apply the hack there. But I don't think that is
> > > > > doable here, the file does not seem to contain any device or software
> > > > > information.
> > > > 
> > > > what do you mean by intended creation time?
> > > > the file format did not exist in 1946. and all the codecs also didnt exist
> > > > so when you encounter a file that says its from that time it must be crafted
> > > > later and backdated or that bug.
> > > > we know the bug is a real thing
> > > > do you want to support crafted and backdatred files? if so can you explain
> > > > the usecase for that ?
> > > 
> > > http://ffmpeg.org/pipermail/ffmpeg-user/2023-April/056265.html
> > > 
> > > Alternatives I can think of:
> > > 
> > > 1) A -unix_time switch what Anton proposed
> > 
> > > 2) doing strict compliant parsing only if mdat version is 1 so creation time
> > > is 64bit. And change our muxer to write mdat version 1 by default, so ffmpeg
> > > will be able to read back what it has written...
> > 
> > What do we know about the buggy files that need this correction ?
> 
> Not much. Samsung Galaxy Nexus phone and Samsung HMX-S16BP camcorder seems
> affected at least. But not exclusively. I found these files among the
> samples:
> 
> Samsung HMX-S16BP:
> samples/ffmpeg-bugs/roundup/issue2517/HDV_0112.MP4
> samples/ffmpeg-bugs/roundup/issue2517_HDV_0113.MP4
> 
> Galaxy Nexus:
> Sample in ticket 1471.
> 
> Unknown:
> samples/ffmpeg-bugs/trac/ticket2095_385 Deadlist Form 2-7-13.mp4
> samples/ffmpeg-bugs/trac/ticket3399_VID_20130619_161750_449.mp4
> user/aac-input-buffer-exhausted-up_1434137794_VID_20120604_172442.mp4
> 
> > Is there any hint/metadata that identifies the muxer/encoder/version ?
> 
> In case of HMX-S16BP yes, in other cases no.
> 
> > 
> > Limiting the correction to the cases that need it is a good idea
> 
> I guess I will start with limiting workaround for version 1 as a start.
> 
> > Iam not sure i feel positive about changing the muxer
> 
> Eventually we have to bump the mdhd version, because after 2040 creation
> time will overflow. We should not wait until 2040 to do that, neither we
> should start writing version 1 only after 2040, that would be Y2K problem
> waiting to happen.

yes sure, if theres a problem with the muxer then thats perfectly fine
to change the muxer. I just meant that if the only reason is a buggy 
3rd party muxer then changing our muxer felt like something to rethink
and sleep over first.

Iam not against bumping the mdhd version in master at some point (not immedeatly
before a release).

thx

[...]
diff mbox series

Patch

diff --git a/libavformat/mov.c b/libavformat/mov.c
index 057fd872b1..7748bdc041 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -1518,8 +1518,7 @@  static int mov_read_moof(MOVContext *c, AVIOContext *pb, MOVAtom atom)
 static void mov_metadata_creation_time(AVDictionary **metadata, int64_t time, void *logctx)
 {
     if (time) {
-        if (time >= 2082844800)
-            time -= 2082844800;  /* seconds between 1904-01-01 and Epoch */
+        time -= 2082844800;  /* seconds between 1904-01-01 and Epoch */
 
         if ((int64_t)(time * 1000000ULL) / 1000000 != time) {
             av_log(logctx, AV_LOG_DEBUG, "creation_time is not representable\n");