Re: [FFmpeg-devel] [PATCH] avformat/movenc: Check that cts fits in 32bit

[Michael Niedermayer <michael@niedermayer.cc>]

[-- Attachment #1.1: Type: text/plain, Size: 2458 bytes --]

On Mon, Apr 01, 2024 at 12:45:36PM -0300, James Almer wrote:
> On 4/1/2024 12:38 PM, Michael Niedermayer wrote:
> > On Sun, Mar 31, 2024 at 01:30:26PM -0300, James Almer wrote:
> > > On 3/31/2024 8:40 AM, Michael Niedermayer wrote:
> > > > Fixes: Assertion av_rescale_rnd(start_dts, mov->movie_timescale, track->timescale, AV_ROUND_DOWN) <= 0 failed at libavformat/movenc.c:3694
> > > > Fixes: poc2
> > > > 
> > > > Found-by: Wang Dawei and Zhou Geng, from Zhongguancun Laboratory
> > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > > > ---
> > > >    libavformat/movenc.c | 6 ++++++
> > > >    1 file changed, 6 insertions(+)
> > > > 
> > > > diff --git a/libavformat/movenc.c b/libavformat/movenc.c
> > > > index ae94d8d5959..5617a2620c5 100644
> > > > --- a/libavformat/movenc.c
> > > > +++ b/libavformat/movenc.c
> > > > @@ -6194,6 +6194,12 @@ int ff_mov_write_packet(AVFormatContext *s, AVPacket *pkt)
> > > >        if (ret < 0)
> > > >            return ret;
> > > > +    if (pkt->pts != AV_NOPTS_VALUE &&
> > > > +        (uint64_t)pkt->dts - pkt->pts != (int32_t)((uint64_t)pkt->dts - pkt->pts)) {
> > > > +        av_log(s, AV_LOG_WARNING, "pts/dts pair unsupported\n");
> > > > +        return AVERROR_PATCHWELCOME;
> > > > +    }
> > > 
> > > Any such check should happen in check_pkt(), called directly above. And
> > > afaict there's no reason to not support 64bit cts. Even in
> > > mov_write_edts_tag() we check for it and write a version 1 of the box that
> > > supports 64bit values.
> > > 
> > > Maybe the problem is that MOVIentry.cts is an int, when it should be an
> > > int64_t like start_cts? Can you test the following?
> > 
> > changing cts to 64bit does avoid the assert with the test sample but
> > 
> > If you chaneg cts to 64bit consider cts is assigned to MOVCtts duration (32bit)
> > in mov_write_ctts_tag() and also compared to it.
> > its also written with avio_wb32() later
> > 
> > theres also
> > avio_wb32(pb, track->cluster[i].cts);
> > in mov_write_trun_tag()
> 
> You're right, there's no 64bit version for these, i guess they will not
> define one either in a future revision of the spec. So your patch should be
> fine.

ok, will apply

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

While the State exists there can be no freedom; when there is freedom there
will be no State. -- Vladimir Lenin

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

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