I have no more comment for this patch, thanks. yejun
-----Original Message----- From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf Of Pedro Arthur Sent: Tuesday, May 29, 2018 2:05 AM To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org> Subject: Re: [FFmpeg-devel] [GSOC] [PATCH] DNN module introduction and SRCNN filter update 2018-05-28 3:32 GMT-03:00 Guo, Yejun <yejun....@intel.com>: > looks that no tensorflow dependency is introduced, a new model format is > created together with some CPU implementation for inference. With this > idea, Android Neural Network would be a very good reference, see > https://developer.android.google.cn/ndk/guides/neuralnetworks/. It defines > how the model is organized, and also provided a CPU optimized inference > implementation (within the NNAPI runtime, it is open source). It is still > under development but mature enough to run some popular dnn models with > proper performance. We can absorb some basic design. Anyway, just a reference > fyi. (btw, I'm not sure about any IP issue) > > For this patch, I have two comments. > > 1. change from "DNNModel* (*load_default_model)(DNNDefaultModel model_type);" > to " DNNModel* (*load_builtin_model)(DNNBuiltinModel model_type);" > The DNNModule can be invoked by many filters, default model is a good name > at the filter level, while built-in model is better within the DNN scope. I have no problem with either name, both seems reasonable. > > typedef struct DNNModule{ > // Loads model and parameters from given file. Returns NULL if it is not > possible. > DNNModel* (*load_model)(const char* model_filename); > // Loads one of the default models > DNNModel* (*load_default_model)(DNNDefaultModel model_type); > // Executes model with specified input and output. Returns DNN_ERROR > otherwise. > DNNReturnType (*execute_model)(const DNNModel* model); > // Frees memory allocated for model. > void (*free_model)(DNNModel** model); } DNNModule; > > > 2. add a new variable 'number' for DNNData/InputParams > As a typical DNN concept, the data shape usually is: <number, height, width, > channel> or <number, channel, height, width>, the last component denotes its > index changes the fastest in the memory. We can add this concept into the > API, and decide to support <NHWC> or <NCHW> or both. You mean how often the layer filter data changes? If yes, I don't see much use for it, atm we are performing only inference. If there are no more concerns about the patch I may push it by tomorrow. Thanks. > > > Thanks > Yejun (郭叶军) > > -----Original Message----- > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf Of > Sergey Lavrushkin > Sent: Saturday, May 26, 2018 2:02 AM > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [GSOC] [PATCH] DNN module introduction and SRCNN > filter update > >> >> You should use the ff_ prefix for internal, non-static functions only. >> You don't need to use any prefix for internal structs. >> The AV* prefix is only for public structs, and the av_ prefix is for >> public functions. >> >> > >> > >> > And you need to indent and prettify the tables a bit. >> > >> > >> > Do you mean kernels and biases in dnn_srcnn.h? >> > Their formatting represents their 4D structure a little bit and it >> > is similar to one that I used for the first srcnn filter version >> > that was successfully pushed before. >> >> Yes, and i suppose they were pushed like that without the reviewer >> noticing they had no indentation or vertical alignment. >> > > Here is the patch with DNN module moved to libavfilter and proper formatting > of tables in dnn_srcnn.h. > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel