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

[James Almer]

On 5/7/2018 8:05 PM, Hendrik Leppkes wrote:
> On Sun, May 6, 2018 at 10:27 PM, James Almer <jamrial at gmail.com> wrote:
>> On 5/5/2018 5:38 PM, James Almer wrote:
>>> On 4/10/2018 2:16 PM, Sergey Lavrushkin wrote:
>>>> diff --git a/libavfilter/vf_srcnn.c b/libavfilter/vf_srcnn.c
>>>> new file mode 100644
>>>> index 0000000000..d9b4891f7f
>>>> --- /dev/null
>>>> +++ b/libavfilter/vf_srcnn.c
>>>> @@ -0,0 +1,420 @@
>>>> +/*
>>>> + * Copyright (c) 2018 Sergey Lavrushkin
>>>> + *
>>>> + * This file is part of FFmpeg.
>>>> + *
>>>> + * FFmpeg is free software; you can redistribute it and/or
>>>> + * modify it under the terms of the GNU Lesser General Public
>>>> + * License as published by the Free Software Foundation; either
>>>> + * version 2.1 of the License, or (at your option) any later version.
>>>> + *
>>>> + * FFmpeg is distributed in the hope that it will be useful,
>>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>>>> + * Lesser General Public License for more details.
>>>> + *
>>>> + * You should have received a copy of the GNU Lesser General Public
>>>> + * License along with FFmpeg; if not, write to the Free Software
>>>> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
>>>> + */
>>>> +
>>>> +/**
>>>> + * @file
>>>> + * Filter implementing image super-resolution using deep convolutional networks.
>>>> + * https://arxiv.org/abs/1501.00092
>>>> + */
>>>> +
>>>> +#include "avfilter.h"
>>>> +#include "formats.h"
>>>> +#include "internal.h"
>>>> +#include "libavutil/opt.h"
>>>> +#include "unistd.h"
>>>
>>> This broke msvc. It should be (if anything) <unistd.h>, and wrapped
>>> inside a HAVE_UNISTD_H preprocessor check.
>>
>> I (accidentally) applied this change as part of an unrelated commit, so
>> that's dealt with at least.
>>
>>>
>>>> +static av_cold int init(AVFilterContext* context)
>>>> +{
>>>> +    SRCNNContext *srcnn_context = context->priv;
>>>> +    AVIOContext* config_file_context;
>>>> +    int64_t file_size, srcnn_size;
>>>> +
>>>> +    /// Check specified confguration file name and read network weights from it
>>>> +    if (!srcnn_context->config_file_path){
>>>> +        av_log(context, AV_LOG_INFO, "configuration file for network was not specified, using default weights for x2 upsampling\n");
>>>> +
>>>> +        /// Create convolution kernels and copy default weights
>>>> +        srcnn_context->conv1.input_channels = 1;
>>>> +        srcnn_context->conv1.output_channels = 64;
>>>> +        srcnn_context->conv1.size = 9;
>>>> +        CHECK_ALLOCATION(allocate_copy_conv_data(&srcnn_context->conv1, conv1_kernel, conv1_biases), )
>>>> +
>>>> +        srcnn_context->conv2.input_channels = 64;
>>>> +        srcnn_context->conv2.output_channels = 32;
>>>> +        srcnn_context->conv2.size = 1;
>>>> +        CHECK_ALLOCATION(allocate_copy_conv_data(&srcnn_context->conv2, conv2_kernel, conv2_biases), )
>>>> +
>>>> +        srcnn_context->conv3.input_channels = 32;
>>>> +        srcnn_context->conv3.output_channels = 1;
>>>> +        srcnn_context->conv3.size = 5;
>>>> +        CHECK_ALLOCATION(allocate_copy_conv_data(&srcnn_context->conv3, conv3_kernel, conv3_biases), )
>>>> +    }
>>>> +    else if (access(srcnn_context->config_file_path, R_OK) != -1){
>>>
>>> This seems to be the function needed from unistd.h
>>>
>>> Looking at configure and libavformat/file.c it's apparently not
>>> available everywhere (we have a HAVE_ACCESS define, and also check for
>>> mode constants like R_OK).
>>
>> MSVC does not have access(), so i have temporarily made this filter
>> depend on its presence until a proper fix is applied. That target has
>> been broken for a few days now and that shouldn't be the case.
>> It should be a matter or doing the same thing as in libavformat/file.c,
>> i guess, or perhaps use _access() instead, which is available in MSVC.
> 
> Your check doesn't work, MSVC building is still broken.
> Probably because MSVC does have access, in io.h, but it does not have
> the R_OK token.
> 
> - Hendrik

I sent a patch to make the access() check stricter.

access() in msvc is in any case kinda weird. The check as is succeeds
(so the function exists in some form in the c library) so HAVE_ACCESS is
set to true in both msvc and mingw, but msvc doesn't define R_OK or any
other such constant.
We have a wrapper using msvc's specific _access() in lavf that's
ultimately only used by mingw because the only file calling access() is
file.c and it first makes sure said constants are also set.

With my configure patch the check will only succeed on mingw, so
technically changing nothing as far as lavf is concerned, but
effectively disabling the filter on msvc so lavfi can compile.

Better solution of course welcome.