diff mbox series

[FFmpeg-devel] fftools/opt_common: add missing include of avf/version.h

Message ID pull.27.ffstaging.FFmpeg.1652435196848.ffmpegagent@gmail.com
State New
Headers show
Series [FFmpeg-devel] fftools/opt_common: add missing include of avf/version.h | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

softworkz May 13, 2022, 9:46 a.m. UTC
From: softworkz <softworkz@hotmail.com>

required for PRINT_LIB_INFO(avfilter...

Signed-off-by: softworkz <softworkz@hotmail.com>
---
    fftools/opt_common: add missing include of avf/version.h
    
    MSVC compiler complains without this include

Published-As: https://github.com/ffstaging/FFmpeg/releases/tag/pr-ffstaging-27%2Fsoftworkz%2Fsubmit_version_include-v1
Fetch-It-Via: git fetch https://github.com/ffstaging/FFmpeg pr-ffstaging-27/softworkz/submit_version_include-v1
Pull-Request: https://github.com/ffstaging/FFmpeg/pull/27

 fftools/opt_common.c | 2 ++
 1 file changed, 2 insertions(+)


base-commit: d2d8b9b972ba2df6b2a2ebe29f5307cbb7a69c33

Comments

Andreas Rheinhardt May 13, 2022, 1:32 p.m. UTC | #1
softworkz:
> From: softworkz <softworkz@hotmail.com>
> 
> required for PRINT_LIB_INFO(avfilter...
> 
> Signed-off-by: softworkz <softworkz@hotmail.com>
> ---
>     fftools/opt_common: add missing include of avf/version.h
>     
>     MSVC compiler complains without this include
> 
> Published-As: https://github.com/ffstaging/FFmpeg/releases/tag/pr-ffstaging-27%2Fsoftworkz%2Fsubmit_version_include-v1
> Fetch-It-Via: git fetch https://github.com/ffstaging/FFmpeg pr-ffstaging-27/softworkz/submit_version_include-v1
> Pull-Request: https://github.com/ffstaging/FFmpeg/pull/27
> 
>  fftools/opt_common.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/fftools/opt_common.c b/fftools/opt_common.c
> index c303db4d09..5a5e35bd7f 100644
> --- a/fftools/opt_common.c
> +++ b/fftools/opt_common.c
> @@ -51,6 +51,8 @@
>  #include "libavdevice/avdevice.h"
>  #include "libavdevice/version.h"
>  
> +#include "libavfilter/version.h"
> +
>  #include "libswscale/swscale.h"
>  #include "libswscale/version.h"
>  
> 
> base-commit: d2d8b9b972ba2df6b2a2ebe29f5307cbb7a69c33

What does "complain" here mean? Compilation failure?
It should already be included via
opt_common.h->cmdutils.h->avfilter.h->lavfi/version.h. The latter
inclusion relies on HAVE_AV_CONFIG_H to not be defined. It should only
be defined for the libraries, not fftools, so if it is defined for you
for this file your setup is wrong.
That being said it is nevertheless good to include this and avfilter.h
directly.

- Andreas
Soft Works May 13, 2022, 11:51 p.m. UTC | #2
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Andreas Rheinhardt
> Sent: Friday, May 13, 2022 3:33 PM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH] fftools/opt_common: add missing
> include of avf/version.h
> 
> softworkz:
> > From: softworkz <softworkz@hotmail.com>
> >
> > required for PRINT_LIB_INFO(avfilter...
> >
> > Signed-off-by: softworkz <softworkz@hotmail.com>
> > ---
> >     fftools/opt_common: add missing include of avf/version.h
> >
> >     MSVC compiler complains without this include
> >
> > Published-As: https://github.com/ffstaging/FFmpeg/releases/tag/pr-
> ffstaging-27%2Fsoftworkz%2Fsubmit_version_include-v1
> > Fetch-It-Via: git fetch https://github.com/ffstaging/FFmpeg pr-
> ffstaging-27/softworkz/submit_version_include-v1
> > Pull-Request: https://github.com/ffstaging/FFmpeg/pull/27
> >
> >  fftools/opt_common.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/fftools/opt_common.c b/fftools/opt_common.c
> > index c303db4d09..5a5e35bd7f 100644
> > --- a/fftools/opt_common.c
> > +++ b/fftools/opt_common.c
> > @@ -51,6 +51,8 @@
> >  #include "libavdevice/avdevice.h"
> >  #include "libavdevice/version.h"
> >
> > +#include "libavfilter/version.h"
> > +
> >  #include "libswscale/swscale.h"
> >  #include "libswscale/version.h"
> >
> >
> > base-commit: d2d8b9b972ba2df6b2a2ebe29f5307cbb7a69c33
> 
> What does "complain" here mean? Compilation failure?
> It should already be included via
> opt_common.h->cmdutils.h->avfilter.h->lavfi/version.h. The latter
> inclusion relies on HAVE_AV_CONFIG_H to not be defined. It should only
> be defined for the libraries, not fftools, so if it is defined for you
> for this file your setup is wrong.

You are right. HAVE_AV_CONFIG_H is defined also for the tools binaries.
But this seems to be the only case where it causes an issue.

> That being said it is nevertheless good to include this and avfilter.h
> directly.

Looking at the generation tool, it seems to be quite a complex task
to shape things in a way that it can be compiled with VS...
I can't speak for the author of the tool, but I think when at least
some cases that can easily be avoided (to require workarounds) at
the ffmpeg side, then it would be a good thing.
In that specific case, the version.h files from ALL libs are included
just not from avfilter, so I think it would also add to clarity to 
include it directly.

Similar goes for the DCE issue. For me there's just more clarity when
those bits are guarded by #if blocks, but I don't want to open a 
discussion about that. Just thank you for the quick patch to resolve
it!

Kind regards,
softworkz
diff mbox series

Patch

diff --git a/fftools/opt_common.c b/fftools/opt_common.c
index c303db4d09..5a5e35bd7f 100644
--- a/fftools/opt_common.c
+++ b/fftools/opt_common.c
@@ -51,6 +51,8 @@ 
 #include "libavdevice/avdevice.h"
 #include "libavdevice/version.h"
 
+#include "libavfilter/version.h"
+
 #include "libswscale/swscale.h"
 #include "libswscale/version.h"