[FFmpeg-devel] [PATCH 07/10] avfilter/vf_nlmeans: Move ff_nlmeans_init into a header

[Hendrik Leppkes]

On Fri, May 13, 2022 at 11:26 AM Andreas Rheinhardt
<andreas.rheinhardt at outlook.com> wrote:
>
> Soft Works:
> >
> >
> >> -----Original Message-----
> >> From: Andreas Rheinhardt <andreas.rheinhardt at outlook.com>
> >> Sent: Friday, May 13, 2022 10:27 AM
> >> To: Soft Works <softworkz at hotmail.com>; FFmpeg development discussions
> >> and patches <ffmpeg-devel at ffmpeg.org>
> >> Subject: Re: [FFmpeg-devel] [PATCH 07/10] avfilter/vf_nlmeans: Move
> >> ff_nlmeans_init into a header
> >>
> >> Soft Works:
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> >>>> Andreas Rheinhardt
> >>>> Sent: Tuesday, May 3, 2022 8:38 AM
> >>>> To: ffmpeg-devel at ffmpeg.org
> >>>> Cc: Andreas Rheinhardt <andreas.rheinhardt at outlook.com>
> >>>> Subject: [FFmpeg-devel] [PATCH 07/10] avfilter/vf_nlmeans: Move
> >>>> ff_nlmeans_init into a header
> >>>>
> >>>> This removes a dependency of checkasm on lavfi/vf_nlmeans.o
> >>>> and also allows to inline ff_nlmeans_init() irrespectively of
> >>>> interposing.
> >>>>
> >>>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at outlook.com>
> >>>> ---
> >>>
> >>> [..]
> >>>
> >>>> +
> >>>> +static av_unused void ff_nlmeans_init(NLMeansDSPContext *dsp)
> >>>> +{
> >>>> +    dsp->compute_safe_ssd_integral_image =
> >>>> compute_safe_ssd_integral_image_c;
> >>>> +    dsp->compute_weights_line = compute_weights_line_c;
> >>>> +
> >>>> +    if (ARCH_AARCH64)
> >>>> +        ff_nlmeans_init_aarch64(dsp);
> >>>
> >>> Hi Andreas,
> >>>
> >>> the above breaks compilation for me:
> >>>
> >>> 1>libavfilterd.lib(libavfilter_vf_nlmeans.obj) : error LNK2019:
> >> unresolved external symbol ff_nlmeans_init_aarch64 referenced in
> >> function ff_nlmeans_init
> >>>
> >>> The reason is that I'm (obviously) not compiling stuff from the
> >>> libavfilter\aarch64 subfolder.
> >>>
> >>> It might need an #ifdef ?
> >>>
> >>> I haven't taken a deeper look at it, though.
> >>>
> >>> Thanks,
> >>> softworkz
> >>>
> >>>
> >>
> >> That surprises me: The earlier code did exactly the same; in fact,
> >> using
> >> if (ARCH_*) is our typical check for arches in dsp-init code.
> >
> > I looked at this a bit further. It seems that the VS project
> > generation tool that I'm using is creating dummy definitions
> > for such cases. In the previous workspace it had generated
> >
> > void ff_nlmeans_init_aarch64(NLMeansDSPContext *dsp) {return;}
> >
> > in a separate code file for being able to work with the ffmpeg
> > code in VS without modifying any of the code.
> >
> > Now that you have moved that code to a header file, this logic
> > doesn't work anymore.
> >
>
> Why does your compiler not just eliminate dead code like all the others?
> Is this MSVC -O0 behaviour?


MSVC does not do DCE when optimizations are disabled, yes. So this is
where this is coming from.

> I just sent the patch
> https://ffmpeg.org/pipermail/ffmpeg-devel/2022-May/296380.html, but now
> that I read this I have to agree with Hendrik: Why not update your tool
> instead?
>

Actually looking at the tool, it seems like it handles header files.
Did you actually re-run the generation tool after applying this patch?

- Hendrik