Re: [FFmpeg-devel] [PATCH 1/2] avformat/movenc: initialize pts/dts/duration of timecode packet
On Tue, Mar 22, 2022 at 02:28:10PM +0100, Andreas Rheinhardt wrote:
> lance.lmw...@gmail.com:
> > On Fri, Mar 11, 2022 at 05:16:09PM +0100, Andreas Rheinhardt wrote:
> >> lance.lmw...@gmail.com:
> >>> On Fri, Mar 11, 2022 at 03:04:32PM +0100, Andreas Rheinhardt wrote:
> lance.lmw...@gmail.com:
> > On Wed, Mar 02, 2022 at 09:58:31PM +0800, lance.lmw...@gmail.com wrote:
> >> From: Limin Wang
> >>
> >> 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
> >> ---
> >> 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-requ...@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-requ...@ffmpeg.org with subject "un
Re: [FFmpeg-devel] [PATCH 1/2] avformat/movenc: initialize pts/dts/duration of timecode packet
lance.lmw...@gmail.com:
> On Fri, Mar 11, 2022 at 05:16:09PM +0100, Andreas Rheinhardt wrote:
>> lance.lmw...@gmail.com:
>>> On Fri, Mar 11, 2022 at 03:04:32PM +0100, Andreas Rheinhardt wrote:
lance.lmw...@gmail.com:
> On Wed, Mar 02, 2022 at 09:58:31PM +0800, lance.lmw...@gmail.com wrote:
>> From: Limin Wang
>>
>> 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
>> ---
>> 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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/2] avformat/movenc: initialize pts/dts/duration of timecode packet
On Fri, Mar 11, 2022 at 05:16:09PM +0100, Andreas Rheinhardt wrote:
> lance.lmw...@gmail.com:
> > On Fri, Mar 11, 2022 at 03:04:32PM +0100, Andreas Rheinhardt wrote:
> >> lance.lmw...@gmail.com:
> >>> On Wed, Mar 02, 2022 at 09:58:31PM +0800, lance.lmw...@gmail.com wrote:
> From: Limin Wang
>
> 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
> ---
> 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-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/2] avformat/movenc: initialize pts/dts/duration of timecode packet
On Fri, Mar 11, 2022 at 05:16:09PM +0100, Andreas Rheinhardt wrote:
> lance.lmw...@gmail.com:
> > On Fri, Mar 11, 2022 at 03:04:32PM +0100, Andreas Rheinhardt wrote:
> >> lance.lmw...@gmail.com:
> >>> On Wed, Mar 02, 2022 at 09:58:31PM +0800, lance.lmw...@gmail.com wrote:
> From: Limin Wang
>
> 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
> ---
> 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-requ...@ffmpeg.org with subject "unsubscribe".
--
Thanks,
Limin Wang
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-de
Re: [FFmpeg-devel] [PATCH 1/2] avformat/movenc: initialize pts/dts/duration of timecode packet
lance.lmw...@gmail.com:
> On Fri, Mar 11, 2022 at 03:04:32PM +0100, Andreas Rheinhardt wrote:
>> lance.lmw...@gmail.com:
>>> On Wed, Mar 02, 2022 at 09:58:31PM +0800, lance.lmw...@gmail.com wrote:
From: Limin Wang
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
---
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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/2] avformat/movenc: initialize pts/dts/duration of timecode packet
On Fri, Mar 11, 2022 at 03:04:32PM +0100, Andreas Rheinhardt wrote:
> lance.lmw...@gmail.com:
> > On Wed, Mar 02, 2022 at 09:58:31PM +0800, lance.lmw...@gmail.com wrote:
> >> From: Limin Wang
> >>
> >> 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
> >> ---
> >> 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-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/2] avformat/movenc: initialize pts/dts/duration of timecode packet
lance.lmw...@gmail.com:
> On Wed, Mar 02, 2022 at 09:58:31PM +0800, lance.lmw...@gmail.com wrote:
>> From: Limin Wang
>>
>> 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
>> ---
>> 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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/2] avformat/movenc: initialize pts/dts/duration of timecode packet
On Wed, Mar 02, 2022 at 09:58:31PM +0800, lance.lmw...@gmail.com wrote:
> From: Limin Wang
>
> 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
> ---
> 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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/2] avformat/movenc: initialize pts/dts/duration of timecode packet
On Thu, Mar 03, 2022 at 02:55:23AM +0100, Andreas Rheinhardt wrote:
> lance.lmw...@gmail.com:
> > From: Limin Wang
> >
> > 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
> > ---
> > 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 6d0c 6164 0074 0804
---
> 00230d0 0028 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-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/2] avformat/movenc: initialize pts/dts/duration of timecode packet
lance.lmw...@gmail.com:
> From: Limin Wang
>
> 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
> ---
> 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 1/2] avformat/movenc: initialize pts/dts/duration of timecode packet
From: Limin Wang
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
---
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-requ...@ffmpeg.org with subject "unsubscribe".
