From: "Dai, Jianhui J" <jianhui.j.dai-at-intel.com@ffmpeg.org>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH v5] avformat/ivfenc: Set the "number of frames" in IVF header
Date: Fri, 7 Jul 2023 01:09:51 +0000
Message-ID: <DM6PR11MB2681777E3FF7138F12390F7BB12DA@DM6PR11MB2681.namprd11.prod.outlook.com> (raw)
In-Reply-To: <DM6PR11MB2681B1579D4FE88C28AC7488B129A@DM6PR11MB2681.namprd11.prod.outlook.com>
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Dai,
> Jianhui J
> Sent: Monday, July 3, 2023 12:26 PM
> To: ffmpeg-devel@ffmpeg.org
> Subject: [FFmpeg-devel] [PATCH v5] avformat/ivfenc: Set the "number of
> frames" in IVF header
>
> 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@intel.com>
> ---
> libavformat/ivfdec.c | 13 ++++++++++---
> 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, 28 insertions(+), 24 deletions(-)
>
> diff --git a/libavformat/ivfdec.c b/libavformat/ivfdec.c index
> 511f2387ed..141ce4f1be 100644
> --- a/libavformat/ivfdec.c
> +++ b/libavformat/ivfdec.c
> @@ -51,11 +51,18 @@ 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
>
> + // Infer duration from nb_frames, in order to be backward compatible
> with
> + // previous IVF demuxer.
> + // It is popular to configure time_base to 1/frame_rate by IVF muxer, that
> + // the duration happens to be the same with nb_frames. See
> + // `https://chromium.googlesource.com/webm/vp8-test-
> vectors/+/refs/heads/main`
> + st->duration = st->nb_frames;
> +
> ffstream(st)->need_parsing = AVSTREAM_PARSE_HEADERS;
>
> if (!time_base.den || !time_base.num) { diff --git a/libavformat/ivfenc.c
> b/libavformat/ivfenc.c index 47b4efbcd1..88399099d4 100644
> --- a/libavformat/ivfenc.c
> +++ b/libavformat/ivfenc.c
> @@ -72,7 +72,8 @@ static int ivf_write_header(AVFormatContext *s)
> avio_wl16(pb, par->height);
> avio_wl32(pb, s->streams[0]->time_base.den);
> avio_wl32(pb, s->streams[0]->time_base.num);
> - avio_wl64(pb, 0xFFFFFFFFFFFFFFFFULL); // length is overwritten at the end
> of muxing
> + avio_wl32(pb, 0xFFFFFFFF); // "number of frames" is overwritten at the
> end of muxing
> + avio_wl32(pb, 0); // unused
>
> return 0;
> }
> @@ -99,16 +100,12 @@ static int ivf_write_trailer(AVFormatContext *s)
> AVIOContext *pb = s->pb;
> IVFEncContext *ctx = s->priv_data;
>
> - if ((pb->seekable & AVIO_SEEKABLE_NORMAL) &&
> - (ctx->frame_cnt > 1 || (ctx->frame_cnt == 1 && ctx->last_pkt_duration)))
> {
> + // overwrite the "number of frames"
> + if ((pb->seekable & AVIO_SEEKABLE_NORMAL)) {
> int64_t end = avio_tell(pb);
>
> avio_seek(pb, 24, SEEK_SET);
> - // overwrite the "length" field (duration)
> - avio_wl32(pb, ctx->last_pkt_duration ?
> - ctx->sum_delta_pts + ctx->last_pkt_duration :
> - ctx->frame_cnt * ctx->sum_delta_pts / (ctx->frame_cnt - 1));
> - avio_wl32(pb, 0); // zero out unused bytes
> + avio_wl32(pb, ctx->frame_cnt);
> avio_seek(pb, end, SEEK_SET);
> }
>
> diff --git a/tests/ref/fate/cbs-vp9-vp90-2-03-deltaq b/tests/ref/fate/cbs-vp9-
> vp90-2-03-deltaq
> index db09cfd5e0..f621d7a480 100644
> --- a/tests/ref/fate/cbs-vp9-vp90-2-03-deltaq
> +++ b/tests/ref/fate/cbs-vp9-vp90-2-03-deltaq
> @@ -1 +1 @@
> -bb630ef560f83951fa6547a664fdb636
> +fe62460fe28202e0666e628afd8602ca
> diff --git a/tests/ref/fate/cbs-vp9-vp90-2-06-bilinear b/tests/ref/fate/cbs-
> vp9-vp90-2-06-bilinear
> index f579459179..9359e21e40 100644
> --- a/tests/ref/fate/cbs-vp9-vp90-2-06-bilinear
> +++ b/tests/ref/fate/cbs-vp9-vp90-2-06-bilinear
> @@ -1 +1 @@
> -2ca9d012c7212e38f5e2727ac66ec6c5
> +179e228004c396a301c89f34b6c72f68
> diff --git a/tests/ref/fate/cbs-vp9-vp90-2-09-lf_deltas b/tests/ref/fate/cbs-
> vp9-vp90-2-09-lf_deltas
> index e0b5686d0b..5b21675c76 100644
> --- a/tests/ref/fate/cbs-vp9-vp90-2-09-lf_deltas
> +++ b/tests/ref/fate/cbs-vp9-vp90-2-09-lf_deltas
> @@ -1 +1 @@
> -78f5e46bfaecbcd62b9126697a0d97b7
> +1d1f0768c547461ae2abef57f0aabc24
> diff --git a/tests/ref/fate/cbs-vp9-vp90-2-10-show-existing-frame
> b/tests/ref/fate/cbs-vp9-vp90-2-10-show-existing-frame
> index 4a4d752428..19b7a78dd8 100644
> --- a/tests/ref/fate/cbs-vp9-vp90-2-10-show-existing-frame
> +++ b/tests/ref/fate/cbs-vp9-vp90-2-10-show-existing-frame
> @@ -1 +1 @@
> -eea9d10a696c6ed971e4fae9fb619b10
> +13fa042ee1b4079c227a5c5c96e2db38
> diff --git a/tests/ref/fate/cbs-vp9-vp90-2-10-show-existing-frame2
> b/tests/ref/fate/cbs-vp9-vp90-2-10-show-existing-frame2
> index 6da8999114..e7bf2a078d 100644
> --- a/tests/ref/fate/cbs-vp9-vp90-2-10-show-existing-frame2
> +++ b/tests/ref/fate/cbs-vp9-vp90-2-10-show-existing-frame2
> @@ -1 +1 @@
> -abf4c7d4be7d3576d96b6f92166b5894
> +2ab7c95e4637fb6a15efd8c0a8d6af98
> diff --git a/tests/ref/fate/cbs-vp9-vp90-2-segmentation-aq-akiyo
> b/tests/ref/fate/cbs-vp9-vp90-2-segmentation-aq-akiyo
> index 12dfb10d40..f30889dbdc 100644
> --- a/tests/ref/fate/cbs-vp9-vp90-2-segmentation-aq-akiyo
> +++ b/tests/ref/fate/cbs-vp9-vp90-2-segmentation-aq-akiyo
> @@ -1 +1 @@
> -86cd3750cc9a0672717643c9b9f87fd5
> +b5be66a6a8792f7aac090beb9f3b4555
> diff --git a/tests/ref/fate/cbs-vp9-vp90-2-segmentation-sf-akiyo
> b/tests/ref/fate/cbs-vp9-vp90-2-segmentation-sf-akiyo
> index c2b1b8723a..dca77f2113 100644
> --- a/tests/ref/fate/cbs-vp9-vp90-2-segmentation-sf-akiyo
> +++ b/tests/ref/fate/cbs-vp9-vp90-2-segmentation-sf-akiyo
> @@ -1 +1 @@
> -5d12fbe6220aae9e62b1d79785a83387
> +7bde6532fc682bfa3f5170cf9d607865
> diff --git a/tests/ref/fate/cbs-vp9-vp90-2-tiling-pedestrian
> b/tests/ref/fate/cbs-vp9-vp90-2-tiling-pedestrian
> index f9cab39bd6..0aa3cc8ce6 100644
> --- a/tests/ref/fate/cbs-vp9-vp90-2-tiling-pedestrian
> +++ b/tests/ref/fate/cbs-vp9-vp90-2-tiling-pedestrian
> @@ -1 +1 @@
> -4c51f3c796baa7c2baa4b7ec0d011406
> +1e40e8b48e4682e8b8004b9e0e60a5b6
> diff --git a/tests/ref/fate/cbs-vp9-vp91-2-04-yuv440 b/tests/ref/fate/cbs-vp9-
> vp91-2-04-yuv440
> index 6289930070..947e1229eb 100644
> --- a/tests/ref/fate/cbs-vp9-vp91-2-04-yuv440
> +++ b/tests/ref/fate/cbs-vp9-vp91-2-04-yuv440
> @@ -1 +1 @@
> -293bdc92851ca1105e27f04737d8c5f3
> +9bb416c0304a13c4f66c56aef8431cd4
> diff --git a/tests/ref/fate/cbs-vp9-vp91-2-04-yuv444 b/tests/ref/fate/cbs-vp9-
> vp91-2-04-yuv444
> index 628ea9a4d9..bf251138ed 100644
> --- a/tests/ref/fate/cbs-vp9-vp91-2-04-yuv444
> +++ b/tests/ref/fate/cbs-vp9-vp91-2-04-yuv444
> @@ -1 +1 @@
> -911eafd8e442e646c5ce97d781757ca8
> +3a7ed001d30f96d4888f5ca16e6263ce
> diff --git a/tests/ref/fate/cbs-vp9-vp92-2-20-10bit-yuv420
> b/tests/ref/fate/cbs-vp9-vp92-2-20-10bit-yuv420
> index eeb7580d74..2cad8b947c 100644
> --- a/tests/ref/fate/cbs-vp9-vp92-2-20-10bit-yuv420
> +++ b/tests/ref/fate/cbs-vp9-vp92-2-20-10bit-yuv420
> @@ -1 +1 @@
> -16198c32c29228e0513004ed1bf6fcee
> +7315bb7b55693a87c350b48cd2ee9811
> diff --git a/tests/ref/fate/cbs-vp9-vp93-2-20-10bit-yuv422
> b/tests/ref/fate/cbs-vp9-vp93-2-20-10bit-yuv422
> index b25bc1166e..bb1c0f7ea7 100644
> --- a/tests/ref/fate/cbs-vp9-vp93-2-20-10bit-yuv422
> +++ b/tests/ref/fate/cbs-vp9-vp93-2-20-10bit-yuv422
> @@ -1 +1 @@
> -4bceedef4aa6a663a09761971e43b5a8
> +1a7b5bf86bf0bbef10c9a1b2c799b276
> diff --git a/tests/ref/fate/cbs-vp9-vp93-2-20-12bit-yuv444
> b/tests/ref/fate/cbs-vp9-vp93-2-20-12bit-yuv444
> index 8d122d1370..9b7b358d04 100644
> --- a/tests/ref/fate/cbs-vp9-vp93-2-20-12bit-yuv444
> +++ b/tests/ref/fate/cbs-vp9-vp93-2-20-12bit-yuv444
> @@ -1 +1 @@
> -0f413b840633bfcfcc78b4c9fab933bf
> +9b7a0b7fc081542d9be1074b23054861
> --
> 2.25.1
Another friendly ping reviewers to apply.
Some decoders will strictly check this field in IVF header, and it causes failures for those decoders.
>
> _______________________________________________
> 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".
_______________________________________________
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".
next prev parent reply other threads:[~2023-07-07 1:10 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-07-03 4:25 Dai, Jianhui J
2023-07-03 4:30 ` Dai, Jianhui J
2023-07-03 8:48 ` mypopy
2023-07-07 1:09 ` Dai, Jianhui J [this message]
2023-07-09 21:07 ` Ronald S. Bultje
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DM6PR11MB2681777E3FF7138F12390F7BB12DA@DM6PR11MB2681.namprd11.prod.outlook.com \
--to=jianhui.j.dai-at-intel.com@ffmpeg.org \
--cc=ffmpeg-devel@ffmpeg.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
This inbox may be cloned and mirrored by anyone:
git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git
# If you have public-inbox 1.1+ installed, you may
# initialize and index your mirror using the following commands:
public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
ffmpegdev@gitmailbox.com
public-inbox-index ffmpegdev
Example config snippet for mirrors.
AGPL code for this site: git clone https://public-inbox.org/public-inbox.git