[FFmpeg-devel] [PATCH v3] avformat/ivfenc: Set the "number of frames" in IVF header

[Dai, Jianhui J]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> Ronald S. Bultje
> Sent: Friday, June 30, 2023 1:27 AM
> To: FFmpeg development discussions and patches <ffmpeg-
> devel at ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH v3] avformat/ivfenc: Set the "number of
> frames" in IVF header
> 
> Hi,
> 
> On Thu, Jun 29, 2023 at 8:54 AM Dai, Jianhui J < jianhui.j.dai-at-
> intel.com at ffmpeg.org> wrote:
> 
> > Should set "number of frames" to bytes 24-27 of IVF header, not
> > duration.
> > It is described by [1], and confirmed by parsing all IVF files in [2].
> >
> > This commit also updates the md5sum of refs to pass fate-cbs.
> >
> > [1] Duck IVF - MultimediaWiki
> > https://wiki.multimedia.cx/index.php/Duck_IVF
> >
> > [2] webm/vp8-test-vectors
> > https://chromium.googlesource.com/webm/vp8-test-vectors
> >
> > Signed-off-by: Jianhui Dai <jianhui.j.dai at intel.com>
> > ---
> >  libavformat/ivfdec.c                                |  6 +++---
> >  libavformat/ivfenc.c                                | 13 +++++--------
> >  tests/ref/fate/cbs-vp9-vp90-2-03-deltaq             |  2 +-
> >  tests/ref/fate/cbs-vp9-vp90-2-06-bilinear           |  2 +-
> >  tests/ref/fate/cbs-vp9-vp90-2-09-lf_deltas          |  2 +-
> >  .../ref/fate/cbs-vp9-vp90-2-10-show-existing-frame  |  2 +-
> >  .../ref/fate/cbs-vp9-vp90-2-10-show-existing-frame2 |  2 +-
> > tests/ref/fate/cbs-vp9-vp90-2-segmentation-aq-akiyo |  2 +-
> > tests/ref/fate/cbs-vp9-vp90-2-segmentation-sf-akiyo |  2 +-
> >  tests/ref/fate/cbs-vp9-vp90-2-tiling-pedestrian     |  2 +-
> >  tests/ref/fate/cbs-vp9-vp91-2-04-yuv440             |  2 +-
> >  tests/ref/fate/cbs-vp9-vp91-2-04-yuv444             |  2 +-
> >  tests/ref/fate/cbs-vp9-vp92-2-20-10bit-yuv420       |  2 +-
> >  tests/ref/fate/cbs-vp9-vp93-2-20-10bit-yuv422       |  2 +-
> >  tests/ref/fate/cbs-vp9-vp93-2-20-12bit-yuv444       |  2 +-
> >  15 files changed, 21 insertions(+), 24 deletions(-)
> >
> > diff --git a/libavformat/ivfdec.c b/libavformat/ivfdec.c index
> > 511f2387ed..c71a28e0d3 100644
> > --- a/libavformat/ivfdec.c
> > +++ b/libavformat/ivfdec.c
> > @@ -51,9 +51,9 @@ static int read_header(AVFormatContext *s)
> >      st->codecpar->codec_id   = ff_codec_get_id(ff_codec_bmp_tags,
> > st->codecpar->codec_tag);
> >      st->codecpar->width      = avio_rl16(s->pb);
> >      st->codecpar->height     = avio_rl16(s->pb);
> > -    time_base.den         = avio_rl32(s->pb);
> > -    time_base.num         = avio_rl32(s->pb);
> > -    st->duration          = avio_rl32(s->pb);
> > +    time_base.den            = avio_rl32(s->pb);
> > +    time_base.num            = avio_rl32(s->pb);
> > +    st->nb_frames            = avio_rl32(s->pb);
> >      avio_skip(s->pb, 4); // unused
> >
> >      ffstream(st)->need_parsing = AVSTREAM_PARSE_HEADERS;
> >
> 
> Is the removal of the st->duration assignment necessary? Applications using
> this field will now see a regression.

Thanks.
It's good for me to set both st->nb_frames and st->duration.
The accuracy of `duration` cannot be guaranteed. 
Luckily, the duration is often correct, because it is popular configure that time_base.den/time_base.num == fps.

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