[FFmpeg-devel] [PATCH 1/2] avformat/movenc: initialize pts/dts/duration of timecode packet

[FFmpeg-devel] [PATCH 1/2] avformat/movenc: initialize pts/dts/duration of timecode packet

[lance.lmwang]

From: Limin Wang <lance.lmwang@gmail.com>

Fix below error message when timecode packet is written.
"Application provided duration: -9223372036854775808 / timestamp: -9223372036854775808 is out of range for mov/mp4 format"

try to reproduce by:
ffmpeg -y -f lavfi -i color -metadata "timecode=00:00:00:00" -t 1 test.mov

Note although error message is printed, the timecode packet will be written anyway. So
the patch 2/2 will try to change the log level to warning.

The first two test case of fate-lavf-ismv have timecode setting, so the crc of ref data is different.
Fixes ticket #9488

Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
---
 libavformat/movenc.c | 2 ++
 tests/ref/lavf/ismv  | 4 ++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 4c86891..74b94cd 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -6383,6 +6383,8 @@ static int mov_create_timecode_track(AVFormatContext *s, int index, int src_inde
     pkt->data = data;
     pkt->stream_index = index;
     pkt->flags = AV_PKT_FLAG_KEY;
+    pkt->pts = pkt->dts = av_rescale_q(tc.start, av_inv_q(rate), (AVRational){1,mov->movie_timescale});
+    pkt->duration = av_rescale_q(1, av_inv_q(rate), (AVRational){1,mov->movie_timescale});
     pkt->size = 4;
     AV_WB32(pkt->data, tc.start);
     ret = ff_mov_write_packet(s, pkt);
diff --git a/tests/ref/lavf/ismv b/tests/ref/lavf/ismv
index ac7f72b..723b432 100644
--- a/tests/ref/lavf/ismv
+++ b/tests/ref/lavf/ismv
@@ -1,7 +1,7 @@
-48fb8d7a5d19bd60f3a49ccf4b7d6593 *tests/data/lavf/lavf.ismv
+7a24b73c096ec0f13f0f7a2d9101c4c1 *tests/data/lavf/lavf.ismv
 313169 tests/data/lavf/lavf.ismv
 tests/data/lavf/lavf.ismv CRC=0x9d9a638a
-d19cd8e310a2e94fe0a0d11c5dc29217 *tests/data/lavf/lavf.ismv
+79646383fd099d45ad0d0c2791c601dd *tests/data/lavf/lavf.ismv
 322075 tests/data/lavf/lavf.ismv
 tests/data/lavf/lavf.ismv CRC=0xe8130120
 3b6023766845b51b075aed474c00f73c *tests/data/lavf/lavf.ismv
-- 
1.8.3.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] [PATCH 2/2] avformat/movenc: use warning log level and small adjustment for the log

[lance.lmwang]

From: Limin Wang <lance.lmwang@gmail.com>

Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
---
 libavformat/movenc.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 74b94cd..8b038c1 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -5638,9 +5638,8 @@ static int check_pkt(AVFormatContext *s, AVPacket *pkt)
 
     duration = pkt->dts - ref;
     if (pkt->dts < ref || duration >= INT_MAX) {
-        av_log(s, AV_LOG_ERROR, "Application provided duration: %"PRId64" / timestamp: %"PRId64" is out of range for mov/mp4 format\n",
-            duration, pkt->dts
-        );
+        av_log(s, AV_LOG_WARNING, "Packet duration: %"PRId64" / dts: %"PRId64" is out of range\n",
+               duration, pkt->dts);
 
         pkt->dts = ref + 1;
         pkt->pts = AV_NOPTS_VALUE;
-- 
1.8.3.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 1/2] avformat/movenc: initialize pts/dts/duration of timecode packet

[Andreas Rheinhardt]

lance.lmwang@gmail.com:
> From: Limin Wang <lance.lmwang@gmail.com>
> 
> Fix below error message when timecode packet is written.
> "Application provided duration: -9223372036854775808 / timestamp: -9223372036854775808 is out of range for mov/mp4 format"
> 
> try to reproduce by:
> ffmpeg -y -f lavfi -i color -metadata "timecode=00:00:00:00" -t 1 test.mov
> 
> Note although error message is printed, the timecode packet will be written anyway. So
> the patch 2/2 will try to change the log level to warning.
> 
> The first two test case of fate-lavf-ismv have timecode setting, so the crc of ref data is different.
> Fixes ticket #9488
> 
> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> ---
>  libavformat/movenc.c | 2 ++
>  tests/ref/lavf/ismv  | 4 ++--
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
> index 4c86891..74b94cd 100644
> --- a/libavformat/movenc.c
> +++ b/libavformat/movenc.c
> @@ -6383,6 +6383,8 @@ static int mov_create_timecode_track(AVFormatContext *s, int index, int src_inde
>      pkt->data = data;
>      pkt->stream_index = index;
>      pkt->flags = AV_PKT_FLAG_KEY;
> +    pkt->pts = pkt->dts = av_rescale_q(tc.start, av_inv_q(rate), (AVRational){1,mov->movie_timescale});
> +    pkt->duration = av_rescale_q(1, av_inv_q(rate), (AVRational){1,mov->movie_timescale});
>      pkt->size = 4;
>      AV_WB32(pkt->data, tc.start);
>      ret = ff_mov_write_packet(s, pkt);
> diff --git a/tests/ref/lavf/ismv b/tests/ref/lavf/ismv
> index ac7f72b..723b432 100644
> --- a/tests/ref/lavf/ismv
> +++ b/tests/ref/lavf/ismv
> @@ -1,7 +1,7 @@
> -48fb8d7a5d19bd60f3a49ccf4b7d6593 *tests/data/lavf/lavf.ismv
> +7a24b73c096ec0f13f0f7a2d9101c4c1 *tests/data/lavf/lavf.ismv
>  313169 tests/data/lavf/lavf.ismv
>  tests/data/lavf/lavf.ismv CRC=0x9d9a638a
> -d19cd8e310a2e94fe0a0d11c5dc29217 *tests/data/lavf/lavf.ismv
> +79646383fd099d45ad0d0c2791c601dd *tests/data/lavf/lavf.ismv
>  322075 tests/data/lavf/lavf.ismv
>  tests/data/lavf/lavf.ismv CRC=0xe8130120
>  3b6023766845b51b075aed474c00f73c *tests/data/lavf/lavf.ismv

Are the currently created files spec-incompliant? Or will the files
created with this patch be spec-incompliant?

- Andreas
_______________________________________________
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 1/2] avformat/movenc: initialize pts/dts/duration of timecode packet

