On 6/2/2018 3:45 PM, Sergey Lavrushkin wrote: > 2018-06-02 19:45 GMT+03:00 James Almer <jamr...@gmail.com > <mailto:jamr...@gmail.com>>: > > On 5/31/2018 12:01 PM, Sergey Lavrushkin wrote: > > diff --git a/Changelog b/Changelog > > index df2024fb59..a667fd045d 100644 > > --- a/Changelog > > +++ b/Changelog > > @@ -11,6 +11,7 @@ version <next>: > > - support mbedTLS based TLS > > - DNN inference interface > > - Reimplemented SRCNN filter using DNN inference interface > > +- TensorFlow DNN backend > > This and the two entries you added earlier don't really belong here. > It's enough with the line stating the filter was introduced back in > ffmpeg 4.0 > > > I should not add any line regarding introduced DNN inference module, > that can be usefull for someone writing another filter based on DNN?
Changelog lists the newly added features of interest to users, not developers. When the CBS framework was added it wasn't mentioned here either, only the filters making use of it. Users don't care that the scrnn filter was reimplemented using this new framework, but are however interested to know that the filter was added, or at most if its features were considerably expanded. So based on that, I guess it's acceptable to add a line in this patch like - libtensorflow backend for DNN based filters like srcnn. Or similar. But the earlier two entries don't belong here. > > > > > > > > version 4.0: > > diff --git a/configure b/configure > > index 09ff0c55e2..47e21fec39 100755 > > --- a/configure > > +++ b/configure > > @@ -259,6 +259,7 @@ External library support: > > --enable-libspeex enable Speex de/encoding via libspeex [no] > > --enable-libsrt enable Haivision SRT protocol via > libsrt [no] > > --enable-libssh enable SFTP protocol via libssh [no] > > + --enable-libtensorflow enable TensorFlow as a DNN module > backend [no] > > Maybe mention it's for the srcnn filter. > > > --enable-libtesseract enable Tesseract, needed for ocr > filter [no] > > --enable-libtheora enable Theora encoding via libtheora [no] > > --enable-libtls enable LibreSSL (via libtls), needed > for https support > > @@ -1713,6 +1714,7 @@ EXTERNAL_LIBRARY_LIST=" > > libspeex > > libsrt > > libssh > > + libtensorflow > > libtesseract > > libtheora > > libtwolame > > @@ -3453,7 +3455,7 @@ avcodec_select="null_bsf" > > avdevice_deps="avformat avcodec avutil" > > avdevice_suggest="libm" > > avfilter_deps="avutil" > > -avfilter_suggest="libm" > > +avfilter_suggest="libm libtensorflow" > > Add instead > > srcnn_filter_suggest="libtensorflow" > > To the corresponding section. > > > But this DNN inference module can be used for other filters. > At least, I think, that after training more complicated models for super > resolution I'll have to add them as separate filters. > So, I thought, this module shouldn't be a part of srcnn filter from the > begining. > Or is it better to add *_filter_suggest="libtensorflow" to the > configure script and > dnn_*.o to the Makefile for every new filter based on this module? Yes. If it's a non public framework/API used by one or more modules that can be disabled (as is the case with filters) then it should be added to the list of object dependencies for each of those optional modules. See framesync.o for another example of this. That being said, another way to implement this is making DNN its own independent module in configure, like for example cbs, fft and many others. This way, configure would look something like dnn_suggest="libtensorflow" srcnn_filter_select="dnn" With dnn being an entry added to the CONFIG_EXTRA list. Then in libavfilter/Makefile you'd do OBJS-$(CONFIG_DNN) += dnn_interface.o dnn_backend_native.o dnn_backend_tf.o Or even better, something like DNN-OBJS-$(CONFIG_LIBTENSORFLOW) += dnn_backend_tf.o OBJS-$(CONFIG_DNN) += dnn_interface.o dnn_backend_native.o $(DNN-OBJS-yes) Which would allow you to remove the #if preprocessor check in dnn_backend_tf.c, as it will only be compiled when both libtensorflow and at least one filter using dnn are enabled. The reason for all this is to have everything as modular as possible, reducing compiled objects and dependencies (like lavfi linking to lavf) to exactly what's strictly needed. > > > > avformat_deps="avcodec avutil" > > avformat_suggest="libm network zlib" > > avresample_deps="avutil" > > @@ -6055,6 +6057,7 @@ enabled libsoxr && require libsoxr > soxr.h soxr_create -lsoxr > > enabled libssh && require_pkg_config libssh libssh > libssh/sftp.h sftp_init > > enabled libspeex && require_pkg_config libspeex speex > speex/speex.h speex_decoder_init > > enabled libsrt && require_pkg_config libsrt "srt >= > 1.2.0" srt/srt.h srt_socket > > +enabled libtensorflow && require libtensorflow > tensorflow/c/c_api.h TF_Version -ltensorflow && add_cflags > -DTENSORFLOW_BACKEND > > Superfluous define. Just check for CONFIG_LIBTENSORFLOW instead. > > > enabled libtesseract && require_pkg_config libtesseract > tesseract tesseract/capi.h TessBaseAPICreate > > enabled libtheora && require libtheora theora/theoraenc.h > th_info_init -ltheoraenc -ltheoradec -logg > > enabled libtls && require_pkg_config libtls libtls > tls.h tls_configure > > diff --git a/libavfilter/Makefile b/libavfilter/Makefile > > index 3201cbeacf..82915e2f75 100644 > > --- a/libavfilter/Makefile > > +++ b/libavfilter/Makefile > > @@ -14,6 +14,7 @@ OBJS = allfilters.o > \ > > buffersrc.o > \ > > dnn_interface.o > \ > > dnn_backend_native.o > \ > > + dnn_backend_tf.o > \ > > See Jan Ekström's patch. Add this to the filter's entry as all these > source files should not be compiled unconditionally. > > > drawutils.o > \ > > fifo.o > \ > > formats.o > \ > > _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel