Re: [FFmpeg-devel] [PATCH v3 01/10] avfilter/dnn: Refactor DNN parameter configuration system

[Zhao Zhili <quinkblack@foxmail.com>]

> On May 6, 2024, at 19:48, Guo, Yejun <yejun.guo-at-intel.com@ffmpeg.org> wrote:
> 
>> -----Original Message-----
>> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org <mailto:ffmpeg-devel-bounces@ffmpeg.org>> On Behalf Of Zhao
>> Zhili
>> Sent: Tuesday, April 30, 2024 3:12 PM
>> To: ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org>
>> Cc: Zhao Zhili <zhilizhao@tencent.com <mailto:zhilizhao@tencent.com>>
>> Subject: [FFmpeg-devel] [PATCH v3 01/10] avfilter/dnn: Refactor DNN
>> parameter configuration system
>> 
>> From: Zhao Zhili <zhilizhao@tencent.com>
>> 
>> This patch trying to resolve mulitiple issues related to parameter
>> configuration:
>> 
>> Firstly, each DNN filters duplicate DNN_COMMON_OPTIONS, which should
>> be the common options of backend.
>> 
>> Secondly, backend options are hidden behind the scene. It's a
>> AV_OPT_TYPE_STRING backend_configs for user, and parsed by each
>> backend. We don't know each backend support what kind of options
>> from the help message.
>> 
>> Third, DNN backends duplicate DNN_BACKEND_COMMON_OPTIONS.
>> 
>> Last but not the least, pass backend options via AV_OPT_TYPE_STRING
>> makes it hard to pass AV_OPT_TYPE_BINARY to backend, if not impossible.
>> 
>> This patch puts backend common options and each backend options inside
>> DnnContext to reduce code duplication, make options user friendly, and
>> easy to extend for future usecase.
>> 
>> There is a known issue that, for a filter which only support one or two
>> of the backends, the help message still show the option of all three
>> backends. Each DNN filter should be able to run on any backend. Current
>> issue is mostly due to incomplete implementation (e.g., libtorch only
>> support DFT_PROCESS_FRAME), and lack of maintenance on the filters.
> 
> This patch 01 basically looks good, two comments:
> - it is possible that we add one dnn filter with one backend support first, and then
> other backends one by one some-time later. So, please adjust the help message 
> accordingly with only the supported backends.

I can’t figure out how to do that properly without duplicating some code for each
filter. Do you have any idea?

> 
> - is it possible to split this patch into small patches for an easier detail review?

The configuration methods were scattered across filter common code, filters, backend
common code and backend implementation. I have written six version, patch 1/10
is hard to split to small patches like patch 2-10.

> 
>> 
>> For example,
>> 
>> ./ffmpeg -h filter=dnn_processing
>> 
>> dnn_processing AVOptions:
>>   dnn_backend       <int>        ..FV....... DNN backend (from INT_MIN to
>> INT_MAX) (default tensorflow)
>>     tensorflow      1            ..FV....... tensorflow backend flag
>>     openvino        2            ..FV....... openvino backend flag
>>     torch           3            ..FV....... torch backend flag
>> 
>> dnn_base AVOptions:
>>   model             <string>     ..F........ path to model file
>>   input             <string>     ..F........ input name of the model
>>   output            <string>     ..F........ output name of the model
>>   backend_configs   <string>     ..F.......P backend configs (deprecated)
>>   options           <string>     ..F.......P backend configs (deprecated)
>>   nireq             <int>        ..F........ number of request (from 0 to INT_MAX)
>> (default 0)
>>   async             <boolean>    ..F........ use DNN async inference (default true)
>>   device            <string>     ..F........ device to run model
>> 
>> dnn_tensorflow AVOptions:
>>   sess_config       <string>     ..F........ config for SessionOptions
>> 
>> dnn_openvino AVOptions:
>>   batch_size        <int>        ..F........ batch size per request (from 1 to 1000)
>> (default 1)
>>   input_resizable   <boolean>    ..F........ can input be resizable or not (default
>> false)
>>   layout            <int>        ..F........ input layout of model (from 0 to 2) (default
>> none)
>>     none            0            ..F........ none
>>     nchw            1            ..F........ nchw
>>     nhwc            2            ..F........ nhwc
>>   scale             <float>      ..F........ Add scale preprocess operation. Divide each
>> element of input by specified value. (from INT_MIN to INT_MAX) (default 0)
>>   mean              <float>      ..F........ Add mean preprocess operation. Subtract
>> specified value from each element of input. (from INT_MIN to INT_MAX)
>> (default 0)
>> 
>> dnn_th AVOptions:
>>   optimize          <int>        ..F........ turn on graph executor optimization (from 0
>> to 1) (default 0)
>> ---
>> libavfilter/dnn/dnn_backend_common.h   |  13 ++-
>> libavfilter/dnn/dnn_backend_openvino.c | 146 ++++++++++---------------
>> libavfilter/dnn/dnn_backend_tf.c       |  82 +++++---------
>> libavfilter/dnn/dnn_backend_torch.cpp  |  67 ++++--------
>> libavfilter/dnn/dnn_interface.c        |  89 +++++++++++++++
>> libavfilter/dnn_filter_common.c        |  38 ++++++-
>> libavfilter/dnn_filter_common.h        |  39 +++----
>> libavfilter/dnn_interface.h            |  67 +++++++++++-
>> libavfilter/vf_derain.c                |   6 +-
>> libavfilter/vf_dnn_classify.c          |   4 +-
>> libavfilter/vf_dnn_detect.c            |   4 +-
>> libavfilter/vf_dnn_processing.c        |   4 +-
>> libavfilter/vf_sr.c                    |   6 +-
>> 13 files changed, 336 insertions(+), 229 deletions(-)
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org>
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org <mailto: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".