[lance.lmwang]

On Thu, Mar 03, 2022 at 02:55:23AM +0100, Andreas Rheinhardt wrote:
> lance.lmwang@gmail.com:
> > From: Limin Wang <lance.lmwang@gmail.com>
> > 
> > Fix below error message when timecode packet is written.
> > "Application provided duration: -9223372036854775808 / timestamp: -9223372036854775808 is out of range for mov/mp4 format"
> > 
> > try to reproduce by:
> > ffmpeg -y -f lavfi -i color -metadata "timecode=00:00:00:00" -t 1 test.mov
> > 
> > Note although error message is printed, the timecode packet will be written anyway. So
> > the patch 2/2 will try to change the log level to warning.
> > 
> > The first two test case of fate-lavf-ismv have timecode setting, so the crc of ref data is different.
> > Fixes ticket #9488
> > 
> > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> > ---
> >  libavformat/movenc.c | 2 ++
> >  tests/ref/lavf/ismv  | 4 ++--
> >  2 files changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/libavformat/movenc.c b/libavformat/movenc.c
> > index 4c86891..74b94cd 100644
> > --- a/libavformat/movenc.c
> > +++ b/libavformat/movenc.c
> > @@ -6383,6 +6383,8 @@ static int mov_create_timecode_track(AVFormatContext *s, int index, int src_inde
> >      pkt->data = data;
> >      pkt->stream_index = index;
> >      pkt->flags = AV_PKT_FLAG_KEY;
> > +    pkt->pts = pkt->dts = av_rescale_q(tc.start, av_inv_q(rate), (AVRational){1,mov->movie_timescale});
> > +    pkt->duration = av_rescale_q(1, av_inv_q(rate), (AVRational){1,mov->movie_timescale});
> >      pkt->size = 4;
> >      AV_WB32(pkt->data, tc.start);
> >      ret = ff_mov_write_packet(s, pkt);
> > diff --git a/tests/ref/lavf/ismv b/tests/ref/lavf/ismv
> > index ac7f72b..723b432 100644
> > --- a/tests/ref/lavf/ismv
> > +++ b/tests/ref/lavf/ismv
> > @@ -1,7 +1,7 @@
> > -48fb8d7a5d19bd60f3a49ccf4b7d6593 *tests/data/lavf/lavf.ismv
> > +7a24b73c096ec0f13f0f7a2d9101c4c1 *tests/data/lavf/lavf.ismv
> >  313169 tests/data/lavf/lavf.ismv
> >  tests/data/lavf/lavf.ismv CRC=0x9d9a638a
> > -d19cd8e310a2e94fe0a0d11c5dc29217 *tests/data/lavf/lavf.ismv
> > +79646383fd099d45ad0d0c2791c601dd *tests/data/lavf/lavf.ismv
> >  322075 tests/data/lavf/lavf.ismv
> >  tests/data/lavf/lavf.ismv CRC=0xe8130120
> >  3b6023766845b51b075aed474c00f73c *tests/data/lavf/lavf.ismv
> 
> Are the currently created files spec-incompliant? Or will the files
> created with this patch be spec-incompliant?

I think both the currently created files and after are spec-compliant as the
pts/dts isn't used by tmcd track I think.

The currently code will trigger below condition as the pts/dts isn't initialized:
[ismv @ 0x56c8c40] Application provided duration: -9223372036854775808 / timestamp: -9223372036854775808 is out of range for mov/mp4 format

so the dts and pts will try to set them as the following code:
5640     if (pkt->dts < ref || duration >= INT_MAX) {
5641         av_log(s, AV_LOG_ERROR, "Application provided duration: %"PRId64" / timestamp: %"PRId64" is out of range for mov/mp4 format\n",
5642             duration, pkt->dts
5643         );
5644
5645         pkt->dts = ref + 1;
5646         pkt->pts = AV_NOPTS_VALUE;
5647     }

By the comparing hex string by before and after, one byte is different, but I haven't figured
out where is it yet, but it's related pts and dts value.

[ffmpeg.git]$ hexdump lavf_before.ismv > lavf_before.log
[ffmpeg.git]$ hexdump lavf_after.ismv > lavf_after.log
[ffmpeg.git]$ diff lavf_before.log lavf_after.log
8974c8974
< 00230d0 0000 0000 0000 0000 6d0c 6164 0074 0804
---
> 00230d0 0000 0000 0028 0000 6d0c 6164 0074 0804


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

-- 
Thanks,
Limin Wang
_______________________________________________
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 1/2] avformat/movenc: initialize pts/dts/duration of timecode packet

[lance.lmwang]

