Skip to content

MSVC: Miscellaneous uncontroversial fixes #1422

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sethk wants to merge 8 commits into
base: master
Choose a base branch
from
Open

MSVC: Miscellaneous uncontroversial fixes #1422

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
d995910
MSVC: Miscellaneous uncontroversial fixes
sethk Mar 19, 2025
1b0661f
Do a header check for corecrt_io.h instead of assuming it always exis…
sethk Apr 1, 2025
8d65d10
Use synchapi.h to get Win32 API function Sleep()
sethk Apr 1, 2025
816b808
Explicitly check for mode_t and struct timespec when configuring.
sethk Jul 20, 2025
2222c58
MSVC: synchapi.h can't be used standalone, so use lean and mean windo…
sethk Apr 7, 2025
db54147
Fix #include for mode_t check.
sethk Apr 17, 2025
913b85b
<malloc.h>/<alloca.h> are not used on this branch; remove their tests…
sethk Jul 20, 2025
30e2de5
Add comment about purpose of <direct.h>
sethk Jul 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions libvmaf/meson.build
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,16 @@ elif host_machine.system() == 'darwin'
endif

# Header checks
if cc.has_header('unistd.h', args: test_args)
add_project_arguments('-DHAVE_UNISTD_H', language: ['c', 'cpp'])
endif
if cc.has_header('direct.h', args: test_args)
add_project_arguments('-DHAVE_DIRECT_H', language: ['c', 'cpp'])
endif
if cc.has_header('corecrt_io.h', args: test_args)
add_project_arguments('-DHAVE_CORECRT_IO_H', language: ['c', 'cpp'])
endif

stdatomic_dependency = []
if not cc.check_header('stdatomic.h')
if cc.get_id() == 'msvc'
Expand All @@ -46,6 +56,15 @@ if not cc.check_header('stdatomic.h')
endif
endif

# Type checks
if cc.has_type('struct timespec', prefix: '#include <time.h>')
# Used by pthreads-win32 <pthread.h>
add_project_arguments('-DHAVE_STRUCT_TIMESPEC', language: ['c', 'cpp'])
endif
if cc.has_type('mode_t', prefix: '#include <sys/types.h>')
add_project_arguments('-DHAVE_MODE_T', language: ['c', 'cpp'])
endif

subdir('include')
subdir('src')
subdir('tools')
Expand Down
13 changes: 13 additions & 0 deletions libvmaf/src/feature/integer_adm.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,19 @@
#include <arm_neon.h>
#endif

#ifdef _MSC_VER
#include <intrin.h>

static inline int __builtin_clz(unsigned x) {
return (int)__lzcnt(x);
}

static inline int __builtin_clzll(unsigned long long x) {
return (int)__lzcnt64(x);
}

#endif

