diff mbox series

[FFmpeg-devel] avcodec/libsvtav1: don't force a default value for deprecated options

Message ID 20221129213636.1504-1-jamrial@gmail.com
State New
Headers show
Series [FFmpeg-devel] avcodec/libsvtav1: don't force a default value for deprecated options | expand

Checks

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

Commit Message

James Almer Nov. 29, 2022, 9:36 p.m. UTC
Stick to the library's default value instead.

Should address AOMediaCodec/SVT-AV1 issue #2011.

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavcodec/libsvtav1.c | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

Comments

Christopher Degawa Dec. 2, 2022, 11:51 p.m. UTC | #1
On Tue, Nov 29, 2022 at 4:38 PM James Almer <jamrial@gmail.com> wrote:

> Stick to the library's default value instead.
>
> Should address AOMediaCodec/SVT-AV1 issue #2011.
>
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  libavcodec/libsvtav1.c | 25 +++++++++++++++----------
>  1 file changed, 15 insertions(+), 10 deletions(-)
>

Ping on this? It would be great since this would allow the library to apply
better defaults for the deprecated options.
James Almer Dec. 3, 2022, 12:31 a.m. UTC | #2
On 12/2/2022 8:51 PM, Christopher Degawa wrote:
> On Tue, Nov 29, 2022 at 4:38 PM James Almer <jamrial@gmail.com> wrote:
> 
>> Stick to the library's default value instead.
>>
>> Should address AOMediaCodec/SVT-AV1 issue #2011.
>>
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>>   libavcodec/libsvtav1.c | 25 +++++++++++++++----------
>>   1 file changed, 15 insertions(+), 10 deletions(-)
>>
> 
> Ping on this? It would be great since this would allow the library to apply
> better defaults for the deprecated options.

Applied.
Guo, Yejun Dec. 3, 2022, 9:32 a.m. UTC | #3
Hi,

There are discussions about dnn module at https://etherpad.mit.edu/p/FF_dev_meeting_20221202.

1) Regarding "Delete the native backend? Yes",
I agree since it is performance sensitive, and I can take this effort. 

And want to get advice on how to delete it. Usually, we'd mark it as deprecated, and then delete after a long time. As for this case, my idea is:
Step1: delete code of native backend under libavfilter/dnn, and also add error message in filters to let end user get a clear message that it is not supported.
Step2:  after a long time (maybe next major release), delete the 'error message' (all code relative to native backend) in filters.

2) Regarding "Move to a separate library? Delete? Move to somewhere else?",
@Pedro Arthur  any comment?

And I have two other opens about the backend.

3) Adding libtorch as a new backend after native backend is deleted.
Deep learning models are usually developed on different frameworks and so the model files are in different format. The current most popular framework is probably pytorch and we see lots of new model files in pytorch format. My idea is to embrace libtorch as a new backend, and @Fu, Ting has finished the code and is willing to upstream it and also adding a new feature in vf_dnn_processing.c with basic VSR (video super resolution) whose model file is now only available in pytorch format.

4) how about many other deep learning frameworks?
There are also many other promising dnn inference frameworks, and how do we support them? One method is that they add a glue layer to implement the interface in libavfilter/dnn_interface.h, and we (ffmpeg) can dlopen the glue layer library. Actually, the implementation can be another framework, or even be a service. Anyway, this can be a long term solution, and can be discussed in more detail when this requirement appears.

Thanks
Yejun
diff mbox series

Patch