On Wed, Mar 02, 2022 at 09:58:31PM +0800, lance.lmwang@gmail.com wrote:
> From: Limin Wang <lance.lmwang@gmail.com>
> 
> Fix below error message when timecode packet is written.
> "Application provided duration: -9223372036854775808 / timestamp: -9223372036854775808 is out of range for mov/mp4 format"
> 
> try to reproduce by:
> ffmpeg -y -f lavfi -i color -metadata "timecode=00:00:00:00" -t 1 test.mov
> 
> Note although error message is printed, the timecode packet will be written anyway. So
> the patch 2/2 will try to change the log level to warning.
> 
> The first two test case of fate-lavf-ismv have timecode setting, so the crc of ref data is different.
> Fixes ticket #9488
> 
> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> ---
>  libavformat/movenc.c | 2 ++
>  tests/ref/lavf/ismv  | 4 ++--
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
> index 4c86891..74b94cd 100644
> --- a/libavformat/movenc.c
> +++ b/libavformat/movenc.c
> @@ -6383,6 +6383,8 @@ static int mov_create_timecode_track(AVFormatContext *s, int index, int src_inde
>      pkt->data = data;
>      pkt->stream_index = index;
>      pkt->flags = AV_PKT_FLAG_KEY;
> +    pkt->pts = pkt->dts = av_rescale_q(tc.start, av_inv_q(rate), (AVRational){1,mov->movie_timescale});
> +    pkt->duration = av_rescale_q(1, av_inv_q(rate), (AVRational){1,mov->movie_timescale});
>      pkt->size = 4;
>      AV_WB32(pkt->data, tc.start);
>      ret = ff_mov_write_packet(s, pkt);
> diff --git a/tests/ref/lavf/ismv b/tests/ref/lavf/ismv
> index ac7f72b..723b432 100644
> --- a/tests/ref/lavf/ismv
> +++ b/tests/ref/lavf/ismv
> @@ -1,7 +1,7 @@
> -48fb8d7a5d19bd60f3a49ccf4b7d6593 *tests/data/lavf/lavf.ismv
> +7a24b73c096ec0f13f0f7a2d9101c4c1 *tests/data/lavf/lavf.ismv
>  313169 tests/data/lavf/lavf.ismv
>  tests/data/lavf/lavf.ismv CRC=0x9d9a638a
> -d19cd8e310a2e94fe0a0d11c5dc29217 *tests/data/lavf/lavf.ismv
> +79646383fd099d45ad0d0c2791c601dd *tests/data/lavf/lavf.ismv
>  322075 tests/data/lavf/lavf.ismv
>  tests/data/lavf/lavf.ismv CRC=0xe8130120
>  3b6023766845b51b075aed474c00f73c *tests/data/lavf/lavf.ismv
> -- 
> 1.8.3.1
> 

will apply the patch set tomorrow unless there are any objections.

-- 
Thanks,
Limin Wang
_______________________________________________
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 1/2] avformat/movenc: initialize pts/dts/duration of timecode packet

[Andreas Rheinhardt]

lance.lmwang@gmail.com:
> On Wed, Mar 02, 2022 at 09:58:31PM +0800, lance.lmwang@gmail.com wrote:
>> From: Limin Wang <lance.lmwang@gmail.com>
>>
>> Fix below error message when timecode packet is written.
>> "Application provided duration: -9223372036854775808 / timestamp: -9223372036854775808 is out of range for mov/mp4 format"
>>
>> try to reproduce by:
>> ffmpeg -y -f lavfi -i color -metadata "timecode=00:00:00:00" -t 1 test.mov
>>
>> Note although error message is printed, the timecode packet will be written anyway. So
>> the patch 2/2 will try to change the log level to warning.
>>
>> The first two test case of fate-lavf-ismv have timecode setting, so the crc of ref data is different.
>> Fixes ticket #9488
>>
>> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
>> ---
>>  libavformat/movenc.c | 2 ++
>>  tests/ref/lavf/ismv  | 4 ++--
>>  2 files changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
>> index 4c86891..74b94cd 100644
>> --- a/libavformat/movenc.c
>> +++ b/libavformat/movenc.c
>> @@ -6383,6 +6383,8 @@ static int mov_create_timecode_track(AVFormatContext *s, int index, int src_inde
>>      pkt->data = data;
>>      pkt->stream_index = index;
>>      pkt->flags = AV_PKT_FLAG_KEY;
>> +    pkt->pts = pkt->dts = av_rescale_q(tc.start, av_inv_q(rate), (AVRational){1,mov->movie_timescale});
>> +    pkt->duration = av_rescale_q(1, av_inv_q(rate), (AVRational){1,mov->movie_timescale});
>>      pkt->size = 4;
>>      AV_WB32(pkt->data, tc.start);
>>      ret = ff_mov_write_packet(s, pkt);
>> diff --git a/tests/ref/lavf/ismv b/tests/ref/lavf/ismv
>> index ac7f72b..723b432 100644
>> --- a/tests/ref/lavf/ismv
>> +++ b/tests/ref/lavf/ismv
>> @@ -1,7 +1,7 @@
>> -48fb8d7a5d19bd60f3a49ccf4b7d6593 *tests/data/lavf/lavf.ismv
>> +7a24b73c096ec0f13f0f7a2d9101c4c1 *tests/data/lavf/lavf.ismv
>>  313169 tests/data/lavf/lavf.ismv
>>  tests/data/lavf/lavf.ismv CRC=0x9d9a638a
>> -d19cd8e310a2e94fe0a0d11c5dc29217 *tests/data/lavf/lavf.ismv
>> +79646383fd099d45ad0d0c2791c601dd *tests/data/lavf/lavf.ismv
>>  322075 tests/data/lavf/lavf.ismv
>>  tests/data/lavf/lavf.ismv CRC=0xe8130120
>>  3b6023766845b51b075aed474c00f73c *tests/data/lavf/lavf.ismv
>> -- 
>> 1.8.3.1
>>
> 
> will apply the patch set tomorrow unless there are any objections.
> 

You have not really answered whether the current files or the new files
are spec-incompliant; you have just reported that one byte is different.

- Andreas
_______________________________________________
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 1/2] avformat/movenc: initialize pts/dts/duration of timecode packet

[lance.lmwang]