typedef struct AdmState {
size_t integer_stride;
AdmBuffer buf;
Expand Down
30 changes: 15 additions & 15 deletions libvmaf/src/feature/integer_vif.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -626,27 +626,27 @@ static int init(VmafFeatureExtractor *fex, enum VmafPixelFormat pix_fmt,
const size_t data_sz =
2 * (pad_size + frame_size + pad_size) + 2 * (h * s->public.buf.stride_16) +
5 * (s->public.buf.stride_32) + 7 * s->public.buf.stride_tmp;
void *data = aligned_malloc(data_sz, MAX_ALIGN);
char *data = aligned_malloc(data_sz, MAX_ALIGN);
if (!data) return -ENOMEM;
memset(data, 0, data_sz);

s->public.buf.data = data; data += pad_size;
s->public.buf.ref = data; data += frame_size + pad_size + pad_size;
s->public.buf.dis = data; data += frame_size + pad_size;
s->public.buf.mu1 = data; data += h * s->public.buf.stride_16;
s->public.buf.mu2 = data; data += h * s->public.buf.stride_16;
s->public.buf.mu1_32 = data; data += s->public.buf.stride_32;
s->public.buf.mu2_32 = data; data += s->public.buf.stride_32;
s->public.buf.ref_sq = data; data += s->public.buf.stride_32;
s->public.buf.dis_sq = data; data += s->public.buf.stride_32;
s->public.buf.ref_dis = data; data += s->public.buf.stride_32;
s->public.buf.tmp.mu1 = data; data += s->public.buf.stride_tmp;
s->public.buf.tmp.mu2 = data; data += s->public.buf.stride_tmp;
s->public.buf.tmp.ref = data; data += s->public.buf.stride_tmp;
s->public.buf.tmp.dis = data; data += s->public.buf.stride_tmp;
s->public.buf.tmp.ref_dis = data; data += s->public.buf.stride_tmp;
s->public.buf.tmp.ref_convol = data; data += s->public.buf.stride_tmp;
s->public.buf.tmp.dis_convol = data;
s->public.buf.mu1 = (uint16_t *)data; data += h * s->public.buf.stride_16;
s->public.buf.mu2 = (uint16_t *)data; data += h * s->public.buf.stride_16;
s->public.buf.mu1_32 = (uint32_t *)data; data += s->public.buf.stride_32;
s->public.buf.mu2_32 = (uint32_t *)data; data += s->public.buf.stride_32;
s->public.buf.ref_sq = (uint32_t *)data; data += s->public.buf.stride_32;
s->public.buf.dis_sq = (uint32_t *)data; data += s->public.buf.stride_32;
s->public.buf.ref_dis = (uint32_t *)data; data += s->public.buf.stride_32;
s->public.buf.tmp.mu1 = (uint32_t *)data; data += s->public.buf.stride_tmp;
s->public.buf.tmp.mu2 = (uint32_t *)data; data += s->public.buf.stride_tmp;
s->public.buf.tmp.ref = (uint32_t *)data; data += s->public.buf.stride_tmp;
s->public.buf.tmp.dis = (uint32_t *)data; data += s->public.buf.stride_tmp;
s->public.buf.tmp.ref_dis = (uint32_t *)data; data += s->public.buf.stride_tmp;
s->public.buf.tmp.ref_convol = (uint32_t *)data; data += s->public.buf.stride_tmp;
s->public.buf.tmp.dis_convol = (uint32_t *)data;

s->feature_name_dict =
vmaf_feature_name_dict_from_provided_features(fex->provided_features,
Expand Down
5 changes: 5 additions & 0 deletions libvmaf/src/feature/mkdirp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,12 @@
// MIT licensed
//

#ifdef HAVE_DIRECT_H
#include <direct.h> // mkdir()
#endif
#ifdef HAVE_UNISTD_H
#include <unistd.h>
#endif
#include <errno.h>
#include <stdlib.h>
#include <string.h>
Expand Down
4 changes: 4 additions & 0 deletions libvmaf/src/feature/mkdirp.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@
#include <sys/types.h>
#include <sys/stat.h>

#if !defined(HAVE_MODE_T)
typedef unsigned short mode_t;
#endif

/*
* Recursively `mkdir(path, mode)`
*/
Expand Down
5 changes: 5 additions & 0 deletions libvmaf/src/log.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,12 @@
#include "libvmaf/libvmaf.h"

#include <stdarg.h>
#ifdef HAVE_CORECRT_IO_H
#include <corecrt_io.h> // isatty()
#endif
#if HAVE_UNISTD_H
#include <unistd.h>
#endif

static enum VmafLogLevel vmaf_log_level = VMAF_LOG_LEVEL_INFO;
static int istty = 0;
Expand Down
6 changes: 4 additions & 2 deletions libvmaf/test/test_framesync.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,10 @@
#include <stdint.h>
#include <string.h>
#ifdef _WIN32
#include <windows.h>
#else
#define WIN32_LEAN_AND_MEAN
#include <windows.h> // Sleep()
#endif
#ifdef HAVE_UNISTD_H
#include <unistd.h>
#endif

Expand Down
5 changes: 5 additions & 0 deletions libvmaf/tools/vmaf.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
#include <stdlib.h>
#include <string.h>
#include <time.h>
#ifdef HAVE_CORECRT_IO_H
#include <corecrt_io.h> // isatty()
#endif
#ifdef HAVE_UNISTD_H
#include <unistd.h>
#endif
Copy link
Collaborator

@kylophone kylophone Apr 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are guarding all of these headers, but if you don't have any of these the code will fail to compile. Is that right?

Copy link
Contributor Author

@sethk sethk Jul 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry about the massive delay getting back to this, had to ship a demo.

Different combinations of headers are required for different runtimes. Most systems put many things into unistd.h, and then there's MSVC/UCRT, which doesn't have unistd.h, but instead isatty() and mkdir() are provided by corecrt_io.h and direct.h, respectively. MINGW64 defines _WIN32 but has unistd.h and lacks corecrt_io.h, meaning that we can't just use #ifdef _WIN32 to guard that. These header tests seem like the simplest way to address the full build matrix.

I have another change that checks for malloc.h/alloca.h so it can use alloca() in place of variable-length arrays. I separated it from this one because I think it could be more controversial.

Copy link
Contributor Author

@sethk sethk Jul 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgot to mention: I revised this patch with some comments to clarify why each header is used. It's true that I could have done:

#ifdef HAVE_OTHER_HEADER_H
#include <other_header.h> // foo()
#elif defined(HAVE_UNISTD_H)
#include <unistd.h>
#else
#error "Don't know which header defines foo() on this system"
#endif

But unistd.h also has a ton of other stuff, and the pattern I used is one I've seen in many autoconf-based projects.


#include "cli_parse.h"
#include "spinner.h"
Expand Down
Loading