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

[Pedro Arthur]

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?