On Fri, Mar 11, 2022 at 03:04:32PM +0100, Andreas Rheinhardt wrote:
> lance.lmwang@gmail.com:
> > On Wed, Mar 02, 2022 at 09:58:31PM +0800, lance.lmwang@gmail.com wrote:
> >> From: Limin Wang <lance.lmwang@gmail.com>
> >>
> >> Fix below error message when timecode packet is written.
> >> "Application provided duration: -9223372036854775808 / timestamp: -9223372036854775808 is out of range for mov/mp4 format"
> >>
> >> try to reproduce by:
> >> ffmpeg -y -f lavfi -i color -metadata "timecode=00:00:00:00" -t 1 test.mov
> >>
> >> Note although error message is printed, the timecode packet will be written anyway. So
> >> the patch 2/2 will try to change the log level to warning.
> >>
> >> The first two test case of fate-lavf-ismv have timecode setting, so the crc of ref data is different.
> >> Fixes ticket #9488
> >>
> >> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> >> ---
> >>  libavformat/movenc.c | 2 ++
> >>  tests/ref/lavf/ismv  | 4 ++--
> >>  2 files changed, 4 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
> >> index 4c86891..74b94cd 100644
> >> --- a/libavformat/movenc.c
> >> +++ b/libavformat/movenc.c
> >> @@ -6383,6 +6383,8 @@ static int mov_create_timecode_track(AVFormatContext *s, int index, int src_inde
> >>      pkt->data = data;
> >>      pkt->stream_index = index;
> >>      pkt->flags = AV_PKT_FLAG_KEY;
> >> +    pkt->pts = pkt->dts = av_rescale_q(tc.start, av_inv_q(rate), (AVRational){1,mov->movie_timescale});
> >> +    pkt->duration = av_rescale_q(1, av_inv_q(rate), (AVRational){1,mov->movie_timescale});
> >>      pkt->size = 4;
> >>      AV_WB32(pkt->data, tc.start);
> >>      ret = ff_mov_write_packet(s, pkt);
> >> diff --git a/tests/ref/lavf/ismv b/tests/ref/lavf/ismv
> >> index ac7f72b..723b432 100644
> >> --- a/tests/ref/lavf/ismv
> >> +++ b/tests/ref/lavf/ismv
> >> @@ -1,7 +1,7 @@
> >> -48fb8d7a5d19bd60f3a49ccf4b7d6593 *tests/data/lavf/lavf.ismv
> >> +7a24b73c096ec0f13f0f7a2d9101c4c1 *tests/data/lavf/lavf.ismv
> >>  313169 tests/data/lavf/lavf.ismv
> >>  tests/data/lavf/lavf.ismv CRC=0x9d9a638a
> >> -d19cd8e310a2e94fe0a0d11c5dc29217 *tests/data/lavf/lavf.ismv
> >> +79646383fd099d45ad0d0c2791c601dd *tests/data/lavf/lavf.ismv
> >>  322075 tests/data/lavf/lavf.ismv
> >>  tests/data/lavf/lavf.ismv CRC=0xe8130120
> >>  3b6023766845b51b075aed474c00f73c *tests/data/lavf/lavf.ismv
> >> -- 
> >> 1.8.3.1
> >>
> > 
> > will apply the patch set tomorrow unless there are any objections.
> > 
> 
> You have not really answered whether the current files or the new files
> are spec-incompliant; you have just reported that one byte is different.

Sorry, I think I have said both current and new file is spec-compliant in the last
email. 

By Quicktime file format specs:
Section Timecode Sample Description, all tmcd field isn't used pts/dts.

As for where is the different for one byte, it's caused by pkt->duration. The
old is 0(uninitialized), after the patch it's 33(1 frame duration).  

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

-- 
Thanks,
Limin Wang
_______________________________________________
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 1/2] avformat/movenc: initialize pts/dts/duration of timecode packet

[Andreas Rheinhardt]

lance.lmwang@gmail.com:
> On Fri, Mar 11, 2022 at 03:04:32PM +0100, Andreas Rheinhardt wrote:
>> lance.lmwang@gmail.com:
>>> On Wed, Mar 02, 2022 at 09:58:31PM +0800, lance.lmwang@gmail.com wrote:
>>>> From: Limin Wang <lance.lmwang@gmail.com>
>>>>
>>>> Fix below error message when timecode packet is written.
>>>> "Application provided duration: -9223372036854775808 / timestamp: -9223372036854775808 is out of range for mov/mp4 format"
>>>>
>>>> try to reproduce by:
>>>> ffmpeg -y -f lavfi -i color -metadata "timecode=00:00:00:00" -t 1 test.mov
>>>>
>>>> Note although error message is printed, the timecode packet will be written anyway. So
>>>> the patch 2/2 will try to change the log level to warning.
>>>>
>>>> The first two test case of fate-lavf-ismv have timecode setting, so the crc of ref data is different.
>>>> Fixes ticket #9488
>>>>
>>>> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
>>>> ---
>>>>  libavformat/movenc.c | 2 ++
>>>>  tests/ref/lavf/ismv  | 4 ++--
>>>>  2 files changed, 4 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
>>>> index 4c86891..74b94cd 100644
>>>> --- a/libavformat/movenc.c
>>>> +++ b/libavformat/movenc.c
>>>> @@ -6383,6 +6383,8 @@ static int mov_create_timecode_track(AVFormatContext *s, int index, int src_inde
>>>>      pkt->data = data;
>>>>      pkt->stream_index = index;
>>>>      pkt->flags = AV_PKT_FLAG_KEY;
>>>> +    pkt->pts = pkt->dts = av_rescale_q(tc.start, av_inv_q(rate), (AVRational){1,mov->movie_timescale});
>>>> +    pkt->duration = av_rescale_q(1, av_inv_q(rate), (AVRational){1,mov->movie_timescale});
>>>>      pkt->size = 4;
>>>>      AV_WB32(pkt->data, tc.start);
>>>>      ret = ff_mov_write_packet(s, pkt);
>>>> diff --git a/tests/ref/lavf/ismv b/tests/ref/lavf/ismv
>>>> index ac7f72b..723b432 100644
>>>> --- a/tests/ref/lavf/ismv
>>>> +++ b/tests/ref/lavf/ismv
>>>> @@ -1,7 +1,7 @@
>>>> -48fb8d7a5d19bd60f3a49ccf4b7d6593 *tests/data/lavf/lavf.ismv
>>>> +7a24b73c096ec0f13f0f7a2d9101c4c1 *tests/data/lavf/lavf.ismv
>>>>  313169 tests/data/lavf/lavf.ismv
>>>>  tests/data/lavf/lavf.ismv CRC=0x9d9a638a
>>>> -d19cd8e310a2e94fe0a0d11c5dc29217 *tests/data/lavf/lavf.ismv
>>>> +79646383fd099d45ad0d0c2791c601dd *tests/data/lavf/lavf.ismv
>>>>  322075 tests/data/lavf/lavf.ismv
>>>>  tests/data/lavf/lavf.ismv CRC=0xe8130120
>>>>  3b6023766845b51b075aed474c00f73c *tests/data/lavf/lavf.ismv
>>>> -- 
>>>> 1.8.3.1
>>>>
>>>
>>> will apply the patch set tomorrow unless there are any objections.
>>>
>>
>> You have not really answered whether the current files or the new files
>> are spec-incompliant; you have just reported that one byte is different.
> 
> Sorry, I think I have said both current and new file is spec-compliant in the last
> email. 
> 

You stated that you think that both files are valid, but you also said
that you don't even know what this byte that is different actually means.

> By Quicktime file format specs:
> Section Timecode Sample Description, all tmcd field isn't used pts/dts.
> 
> As for where is the different for one byte, it's caused by pkt->duration. The
> old is 0(uninitialized), after the patch it's 33(1 frame duration).  
> 

The text about Timecode Sample Description reads as follows: "Frame
duration: A 32-bit integer that indicates how long each frame lasts in
real time." This implies that only one of the two files can be
spec-compliant. I am not a mov/ISOBMFF expert, but it seems to me that
the current way of doing things is wrong. But I wonder about whether
your patch is correct for vfr content. Doesn't the property of being vfr
need to be reflected in the timecodes somehow (with different durations
for different packets)?

- Andreas
_______________________________________________
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 1/2] avformat/movenc: initialize pts/dts/duration of timecode packet