diff --git a/libavcodec/libsvtav1.c b/libavcodec/libsvtav1.c
index 7605baddfe..0742c4e94c 100644
--- a/libavcodec/libsvtav1.c
+++ b/libavcodec/libsvtav1.c
@@ -155,11 +155,16 @@  static int config_enc_params(EbSvtAv1EncConfiguration *param,
 
     // Update param from options
 #if FF_API_SVTAV1_OPTS
-    param->hierarchical_levels      = svt_enc->hierarchical_level;
-    param->tier                     = svt_enc->tier;
-    param->scene_change_detection   = svt_enc->scd;
-    param->tile_columns             = svt_enc->tile_columns;
-    param->tile_rows                = svt_enc->tile_rows;
+    if (svt_enc->hierarchical_level >= 0)
+        param->hierarchical_levels  = svt_enc->hierarchical_level;
+    if (svt_enc->tier >= 0)
+        param->tier                 = svt_enc->tier;
+    if (svt_enc->scd >= 0)
+        param->scene_change_detection = svt_enc->scd;
+    if (svt_enc->tile_columns >= 0)
+        param->tile_columns         = svt_enc->tile_columns;
+    if (svt_enc->tile_rows >= 0)
+        param->tile_rows            = svt_enc->tile_rows;
 
     if (svt_enc->la_depth >= 0)
         param->look_ahead_distance  = svt_enc->la_depth;
@@ -572,7 +577,7 @@  static av_cold int eb_enc_close(AVCodecContext *avctx)
 static const AVOption options[] = {
 #if FF_API_SVTAV1_OPTS
     { "hielevel", "Hierarchical prediction levels setting (Deprecated, use svtav1-params)", OFFSET(hierarchical_level),
-      AV_OPT_TYPE_INT, { .i64 = 4 }, 3, 4, VE | AV_OPT_FLAG_DEPRECATED , "hielevel"},
+      AV_OPT_TYPE_INT, { .i64 = -1 }, -1, 4, VE | AV_OPT_FLAG_DEPRECATED , "hielevel"},
         { "3level", NULL, 0, AV_OPT_TYPE_CONST, { .i64 = 3 },  INT_MIN, INT_MAX, VE, "hielevel" },
         { "4level", NULL, 0, AV_OPT_TYPE_CONST, { .i64 = 4 },  INT_MIN, INT_MAX, VE, "hielevel" },
 
@@ -580,7 +585,7 @@  static const AVOption options[] = {
       AV_OPT_TYPE_INT, { .i64 = -1 }, -1, 120, VE | AV_OPT_FLAG_DEPRECATED },
 
     { "tier", "Set operating point tier (Deprecated, use svtav1-params)", OFFSET(tier),
-      AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 1, VE | AV_OPT_FLAG_DEPRECATED, "tier" },
+      AV_OPT_TYPE_INT, { .i64 = -1 }, -1, 1, VE | AV_OPT_FLAG_DEPRECATED, "tier" },
         { "main", NULL, 0, AV_OPT_TYPE_CONST, { .i64 = 0 }, 0, 0, VE, "tier" },
         { "high", NULL, 0, AV_OPT_TYPE_CONST, { .i64 = 1 }, 0, 0, VE, "tier" },
 #endif
@@ -623,10 +628,10 @@  static const AVOption options[] = {
       AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 63, VE },
 #if FF_API_SVTAV1_OPTS
     { "sc_detection", "Scene change detection (Deprecated, use svtav1-params)", OFFSET(scd),
-      AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VE | AV_OPT_FLAG_DEPRECATED },
+      AV_OPT_TYPE_BOOL, { .i64 = -1 }, -1, 1, VE | AV_OPT_FLAG_DEPRECATED },
 
-    { "tile_columns", "Log2 of number of tile columns to use (Deprecated, use svtav1-params)", OFFSET(tile_columns), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 4, VE | AV_OPT_FLAG_DEPRECATED },
-    { "tile_rows", "Log2 of number of tile rows to use (Deprecated, use svtav1-params)", OFFSET(tile_rows), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 6, VE | AV_OPT_FLAG_DEPRECATED },
+    { "tile_columns", "Log2 of number of tile columns to use (Deprecated, use svtav1-params)", OFFSET(tile_columns), AV_OPT_TYPE_INT, { .i64 = -1 }, -1, 4, VE | AV_OPT_FLAG_DEPRECATED },
+    { "tile_rows", "Log2 of number of tile rows to use (Deprecated, use svtav1-params)", OFFSET(tile_rows), AV_OPT_TYPE_INT, { .i64 = -1 }, -1, 6, VE | AV_OPT_FLAG_DEPRECATED },
 #endif
 
     { "svtav1-params", "Set the SVT-AV1 configuration using a :-separated list of key=value parameters", OFFSET(svtav1_opts), AV_OPT_TYPE_DICT, { 0 }, 0, 0, VE },