| Message ID | AS8P250MB074446341A647914DB5851A28FE5A@AS8P250MB0744.EURP250.PROD.OUTLOOK.COM |
|---|---|
| State | Accepted |
| Commit | ee485b4051c24e2cebe46b426294b346f0f5fff4 |
| Headers | show |
| Series | [FFmpeg-devel] avformat/internal: Don't auto-include os_support.h | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
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
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
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;
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(-)