diff mbox series

[FFmpeg-devel] libsvt_av1: remove forced-idr option

Message ID MDRK84g--3-2@lynne.ee
State Accepted
Commit 6ceaeecaf562020f25beab0614157fb2dae32391
Headers show
Series [FFmpeg-devel] libsvt_av1: remove forced-idr option | expand

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Lynne July 29, 2020, 8:47 p.m. UTC 
This option is directly copy-pasted from the SVT1-HEVC wrapper and has 
no place in the options for an AV1 encoder.

AV1 has no H.264/5 IDR frames nor anything like them.
All this option does is change all real keyframes to an intra-only
AV1 frame, which is not seekable. Hence, any streams encoded with
this option enabled will not be seekable.

Having an option that breaks encoded streams this much is not what
we'd like. If SVT-AV1 developers would like to have an open-gop
option a patch to enable that could be added with an appropriate
name and after a discussion, rather than slipping through reviews.
Subject: [PATCH] libsvt_av1: remove forced-idr option

This option is directly copy-pasted from the SVT1-HEVC wrapper and has
no place in the options for an AV1 encoder.

AV1 has no H.264/5 IDR frames nor anything like them.
All this option does is change all real keyframes to an intra-only
AV1 frame, which is not seekable. Hence, any streams encoded with
this option enabled will not be seekable.

Having an option that breaks encoded streams this much is not what
we'd like. If SVT-AV1 developers would like to have an open-gop
option a patch to enable that could be added with an appropriate
name and after a discussion, rather than slipping through reviews.
---
 libavcodec/libsvt_av1.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

Comments

James Almer July 29, 2020, 8:58 p.m. UTC | #1 
On 7/29/2020 5:47 PM, Lynne wrote:
> This option is directly copy-pasted from the SVT1-HEVC wrapper and has 
> no place in the options for an AV1 encoder.
> 
> AV1 has no H.264/5 IDR frames nor anything like them.
> All this option does is change all real keyframes to an intra-only
> AV1 frame, which is not seekable. Hence, any streams encoded with
> this option enabled will not be seekable.

Can confirm it creates streams with a single key-frame at the beginning.

> 
> Having an option that breaks encoded streams this much is not what
> we'd like. If SVT-AV1 developers would like to have an open-gop
> option a patch to enable that could be added with an appropriate
> name and after a discussion, rather than slipping through reviews.

Will apply.
diff mbox series

Patch

diff --git a/libavcodec/libsvt_av1.c b/libavcodec/libsvt_av1.c
index b30211d15b..c7ae5f9691 100644
--- a/libavcodec/libsvt_av1.c
+++ b/libavcodec/libsvt_av1.c
@@ -67,8 +67,6 @@  typedef struct SvtContext {
     int scd;
     int qp;
 
-    int forced_idr;
-
     int tier;
 
     int tile_columns;
@@ -212,7 +210,7 @@  static int config_enc_params(EbSvtAv1EncConfiguration *param,
         param->min_qp_allowed       = avctx->qmin;
     }
 
-    param->intra_refresh_type       = svt_enc->forced_idr + 1;
+    param->intra_refresh_type       = 2; /* Real keyframes only */
 
     if (svt_enc->la_depth >= 0)
         param->look_ahead_distance  = svt_enc->la_depth;
@@ -538,9 +536,6 @@  static const AVOption options[] = {
     { "sc_detection", "Scene change detection", OFFSET(scd),
       AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VE },
 
-    { "forced-idr", "If forcing keyframes, force them as IDR frames", OFFSET(forced_idr),
-      AV_OPT_TYPE_BOOL,   { .i64 = 1 }, 0, 1, VE },
-
     { "tile-columns", "Log2 of number of tile columns to use", OFFSET(tile_columns), AV_OPT_TYPE_INT, {.i64 = 0}, 0, 4, VE},
     { "tile-rows", "Log2 of number of tile rows to use", OFFSET(tile_rows), AV_OPT_TYPE_INT, {.i64 = 0}, 0, 6, VE},