[FFmpeg-devel] [PATCH] avformat/internal: Don't auto-include os_support.h

[FFmpeg-devel] [PATCH] avformat/internal: Don't auto-include os_support.h

[Andreas Rheinhardt]

It includes various Windows-specific headers when compiling
for Windows and these sometimes cause issues: E.g. winbase.h
defines IGNORE, which clashes with a macro used in the Matroska
muxer (since 884653ee5be03ed38db957c14fad51b300611c8c) and demuxer.

This header provides fallback defines for various stuff that is
mostly not used directly by (de)muxers at all:
mkdir, rename, rmdir, unlink, access, poll, pollfd, nfds_t,
closesocket, socklen_t, fstat, stat, lseek, SHUT_(RD|WR|RDWR)
and various POLL* constants.

Ergo fix this issue by not auto-including this header in lots
of places via an inclusion in internal.h and instead include
it everywhere where the above stuff is used (most of these
translation units already included os_support.h).

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavformat/img2dec.c  | 1 +
 libavformat/internal.h | 1 -
 libavformat/rtsp.h     | 1 +
 libavformat/utils.c    | 1 +
 4 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/libavformat/img2dec.c b/libavformat/img2dec.c
index 15fd67927f..2761cb37a4 100644
--- a/libavformat/img2dec.c
+++ b/libavformat/img2dec.c
@@ -36,6 +36,7 @@
 #include "avio_internal.h"
 #include "internal.h"
 #include "img2.h"
+#include "os_support.h"
 #include "libavcodec/jpegxl_parse.h"
 #include "libavcodec/mjpeg.h"
 #include "libavcodec/vbn.h"
diff --git a/libavformat/internal.h b/libavformat/internal.h
index 594afd731d..53e70ccb53 100644
--- a/libavformat/internal.h
+++ b/libavformat/internal.h
@@ -26,7 +26,6 @@
 #include "libavcodec/packet_internal.h"
 
 #include "avformat.h"
-#include "os_support.h"
 
 #define MAX_URL_SIZE 4096
 
diff --git a/libavformat/rtsp.h b/libavformat/rtsp.h
index 6e500fd56a..83b2e3f4fb 100644
--- a/libavformat/rtsp.h
+++ b/libavformat/rtsp.h
@@ -28,6 +28,7 @@
 #include "network.h"
 #include "httpauth.h"
 #include "internal.h"
+#include "os_support.h"
 
 #include "libavutil/log.h"
 #include "libavutil/opt.h"
diff --git a/libavformat/utils.c b/libavformat/utils.c
index cf4d68bff9..c722743744 100644
--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -37,6 +37,7 @@
 #if CONFIG_NETWORK
 #include "network.h"
 #endif
+#include "os_support.h"
 
 static AVMutex avformat_mutex = AV_MUTEX_INITIALIZER;
 
-- 
2.34.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avformat/internal: Don't auto-include os_support.h

[Martin Storsjö]

On Thu, 31 Aug 2023, Andreas Rheinhardt wrote:

> It includes various Windows-specific headers when compiling
> for Windows and these sometimes cause issues: E.g. winbase.h
> defines IGNORE, which clashes with a macro used in the Matroska
> muxer (since 884653ee5be03ed38db957c14fad51b300611c8c) and demuxer.
>
> This header provides fallback defines for various stuff that is
> mostly not used directly by (de)muxers at all:
> mkdir, rename, rmdir, unlink, access, poll, pollfd, nfds_t,
> closesocket, socklen_t, fstat, stat, lseek, SHUT_(RD|WR|RDWR)
> and various POLL* constants.
>
> Ergo fix this issue by not auto-including this header in lots
> of places via an inclusion in internal.h and instead include
> it everywhere where the above stuff is used (most of these
> translation units already included os_support.h).
>
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
> libavformat/img2dec.c  | 1 +
> libavformat/internal.h | 1 -
> libavformat/rtsp.h     | 1 +
> libavformat/utils.c    | 1 +
> 4 files changed, 3 insertions(+), 1 deletion(-)

Ok with me, if you did a reasonable effort to check that this covers all 
the cases of the redirected functions. Spreading those includes everywhere 
isn't very nice indeed.

This increases the risk of accidentally adding new uses of these functions 
without picking up the redirects, but I guess that's manageable.

// Martin

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avformat/internal: Don't auto-include os_support.h

[Andreas Rheinhardt]

Martin Storsjö:
> On Thu, 31 Aug 2023, Andreas Rheinhardt wrote:
> 
>> It includes various Windows-specific headers when compiling
>> for Windows and these sometimes cause issues: E.g. winbase.h
>> defines IGNORE, which clashes with a macro used in the Matroska
>> muxer (since 884653ee5be03ed38db957c14fad51b300611c8c) and demuxer.
>>
>> This header provides fallback defines for various stuff that is
>> mostly not used directly by (de)muxers at all:
>> mkdir, rename, rmdir, unlink, access, poll, pollfd, nfds_t,
>> closesocket, socklen_t, fstat, stat, lseek, SHUT_(RD|WR|RDWR)
>> and various POLL* constants.
>>
>> Ergo fix this issue by not auto-including this header in lots
>> of places via an inclusion in internal.h and instead include
>> it everywhere where the above stuff is used (most of these
>> translation units already included os_support.h).
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
>> ---
>> libavformat/img2dec.c  | 1 +
>> libavformat/internal.h | 1 -
>> libavformat/rtsp.h     | 1 +
>> libavformat/utils.c    | 1 +
>> 4 files changed, 3 insertions(+), 1 deletion(-)
> 
> Ok with me, if you did a reasonable effort to check that this covers all
> the cases of the redirected functions. Spreading those includes
> everywhere isn't very nice indeed.
> 

I used
'\b(mkdir|rename|rmdir|unlink|access|poll|pollfd|nfds_t|closesocket|socklen_t|fstat|stat|lseek|SHUT_(RD|WR|RDWR)|POLL(IN|OUT|RDNORM|WRNORM|RDBAND|WRBAND|PRI|ERR|HUP|NVAL))\b'

(Also note that internal.h is not included everywhere.)

> This increases the risk of accidentally adding new uses of these
> functions without picking up the redirects, but I guess that's manageable.
> 

It also reduces the risk of accidentally causing macro conflicts like
with IGNORE.

- Andreas

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".