> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> On Behalf Of
> Alexander Strasser
> Sent: 2020年8月21日 0:58
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH V4] dnn_backend_openvino.c: parse options
> in openvino backend
>
> On 2020-08-20 01:10 +0000, Guo, Yejun wrote:
> >
> > > -----Original Message-----
> > > From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> On Behalf Of
> > > Alexander Strasser
> > > Sent: 2020年8月20日 4:06
> > > To: FFmpeg development discussions and patches
> > > <ffmpeg-devel@ffmpeg.org>
> > > Subject: Re: [FFmpeg-devel] [PATCH V4] dnn_backend_openvino.c: parse
> > > options in openvino backend
> > >
> > > On 2020-08-18 23:08 +0800, Guo, Yejun wrote:
> > > > Signed-off-by: Guo, Yejun <yejun....@intel.com>
> > > > ---
> > > > v3: use AVOption
> > > > v4: don't add new file dnn_common.h/c
> > > >
> > > > libavfilter/dnn/dnn_backend_openvino.c | 50
> > > > +++++++++++++++++++++++++++++++++-
> > > > 1 file changed, 49 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/libavfilter/dnn/dnn_backend_openvino.c
> > > > b/libavfilter/dnn/dnn_backend_openvino.c
> > > > index d343bf2..277c313 100644
> > > > --- a/libavfilter/dnn/dnn_backend_openvino.c
> > > > +++ b/libavfilter/dnn/dnn_backend_openvino.c
> > > > @@ -26,9 +26,37 @@
> > > > #include "dnn_backend_openvino.h"
> > > > #include "libavformat/avio.h"
> > > > #include "libavutil/avassert.h"
> > > > +#include "libavutil/opt.h"
> > > > #include <c_api/ie_c_api.h>
> > > >
> > > > +typedef struct OVOptions{
> > > > + uint32_t batch_size;
> > > > + uint32_t req_num;
> > > > +} OVOptions;
> > > > +
> > > > +typedef struct OvContext {
> > > > + const AVClass *class;
> > > > + OVOptions options;
> > > > +} OvContext;
> > > > +
> > > > +#define OFFSET(x) offsetof(OvContext, x) #define FLAGS
> > > > +AV_OPT_FLAG_FILTERING_PARAM static const AVOption
> > > > +dnn_ov_options[] = {
> > > > + { "batch", "batch size", OFFSET(options.batch_size),
> > > AV_OPT_TYPE_INT, { .i64 = 1 }, INT_MIN, INT_MAX, FLAGS },
> > > > + { "nireq", "number of request", OFFSET(options.req_num),
> > > AV_OPT_TYPE_INT, { .i64 = 1 }, INT_MIN, INT_MAX, FLAGS },
> > > > + { NULL },
> > > > +};
> > >
> > > If I'm not mistaken, you must use type int when using AV_OPT_TYPE_INT .
> > >
> > > AFAIK we have these integer types
> > >
> > > * AV_OPT_TYPE_INT -> int
> > > * AV_OPT_TYPE_INT64 -> int64_t
> > > * AV_OPT_TYPE_UINT64 -> uint64_t
> > >
> > > and you can assume int to be at least 32 bits wide.
> > >
> >
> > thanks, I'll change from uint32_t to int.
>
> Sounds about right.
>
> Though as I'm looking at the code again, you should correct the allowed range
> as
> well. I assume full signed int range was never intended, since the original
> type
> was uint32_t .
thanks, the range will be [0, int_max]
>
>
> > > > @@ -186,7 +235,6 @@ DNNModel *ff_dnn_load_model_ov(const char
> > > *model_filename, const char *options)
> > > > model->model = (void *)ov_model;
> > > > model->set_input_output = &set_input_output_ov;
> > > > model->get_input = &get_input_ov;
> > > > - model->options = options;
> > > >
> > > > return model;
> > > >
> > > > --
> > >
> > > Sorry, if I missed it, are the values set from the options used anywhere?
> >
> > You are right, the options are not used.
> >
> > My teammates and I are working on the dnn backend performance
> > improvement, we have done locally many quick dirty code to verify our
> > ideas and found it requires some changes in the DNN module including these
> options.
> > (In our quick code, we are using fixed magic number for these options)
>
> I feel you. It can be a long path, including back tracking at some points, to
> properly include some quick hacks.
>
>
> > So, as a collaboration, my idea is to separate the changes one patch
> > by one patch, and we can keep rebase locally, the final purpose is to
> > upstream
> all our local code with refinement.
>
> Sounds like a good idea.
>
> Would be good if you could do it in a way that the individual commits are
> mostly
> understandable on their own. Like here: parsing the options but not using them
> somewhere looks strange.
yes, good point. I'll just pending it and will send with other patch which uses
the options.
>
> If it's not feasibly possible, it would at least be required to mention the
> planned
> follow-ups in the commit message. So we can make sense of it when looking at
> the commit message years from now.
>
>
> Alexander
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email ffmpeg-devel-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".