[lance.lmwang]

On Fri, Mar 11, 2022 at 05:16:09PM +0100, Andreas Rheinhardt wrote:
> lance.lmwang@gmail.com:
> > On Fri, Mar 11, 2022 at 03:04:32PM +0100, Andreas Rheinhardt wrote:
> >> lance.lmwang@gmail.com:
> >>> On Wed, Mar 02, 2022 at 09:58:31PM +0800, lance.lmwang@gmail.com wrote:
> >>>> From: Limin Wang <lance.lmwang@gmail.com>
> >>>>
> >>>> Fix below error message when timecode packet is written.
> >>>> "Application provided duration: -9223372036854775808 / timestamp: -9223372036854775808 is out of range for mov/mp4 format"
> >>>>
> >>>> try to reproduce by:
> >>>> ffmpeg -y -f lavfi -i color -metadata "timecode=00:00:00:00" -t 1 test.mov
> >>>>
> >>>> Note although error message is printed, the timecode packet will be written anyway. So
> >>>> the patch 2/2 will try to change the log level to warning.
> >>>>
> >>>> The first two test case of fate-lavf-ismv have timecode setting, so the crc of ref data is different.
> >>>> Fixes ticket #9488
> >>>>
> >>>> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> >>>> ---
> >>>>  libavformat/movenc.c | 2 ++
> >>>>  tests/ref/lavf/ismv  | 4 ++--
> >>>>  2 files changed, 4 insertions(+), 2 deletions(-)
> >>>>
> >>>> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
> >>>> index 4c86891..74b94cd 100644
> >>>> --- a/libavformat/movenc.c
> >>>> +++ b/libavformat/movenc.c
> >>>> @@ -6383,6 +6383,8 @@ static int mov_create_timecode_track(AVFormatContext *s, int index, int src_inde
> >>>>      pkt->data = data;
> >>>>      pkt->stream_index = index;
> >>>>      pkt->flags = AV_PKT_FLAG_KEY;
> >>>> +    pkt->pts = pkt->dts = av_rescale_q(tc.start, av_inv_q(rate), (AVRational){1,mov->movie_timescale});
> >>>> +    pkt->duration = av_rescale_q(1, av_inv_q(rate), (AVRational){1,mov->movie_timescale});
> >>>>      pkt->size = 4;
> >>>>      AV_WB32(pkt->data, tc.start);
> >>>>      ret = ff_mov_write_packet(s, pkt);
> >>>> diff --git a/tests/ref/lavf/ismv b/tests/ref/lavf/ismv
> >>>> index ac7f72b..723b432 100644
> >>>> --- a/tests/ref/lavf/ismv
> >>>> +++ b/tests/ref/lavf/ismv
> >>>> @@ -1,7 +1,7 @@
> >>>> -48fb8d7a5d19bd60f3a49ccf4b7d6593 *tests/data/lavf/lavf.ismv
> >>>> +7a24b73c096ec0f13f0f7a2d9101c4c1 *tests/data/lavf/lavf.ismv
> >>>>  313169 tests/data/lavf/lavf.ismv
> >>>>  tests/data/lavf/lavf.ismv CRC=0x9d9a638a
> >>>> -d19cd8e310a2e94fe0a0d11c5dc29217 *tests/data/lavf/lavf.ismv
> >>>> +79646383fd099d45ad0d0c2791c601dd *tests/data/lavf/lavf.ismv
> >>>>  322075 tests/data/lavf/lavf.ismv
> >>>>  tests/data/lavf/lavf.ismv CRC=0xe8130120
> >>>>  3b6023766845b51b075aed474c00f73c *tests/data/lavf/lavf.ismv
> >>>> -- 
> >>>> 1.8.3.1
> >>>>
> >>>
> >>> will apply the patch set tomorrow unless there are any objections.
> >>>
> >>
> >> You have not really answered whether the current files or the new files
> >> are spec-incompliant; you have just reported that one byte is different.
> > 
> > Sorry, I think I have said both current and new file is spec-compliant in the last
> > email. 
> > 
> 
> You stated that you think that both files are valid, but you also said
> that you don't even know what this byte that is different actually means.
> 
> > By Quicktime file format specs:
> > Section Timecode Sample Description, all tmcd field isn't used pts/dts.
> > 
> > As for where is the different for one byte, it's caused by pkt->duration. The
> > old is 0(uninitialized), after the patch it's 33(1 frame duration).  
> > 
> 
> The text about Timecode Sample Description reads as follows: "Frame
> duration: A 32-bit integer that indicates how long each frame lasts in
> real time." This implies that only one of the two files can be
> spec-compliant. I am not a mov/ISOBMFF expert, but it seems to me that
> the current way of doing things is wrong. But I wonder about whether
> your patch is correct for vfr content. Doesn't the property of being vfr
> need to be reflected in the timecodes somehow (with different durations
> for different packets)?

No, it's packet duration, not tmcd frame duration, my patch have do nothing 
for that.(see movenc.c:2348). In addition, for timecode, I don't think vfr is
supported.

The tmcd track just contains one packet with the frame number(4byte), so the
packet data is used by start of timecode. So I set the dts/pts is avoid the
following code think it's invalid packet. If you wonder the patch will change
something, I can update the patch keep packet duration to default zero, then
we can the fate data untouched, for the following track_duration will use it
and make the crc of output is different.


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

-- 
Thanks,
Limin Wang
_______________________________________________
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] [PATCH v2 1/2] avformat/movenc: initialize pts/dts of timecode packet

