| Message ID | DM6PR11MB2681A53C0378EE000FE92DB2B125A@DM6PR11MB2681.namprd11.prod.outlook.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,v3] avformat/ivfenc: Set the "number of frames" in IVF header | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
Hi, On Thu, Jun 29, 2023 at 8:54 AM Dai, Jianhui J < jianhui.j.dai-at-intel.com@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@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
> -----Original Message----- > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > Ronald S. Bultje > Sent: Friday, June 30, 2023 1:27 AM > To: FFmpeg development discussions and patches <ffmpeg- > devel@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@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@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@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org > with subject "unsubscribe".
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; 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
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 | 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(-)