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

[Ronald S. Bultje]

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.

Ronald