[lance.lmwang]

From: Limin Wang <lance.lmwang@gmail.com>

Fix below error message when timecode packet is written.
"Application provided duration: -9223372036854775808 / timestamp: -9223372036854775808 is out of range for mov/mp4 format"

try to reproduce by:
ffmpeg -y -f lavfi -i color -metadata "timecode=00:00:00:00" -t 1 test.mov

Note although error message is printed, the timecode packet will be written anyway. So
the patch 2/2 will try to change the log level to warning.

Fixes ticket #9488

Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
---
 libavformat/movenc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index ee1629d..436ceb8 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -6356,6 +6356,7 @@ static int mov_create_timecode_track(AVFormatContext *s, int index, int src_inde
     pkt->data = data;
     pkt->stream_index = index;
     pkt->flags = AV_PKT_FLAG_KEY;
+    pkt->pts = pkt->dts = av_rescale_q(tc.start, av_inv_q(rate), (AVRational){1,mov->movie_timescale});
     pkt->size = 4;
     AV_WB32(pkt->data, tc.start);
     ret = ff_mov_write_packet(s, pkt);
-- 
1.8.3.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] [PATCH v2 2/2] avformat/movenc: use warning log level and small adjustment for the log

[lance.lmwang]

From: Limin Wang <lance.lmwang@gmail.com>

Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
---
 libavformat/movenc.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 436ceb8..d0072f5 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -5638,9 +5638,8 @@ static int check_pkt(AVFormatContext *s, AVPacket *pkt)
 
     duration = pkt->dts - ref;
     if (pkt->dts < ref || duration >= INT_MAX) {
-        av_log(s, AV_LOG_ERROR, "Application provided duration: %"PRId64" / timestamp: %"PRId64" is out of range for mov/mp4 format\n",
-            duration, pkt->dts
-        );
+        av_log(s, AV_LOG_WARNING, "Packet duration: %"PRId64" / dts: %"PRId64" is out of range\n",
+               duration, pkt->dts);
 
         pkt->dts = ref + 1;
         pkt->pts = AV_NOPTS_VALUE;
-- 
1.8.3.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 1/2] avformat/movenc: initialize pts/dts/duration of timecode packet

[lance.lmwang]

On Fri, Mar 11, 2022 at 05:16:09PM +0100, Andreas Rheinhardt wrote:
> lance.lmwang@gmail.com:
> > On Fri, Mar 11, 2022 at 03:04:32PM +0100, Andreas Rheinhardt wrote:
> >> lance.lmwang@gmail.com:
> >>> On Wed, Mar 02, 2022 at 09:58:31PM +0800, lance.lmwang@gmail.com wrote:
> >>>> From: Limin Wang <lance.lmwang@gmail.com>
> >>>>
> >>>> Fix below error message when timecode packet is written.
> >>>> "Application provided duration: -9223372036854775808 / timestamp: -9223372036854775808 is out of range for mov/mp4 format"
> >>>>
> >>>> try to reproduce by:
> >>>> ffmpeg -y -f lavfi -i color -metadata "timecode=00:00:00:00" -t 1 test.mov
> >>>>
> >>>> Note although error message is printed, the timecode packet will be written anyway. So
> >>>> the patch 2/2 will try to change the log level to warning.
> >>>>
> >>>> The first two test case of fate-lavf-ismv have timecode setting, so the crc of ref data is different.
> >>>> Fixes ticket #9488
> >>>>
> >>>> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> >>>> ---
> >>>>  libavformat/movenc.c | 2 ++
> >>>>  tests/ref/lavf/ismv  | 4 ++--
> >>>>  2 files changed, 4 insertions(+), 2 deletions(-)
> >>>>
> >>>> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
> >>>> index 4c86891..74b94cd 100644
> >>>> --- a/libavformat/movenc.c
> >>>> +++ b/libavformat/movenc.c
> >>>> @@ -6383,6 +6383,8 @@ static int mov_create_timecode_track(AVFormatContext *s, int index, int src_inde
> >>>>      pkt->data = data;
> >>>>      pkt->stream_index = index;
> >>>>      pkt->flags = AV_PKT_FLAG_KEY;
> >>>> +    pkt->pts = pkt->dts = av_rescale_q(tc.start, av_inv_q(rate), (AVRational){1,mov->movie_timescale});
> >>>> +    pkt->duration = av_rescale_q(1, av_inv_q(rate), (AVRational){1,mov->movie_timescale});
> >>>>      pkt->size = 4;
> >>>>      AV_WB32(pkt->data, tc.start);
> >>>>      ret = ff_mov_write_packet(s, pkt);
> >>>> diff --git a/tests/ref/lavf/ismv b/tests/ref/lavf/ismv
> >>>> index ac7f72b..723b432 100644
> >>>> --- a/tests/ref/lavf/ismv
> >>>> +++ b/tests/ref/lavf/ismv
> >>>> @@ -1,7 +1,7 @@
> >>>> -48fb8d7a5d19bd60f3a49ccf4b7d6593 *tests/data/lavf/lavf.ismv
> >>>> +7a24b73c096ec0f13f0f7a2d9101c4c1 *tests/data/lavf/lavf.ismv
> >>>>  313169 tests/data/lavf/lavf.ismv
> >>>>  tests/data/lavf/lavf.ismv CRC=0x9d9a638a
> >>>> -d19cd8e310a2e94fe0a0d11c5dc29217 *tests/data/lavf/lavf.ismv
> >>>> +79646383fd099d45ad0d0c2791c601dd *tests/data/lavf/lavf.ismv
> >>>>  322075 tests/data/lavf/lavf.ismv
> >>>>  tests/data/lavf/lavf.ismv CRC=0xe8130120
> >>>>  3b6023766845b51b075aed474c00f73c *tests/data/lavf/lavf.ismv
> >>>> -- 
> >>>> 1.8.3.1
> >>>>
> >>>
> >>> will apply the patch set tomorrow unless there are any objections.
> >>>
> >>
> >> You have not really answered whether the current files or the new files
> >> are spec-incompliant; you have just reported that one byte is different.
> > 
> > Sorry, I think I have said both current and new file is spec-compliant in the last
> > email. 
> > 
> 
> You stated that you think that both files are valid, but you also said
> that you don't even know what this byte that is different actually means.
> 
> > By Quicktime file format specs:
> > Section Timecode Sample Description, all tmcd field isn't used pts/dts.
> > 
> > As for where is the different for one byte, it's caused by pkt->duration. The
> > old is 0(uninitialized), after the patch it's 33(1 frame duration).  
> > 
> 
> The text about Timecode Sample Description reads as follows: "Frame
> duration: A 32-bit integer that indicates how long each frame lasts in
> real time." This implies that only one of the two files can be
> spec-compliant. I am not a mov/ISOBMFF expert, but it seems to me that
> the current way of doing things is wrong. But I wonder about whether
> your patch is correct for vfr content. Doesn't the property of being vfr
> need to be reflected in the timecodes somehow (with different durations
> for different packets)?

Andreas, I have updated the patch and remove the fate difference which is
caused by duration, do you have any other comments for v2 patch?

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

-- 
Thanks,
Limin Wang
_______________________________________________
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 1/2] avformat/movenc: initialize pts/dts/duration of timecode packet

