[FFmpeg-devel] [GSOC] [PATCH] SRCNN filter

Michael Niedermayer michael at niedermayer.cc
Thu May 3 23:58:46 EEST 2018


On Thu, May 03, 2018 at 03:17:11PM -0300, Pedro Arthur wrote:
> 2018-04-10 14:16 GMT-03:00 Sergey Lavrushkin <dualfal at gmail.com>:
> > 2018-03-29 3:55 GMT+03:00 Michael Niedermayer <michael at niedermayer.cc>:
> >
> >> On Wed, Mar 28, 2018 at 11:17:40AM +0300, Sergey Lavrushkin wrote:
> >> > > [...]
> >> > > > +#define OFFSET(x) offsetof(SRCNNContext, x)
> >> > > > +#define FLAGS AV_OPT_FLAG_FILTERING_PARAM | AV_OPT_FLAG_VIDEO_PARAM
> >> > > > +static const AVOption srcnn_options[] = {
> >> > > > +    { "config_file", "path to configuration file with network
> >> > > parameters", OFFSET(config_file_path), AV_OPT_TYPE_STRING,
> >> {.str=NULL}, 0,
> >> > > 0, FLAGS },
> >> > > > +    { NULL }
> >> > > > +};
> >> > > > +
> >> > > > +AVFILTER_DEFINE_CLASS(srcnn);
> >> > > > +
> >> > > > +#define CHECK_FILE(file)    if (ferror(file) || feof(file)){ \
> >> > > > +                                av_log(context, AV_LOG_ERROR, "error
> >> > > reading configuration file\n");\
> >> > > > +                                fclose(file); \
> >> > > > +                                return AVERROR(EIO); \
> >> > > > +                            }
> >> > > > +
> >> > > > +#define CHECK_ALLOCATION(conv, file)    if
> >> > > (allocate_and_read_convolution_data(&conv, file)){ \
> >> > > > +                                            av_log(context,
> >> > > AV_LOG_ERROR, "could not allocate memory for convolutions\n"); \
> >> > > > +                                            fclose(file); \
> >> > > > +                                            return AVERROR(ENOMEM);
> >> \
> >> > > > +                                        }
> >> > > > +
> >> > >
> >> > > > +static int allocate_and_read_convolution_data(Convolution* conv,
> >> FILE*
> >> > > config_file)
> >> > > > +{
> >> > > > +    int32_t kernel_size = conv->output_channels * conv->size *
> >> > > conv->size * conv->input_channels;
> >> > > > +    conv->kernel = av_malloc(kernel_size * sizeof(double));
> >> > > > +    if (!conv->kernel){
> >> > > > +        return 1;
> >> > >
> >> > > this should return an AVERROR code for consistency with the rest of
> >> > > the codebase
> >> > >
> >> >
> >> > Ok.
> >> >
> >> >
> >> > > > +    }
> >> > >
> >> > > > +    fread(conv->kernel, sizeof(double), kernel_size, config_file);
> >> > >
> >> > > directly reading data types is not portable, it would for example be
> >> > > endian specific
> >> > > and using avio for reading may be better, though fread is as far as iam
> >> > > concerned also ok
> >> > >
> >> >
> >> > Ok, I understand the problem, but I have not really worked with it
> >> before,
> >> > so I need an advice of how to properly fix it. If I understand correctly,
> >> > for
> >>
> >> > int32_t I need to check endianness and reverse bytes if necessary. But
> >> with
> >>
> >> see avio_rb32()
> >> might not be as fast as reading a whole array and byteswaping though.
> >> Not sure it matters
> >>
> >>
> >> > doubles it is more complicated. Should I write a IEEE 754 converter from
> >> > binary
> >> > to double or maybe I can somehow check IEEE 754 doubles support and
> >> > depending
> >> > on it either stick to the default network weights, or just read bytes and
> >> > check
> >> > endianness, if IEEE 754 doubles are supported? Or maybe avio provide some
> >> > utility to deal with this problem?
> >>
> >> something like this should work:
> >> av_int2float(avio_rb32(pb));
> >> av_int2double(avio_rb64(pb));
> >>
> >> >
> >> >
> >> > > [...]
> >> > > > +/**
> >> > > > + * @file
> >> > > > + * Default cnn weights for x2 upsampling with srcnn filter.
> >> > > > + */
> >> > > > +
> >> > > > +/// First convolution kernel
> >> > >
> >> > > > +static double conv1_kernel[] = {
> >> > >
> >> > > static data should be also const, otherwise it may be changed and could
> >> > > cause
> >> > > thread saftey issues
> >> > >
> >> >
> >> > Ok, I just wanted to not allocate additional memory in case of using
> >> > default weights.
> >>
> >> well, there can be more than one instance of a filter running at the same
> >> time
> >> so static variables that are actually changing will affect the other
> >> filter instance
> >>
> >
> > Hi,
> >
> > Here is the updated patch with fixed issues.
> > Sorry for the late reply, I was busy doing things related to my university
> > activities.
> >
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel at ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> 
> I think all concerns were addressed, can the patch be pushed?

sure you can push it if it looks ok to you.
please make sure fate-source passes (i think it fails with the patch)

thanks

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Into a blind darkness they enter who follow after the Ignorance,
they as if into a greater darkness enter who devote themselves
to the Knowledge alone. -- Isha Upanishad
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20180503/f0f0acb7/attachment.sig>


More information about the ffmpeg-devel mailing list