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

[lance.lmwang at gmail.com]

On Thu, Mar 03, 2022 at 02:55:23AM +0100, Andreas Rheinhardt wrote:
> lance.lmwang at gmail.com:
> > From: Limin Wang <lance.lmwang at 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 at 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 at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".

-- 
Thanks,
Limin Wang