[Andreas Rheinhardt]

lance.lmwang@gmail.com:
> On Fri, Mar 11, 2022 at 05:16:09PM +0100, Andreas Rheinhardt wrote:
>> lance.lmwang@gmail.com:
>>> On Fri, Mar 11, 2022 at 03:04:32PM +0100, Andreas Rheinhardt wrote:
>>>> lance.lmwang@gmail.com:
>>>>> On Wed, Mar 02, 2022 at 09:58:31PM +0800, lance.lmwang@gmail.com wrote:
>>>>>> From: Limin Wang <lance.lmwang@gmail.com>
>>>>>>
>>>>>> Fix below error message when timecode packet is written.
>>>>>> "Application provided duration: -9223372036854775808 / timestamp: -9223372036854775808 is out of range for mov/mp4 format"
>>>>>>
>>>>>> try to reproduce by:
>>>>>> ffmpeg -y -f lavfi -i color -metadata "timecode=00:00:00:00" -t 1 test.mov
>>>>>>
>>>>>> Note although error message is printed, the timecode packet will be written anyway. So
>>>>>> the patch 2/2 will try to change the log level to warning.
>>>>>>
>>>>>> The first two test case of fate-lavf-ismv have timecode setting, so the crc of ref data is different.
>>>>>> Fixes ticket #9488
>>>>>>
>>>>>> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
>>>>>> ---
>>>>>>  libavformat/movenc.c | 2 ++
>>>>>>  tests/ref/lavf/ismv  | 4 ++--
>>>>>>  2 files changed, 4 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
>>>>>> index 4c86891..74b94cd 100644
>>>>>> --- a/libavformat/movenc.c
>>>>>> +++ b/libavformat/movenc.c
>>>>>> @@ -6383,6 +6383,8 @@ static int mov_create_timecode_track(AVFormatContext *s, int index, int src_inde
>>>>>>      pkt->data = data;
>>>>>>      pkt->stream_index = index;
>>>>>>      pkt->flags = AV_PKT_FLAG_KEY;
>>>>>> +    pkt->pts = pkt->dts = av_rescale_q(tc.start, av_inv_q(rate), (AVRational){1,mov->movie_timescale});
>>>>>> +    pkt->duration = av_rescale_q(1, av_inv_q(rate), (AVRational){1,mov->movie_timescale});
>>>>>>      pkt->size = 4;
>>>>>>      AV_WB32(pkt->data, tc.start);
>>>>>>      ret = ff_mov_write_packet(s, pkt);
>>>>>> diff --git a/tests/ref/lavf/ismv b/tests/ref/lavf/ismv
>>>>>> index ac7f72b..723b432 100644
>>>>>> --- a/tests/ref/lavf/ismv
>>>>>> +++ b/tests/ref/lavf/ismv
>>>>>> @@ -1,7 +1,7 @@
>>>>>> -48fb8d7a5d19bd60f3a49ccf4b7d6593 *tests/data/lavf/lavf.ismv
>>>>>> +7a24b73c096ec0f13f0f7a2d9101c4c1 *tests/data/lavf/lavf.ismv
>>>>>>  313169 tests/data/lavf/lavf.ismv
>>>>>>  tests/data/lavf/lavf.ismv CRC=0x9d9a638a
>>>>>> -d19cd8e310a2e94fe0a0d11c5dc29217 *tests/data/lavf/lavf.ismv
>>>>>> +79646383fd099d45ad0d0c2791c601dd *tests/data/lavf/lavf.ismv
>>>>>>  322075 tests/data/lavf/lavf.ismv
>>>>>>  tests/data/lavf/lavf.ismv CRC=0xe8130120
>>>>>>  3b6023766845b51b075aed474c00f73c *tests/data/lavf/lavf.ismv
>>>>>> -- 
>>>>>> 1.8.3.1
>>>>>>
>>>>>
>>>>> will apply the patch set tomorrow unless there are any objections.
>>>>>
>>>>
>>>> You have not really answered whether the current files or the new files
>>>> are spec-incompliant; you have just reported that one byte is different.
>>>
>>> Sorry, I think I have said both current and new file is spec-compliant in the last
>>> email. 
>>>
>>
>> You stated that you think that both files are valid, but you also said
>> that you don't even know what this byte that is different actually means.
>>
>>> By Quicktime file format specs:
>>> Section Timecode Sample Description, all tmcd field isn't used pts/dts.
>>>
>>> As for where is the different for one byte, it's caused by pkt->duration. The
>>> old is 0(uninitialized), after the patch it's 33(1 frame duration).  
>>>
>>
>> The text about Timecode Sample Description reads as follows: "Frame
>> duration: A 32-bit integer that indicates how long each frame lasts in
>> real time." This implies that only one of the two files can be
>> spec-compliant. I am not a mov/ISOBMFF expert, but it seems to me that
>> the current way of doing things is wrong. But I wonder about whether
>> your patch is correct for vfr content. Doesn't the property of being vfr
>> need to be reflected in the timecodes somehow (with different durations
>> for different packets)?
> 
> Andreas, I have updated the patch and remove the fate difference which is
> caused by duration, do you have any other comments for v2 patch?
> 

No.

- Andreas
_______________________________________________
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 1/2] avformat/movenc: initialize pts/dts/duration of timecode packet

