| Message ID | 1646229512-12103-1-git-send-email-lance.lmwang@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/2] avformat/movenc: initialize pts/dts/duration of timecode packet | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
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
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".
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.
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
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".
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
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".
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".
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
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".
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