[FFmpeg-devel] [PATCH v3 1/2] avfilter/dnn/dnn_backend_tf: simplify the code with ff_hex_to_data

[lance.lmwang at gmail.com]

On Thu, Apr 29, 2021 at 12:20:17PM +0000, Guo, Yejun wrote:
> 
> 
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> > lance.lmwang at gmail.com
> > Sent: 2021年4月28日 21:17
> > To: ffmpeg-devel at ffmpeg.org
> > Subject: Re: [FFmpeg-devel] [PATCH v3 1/2] avfilter/dnn/dnn_backend_tf:
> > simplify the code with ff_hex_to_data
> > 
> > On Wed, Apr 28, 2021 at 12:26:54PM +0000, Guo, Yejun wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> > > > lance.lmwang at gmail.com
> > > > Sent: 2021年4月28日 18:47
> > > > To: ffmpeg-devel at ffmpeg.org
> > > > Cc: Limin Wang <lance.lmwang at gmail.com>
> > > > Subject: [FFmpeg-devel] [PATCH v3 1/2] avfilter/dnn/dnn_backend_tf:
> > > > simplify the code with ff_hex_to_data
> > > >
> > > > From: Limin Wang <lance.lmwang at gmail.com>
> > > >
> > > > please use tools/python/tf_sess_config.py to get the sess_config after
> > that.
> > > > note the byte order of session config is in normal order.
> > > > bump the MICRO version for the config change.
> > > >
> > > > Signed-off-by: Limin Wang <lance.lmwang at gmail.com>
> > > > ---
> > > >  libavfilter/dnn/dnn_backend_tf.c | 42 +++++++------------------------------
> > > >  libavfilter/version.h            |  2 +-
> > > >  tools/python/tf_sess_config.py   | 45
> > > > ++++++++++++++++++++++++++++++++++++++++
> > > >  3 files changed, 54 insertions(+), 35 deletions(-)
> > > >  create mode 100644 tools/python/tf_sess_config.py
> > > >
> > > > diff --git a/libavfilter/dnn/dnn_backend_tf.c
> > > > b/libavfilter/dnn/dnn_backend_tf.c
> > > > index fb799d2..076dd3d 100644
> > > > --- a/libavfilter/dnn/dnn_backend_tf.c
> > > > +++ b/libavfilter/dnn/dnn_backend_tf.c
> > > > @@ -28,6 +28,7 @@
> > > >  #include "dnn_backend_native_layer_conv2d.h"
> > > >  #include "dnn_backend_native_layer_depth2space.h"
> > > >  #include "libavformat/avio.h"
> > > > +#include "libavformat/internal.h"
> > > >  #include "libavutil/avassert.h"
> > > >  #include "../internal.h"
> > > >  #include "dnn_backend_native_layer_pad.h"
> > > > @@ -206,53 +207,26 @@ static DNNReturnType load_tf_model(TFModel
> > > > *tf_model, const char *model_filename
> > > >
> > > >      // prepare the sess config data
> > > >      if (tf_model->ctx.options.sess_config != NULL) {
> > > > +        const char *config;
> > > >          /*
> > > >          tf_model->ctx.options.sess_config is hex to present the
> > serialized
> > > > proto
> > > >          required by TF_SetConfig below, so we need to first generate
> > the
> > > > serialized
> > > > -        proto in a python script, the following is a script example to
> > > > generate
> > > > -        serialized proto which specifies one GPU, we can change the
> > script
> > > > to add
> > > > -        more options.
> > > > -
> > > > -        import tensorflow as tf
> > > > -        gpu_options = tf.GPUOptions(visible_device_list='0')
> > > > -        config = tf.ConfigProto(gpu_options=gpu_options)
> > > > -        s = config.SerializeToString()
> > > > -        b = ''.join("%02x" % int(ord(b)) for b in s[::-1])
> > > > -        print('0x%s' % b)
> > > > -
> > > > -        the script output looks like: 0xab...cd, and then pass 0xab...cd to
> > > > sess_config.
> > > > +        proto in a python script, tools/python/tf_sess_config.py is a
> > script
> > > > example
> > > > +        to generate the configs of sess_config.
> > > >          */
> > > > -        char tmp[3];
> > > > -        tmp[2] = '\0';
> > > > -
> > > >          if (strncmp(tf_model->ctx.options.sess_config, "0x", 2) != 0) {
> > > >              av_log(ctx, AV_LOG_ERROR, "sess_config should start with
> > > > '0x'\n");
> > > >              return DNN_ERROR;
> > > >          }
> > > > +        config = tf_model->ctx.options.sess_config + 2;
> > > > +        sess_config_length = ff_hex_to_data(NULL, config);
> > > >
> > > > -        sess_config_length = strlen(tf_model->ctx.options.sess_config);
> > > > -        if (sess_config_length % 2 != 0) {
> > > > -            av_log(ctx, AV_LOG_ERROR, "the length of sess_config is
> > not
> > > > even (%s), "
> > > > -                                      "please re-generate the
> > > > config.\n",
> > > > -
> > > > tf_model->ctx.options.sess_config);
> > > > -            return DNN_ERROR;
> > > > -        }
> > > > -
> > > > -        sess_config_length -= 2; //ignore the first '0x'
> > > > -        sess_config_length /= 2; //get the data length in byte
> > > > -
> > > > -        sess_config = av_malloc(sess_config_length);
> > > > +        sess_config = av_mallocz(sess_config_length +
> > > > AV_INPUT_BUFFER_PADDING_SIZE);
> > >
> > > just get a concern, why we need to add PADDING_SIZE here.
> > > Will there be potential issue if not add?
> > 
> > I just want to make sure it's safe even if the sess_config_length is zero.
> > 
> ok, will push soon.

I plan to submit a patch for ff_hex_to_data(), add one more parameter for data_size.

int ff_hex_to_data(uint8_t *data, const char *p);

to 

int ff_hex_to_data(uint8_t *data, int data_size, const char *p);

Or if the string size is large than alloc memory, it'll cause invalid memory access.

> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".

-- 
Thanks,
Limin Wang