[lance.lmwang]

On Tue, Mar 22, 2022 at 02:28:10PM +0100, Andreas Rheinhardt wrote:
> lance.lmwang@gmail.com:
> > On Fri, Mar 11, 2022 at 05:16:09PM +0100, Andreas Rheinhardt wrote:
> >> lance.lmwang@gmail.com:
> >>> On Fri, Mar 11, 2022 at 03:04:32PM +0100, Andreas Rheinhardt wrote:
> >>>> lance.lmwang@gmail.com:
> >>>>> On Wed, Mar 02, 2022 at 09:58:31PM +0800, lance.lmwang@gmail.com wrote:
> >>>>>> From: Limin Wang <lance.lmwang@gmail.com>
> >>>>>>
> >>>>>> Fix below error message when timecode packet is written.
> >>>>>> "Application provided duration: -9223372036854775808 / timestamp: -9223372036854775808 is out of range for mov/mp4 format"
> >>>>>>
> >>>>>> try to reproduce by:
> >>>>>> ffmpeg -y -f lavfi -i color -metadata "timecode=00:00:00:00" -t 1 test.mov
> >>>>>>
> >>>>>> Note although error message is printed, the timecode packet will be written anyway. So
> >>>>>> the patch 2/2 will try to change the log level to warning.
> >>>>>>
> >>>>>> The first two test case of fate-lavf-ismv have timecode setting, so the crc of ref data is different.
> >>>>>> Fixes ticket #9488
> >>>>>>
> >>>>>> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> >>>>>> ---
> >>>>>>  libavformat/movenc.c | 2 ++
> >>>>>>  tests/ref/lavf/ismv  | 4 ++--
> >>>>>>  2 files changed, 4 insertions(+), 2 deletions(-)
> >>>>>>
> >>>>>> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
> >>>>>> index 4c86891..74b94cd 100644
> >>>>>> --- a/libavformat/movenc.c
> >>>>>> +++ b/libavformat/movenc.c
> >>>>>> @@ -6383,6 +6383,8 @@ static int mov_create_timecode_track(AVFormatContext *s, int index, int src_inde
> >>>>>>      pkt->data = data;
> >>>>>>      pkt->stream_index = index;
> >>>>>>      pkt->flags = AV_PKT_FLAG_KEY;
> >>>>>> +    pkt->pts = pkt->dts = av_rescale_q(tc.start, av_inv_q(rate), (AVRational){1,mov->movie_timescale});
> >>>>>> +    pkt->duration = av_rescale_q(1, av_inv_q(rate), (AVRational){1,mov->movie_timescale});
> >>>>>>      pkt->size = 4;
> >>>>>>      AV_WB32(pkt->data, tc.start);
> >>>>>>      ret = ff_mov_write_packet(s, pkt);
> >>>>>> diff --git a/tests/ref/lavf/ismv b/tests/ref/lavf/ismv
> >>>>>> index ac7f72b..723b432 100644
> >>>>>> --- a/tests/ref/lavf/ismv
> >>>>>> +++ b/tests/ref/lavf/ismv
> >>>>>> @@ -1,7 +1,7 @@
> >>>>>> -48fb8d7a5d19bd60f3a49ccf4b7d6593 *tests/data/lavf/lavf.ismv
> >>>>>> +7a24b73c096ec0f13f0f7a2d9101c4c1 *tests/data/lavf/lavf.ismv
> >>>>>>  313169 tests/data/lavf/lavf.ismv
> >>>>>>  tests/data/lavf/lavf.ismv CRC=0x9d9a638a
> >>>>>> -d19cd8e310a2e94fe0a0d11c5dc29217 *tests/data/lavf/lavf.ismv
> >>>>>> +79646383fd099d45ad0d0c2791c601dd *tests/data/lavf/lavf.ismv
> >>>>>>  322075 tests/data/lavf/lavf.ismv
> >>>>>>  tests/data/lavf/lavf.ismv CRC=0xe8130120
> >>>>>>  3b6023766845b51b075aed474c00f73c *tests/data/lavf/lavf.ismv
> >>>>>> -- 
> >>>>>> 1.8.3.1
> >>>>>>
> >>>>>
> >>>>> will apply the patch set tomorrow unless there are any objections.
> >>>>>
> >>>>
> >>>> You have not really answered whether the current files or the new files
> >>>> are spec-incompliant; you have just reported that one byte is different.
> >>>
> >>> Sorry, I think I have said both current and new file is spec-compliant in the last
> >>> email. 
> >>>
> >>
> >> You stated that you think that both files are valid, but you also said
> >> that you don't even know what this byte that is different actually means.
> >>
> >>> By Quicktime file format specs:
> >>> Section Timecode Sample Description, all tmcd field isn't used pts/dts.
> >>>
> >>> As for where is the different for one byte, it's caused by pkt->duration. The
> >>> old is 0(uninitialized), after the patch it's 33(1 frame duration).  
> >>>
> >>
> >> The text about Timecode Sample Description reads as follows: "Frame
> >> duration: A 32-bit integer that indicates how long each frame lasts in
> >> real time." This implies that only one of the two files can be
> >> spec-compliant. I am not a mov/ISOBMFF expert, but it seems to me that
> >> the current way of doing things is wrong. But I wonder about whether
> >> your patch is correct for vfr content. Doesn't the property of being vfr
> >> need to be reflected in the timecodes somehow (with different durations
> >> for different packets)?
> > 
> > Andreas, I have updated the patch and remove the fate difference which is
> > caused by duration, do you have any other comments for v2 patch?
> > 
> 
> No.

Thanks, then will apply the v2 patchsetet.

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

-- 
Thanks,
Limin Wang
_______________________________________________
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".