Skip to content
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.

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

Windows: add a manifest to use UTF-8 code pages #1900

Merged
merged 4 commits into from
Jan 8, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
23 changes: 18 additions & 5 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -668,20 +668,27 @@ endif()

if(AVIF_BUILD_APPS)
add_executable(avifenc apps/avifenc.c)
if(WIN32)
vrabaud marked this conversation as resolved.
Show resolved Hide resolved
target_sources(avifenc PRIVATE apps/utf8.rc)
if(NOT MINGW)
target_link_options(avifenc PRIVATE /MANIFEST:NO)
endif()
endif()
if(AVIF_USE_CXX)
set_target_properties(avifenc PROPERTIES LINKER_LANGUAGE "CXX")
endif()
target_link_libraries(avifenc avif_apps)
add_executable(avifdec apps/avifdec.c)
if(WIN32)
target_sources(avifdec PRIVATE apps/utf8.rc)
if(NOT MINGW)
target_link_options(avifdec PRIVATE /MANIFEST:NO)
endif()
endif()
if(AVIF_USE_CXX)
set_target_properties(avifdec PROPERTIES LINKER_LANGUAGE "CXX")
endif()
target_link_libraries(avifdec avif_apps)
if(MINGW AND NOT "$ENV{MSYSTEM}" MATCHES "(MINGW32|MINGW64)")
# MINGW* are legacy environments that link against MSVCRT which does not support UTF8.
target_link_options(avifenc PRIVATE -municode)
target_link_options(avifdec PRIVATE -municode)
endif()

if(NOT SKIP_INSTALL_APPS AND NOT SKIP_INSTALL_ALL)
install(
Expand Down Expand Up @@ -709,6 +716,12 @@ if(AVIF_BUILD_APPS)
)

add_executable(avifgainmaputil "${AVIFGAINMAPUTIL_SRCS}")
if(WIN32)
target_sources(avifgainmaputil PRIVATE apps/utf8.rc)
if(NOT MINGW)
target_link_options(avifgainmaputil PRIVATE /MANIFEST:NO)
endif()
endif()
set_target_properties(avifgainmaputil PROPERTIES LINKER_LANGUAGE "CXX")
target_include_directories(avifgainmaputil PRIVATE apps/avifgainmaputil/)
target_link_libraries(avifgainmaputil libargparse avif_apps)
Expand Down
19 changes: 5 additions & 14 deletions apps/avifdec.c
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,12 @@
#include <stdlib.h>
#include <string.h>

#if defined(_WIN32)
#include <locale.h>
#define WIN32_LEAN_AND_MEAN
#include <windows.h>
#endif

#define DEFAULT_JPEG_QUALITY 90

#define NEXTARG() \
if (((argIndex + 1) == argc) || (argv[argIndex + 1][0] == '-')) { \
fprintf(stderr, "%s requires an argument.", arg); \
goto cleanup; \
return 1; \
} \
arg = argv[++argIndex]

Expand Down Expand Up @@ -59,7 +53,7 @@ static void syntax(void)
avifPrintVersions();
}

MAIN()
int main(int argc, char * argv[])
wantehchang marked this conversation as resolved.
Show resolved Hide resolved
{
const char * inputFilename = NULL;
const char * outputFilename = NULL;
Expand All @@ -77,15 +71,12 @@ MAIN()
uint32_t frameIndex = 0;
uint32_t imageSizeLimit = AVIF_DEFAULT_IMAGE_SIZE_LIMIT;
uint32_t imageDimensionLimit = AVIF_DEFAULT_IMAGE_DIMENSION_LIMIT;
int returnCode = 1;
avifDecoder * decoder = NULL;

if (argc < 2) {
syntax();
return 1;
}

INIT_ARGV()
int argIndex = 1;
while (argIndex < argc) {
const char * arg = argv[argIndex];
Expand Down Expand Up @@ -240,7 +231,7 @@ MAIN()
return 1;
}

decoder = avifDecoderCreate();
avifDecoder * decoder = avifDecoderCreate();
if (!decoder) {
fprintf(stderr, "Memory allocation failure\n");
return 1;
Expand Down Expand Up @@ -318,7 +309,8 @@ MAIN()
jobs,
(jobs == 1) ? "" : "s");

decoder = avifDecoderCreate();
int returnCode = 1;
avifDecoder * decoder = avifDecoderCreate();
if (!decoder) {
fprintf(stderr, "Memory allocation failure\n");
goto cleanup;
Expand Down Expand Up @@ -396,6 +388,5 @@ MAIN()
}
avifDecoderDestroy(decoder);
}
FREE_ARGV()
return returnCode;
}
11 changes: 3 additions & 8 deletions apps/avifenc.c
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,6 @@
// for setmode()
#include <fcntl.h>
#include <io.h>
#include <locale.h>
#define WIN32_LEAN_AND_MEAN
// Avoid the DEFAULT_QUALITY macro redefinition warning caused by including wingdi.h.
#define NOGDI
#include <windows.h>
#endif

#define NEXTARG() \
Expand Down Expand Up @@ -1396,13 +1391,13 @@ static avifBool avifEncodeImages(avifSettings * settings,
return AVIF_TRUE;
}

MAIN()
int main(int argc, char * argv[])
{
if (argc < 2) {
syntaxShort();
return 1;
}
INIT_ARGV()

const char * outputFilename = NULL;

avifInput input;
Expand Down Expand Up @@ -2593,6 +2588,6 @@ MAIN()
avifCodecSpecificOptionsFree(&file->settings.codecSpecificOptions);
}
free(input.files);
FREE_ARGV()

return returnCode;
}
10 changes: 1 addition & 9 deletions apps/avifgainmaputil/avifgainmaputil.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,6 @@
#include "swapbase_command.h"
#include "tonemap_command.h"

#if defined(_WIN32)
#include <locale.h>
#define WIN32_LEAN_AND_MEAN
#include <windows.h>
#endif

namespace avif {
namespace {

Expand Down Expand Up @@ -56,7 +50,7 @@ void PrintUsage(const std::vector<std::unique_ptr<ProgramCommand>>& commands) {
} // namespace
} // namespace avif

MAIN() {
int main(int argc, char** argv) {
std::vector<std::unique_ptr<avif::ProgramCommand>> commands;
commands.emplace_back(std::make_unique<avif::HelpCommand>());
commands.emplace_back(std::make_unique<avif::CombineCommand>());
Expand All @@ -72,8 +66,6 @@ MAIN() {
return 1;
}

INIT_ARGV()

const std::string command_name(argv[1]);
if (command_name == "help") {
if (argc >= 3) {
Expand Down
61 changes: 0 additions & 61 deletions apps/shared/avifutil.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,67 +10,6 @@
extern "C" {
#endif

// MAIN(), INIT_ARGV(), FREE_ARGV() for UTF8-aware command line parsing.
// UCRT supports UTF8, contrary to MSVCRT.
#if defined(_WIN32) && defined(_UCRT)
#define MAIN() int wmain(int argc, wchar_t * wargv[])
#else
#define MAIN() int main(int argc, char * argv[])
#endif

#if defined(_WIN32) && defined(_UCRT)
#ifdef __cplusplus
#define INIT_ARGV() \
if (setlocale(LC_ALL, ".UTF8") == NULL) { \
fprintf(stderr, "setlocale failed\n"); \
return 1; \
} \
std::vector<char> argvAllVector(1024 * argc); \
std::vector<char *> argvVector(argc); \
char ** argv = argvVector.data(); \
for (int i = 0; i < argc; ++i) { \
argvVector[i] = &argvAllVector[1024 * i]; \
int rc = WideCharToMultiByte(CP_UTF8, WC_ERR_INVALID_CHARS, wargv[i], -1, argv[i], 1024, NULL, NULL); \
if (rc == 0) { \
fprintf(stderr, "WideCharToMultiByte() failed\n"); \
return 1; \
} \
}
#else
#define INIT_ARGV() \
char * argvAll = NULL; \
char ** argv = NULL; \
if (setlocale(LC_ALL, ".UTF8") == NULL) { \
fprintf(stderr, "setlocale failed\n"); \
return 1; \
} \
argvAll = (char *)malloc(1024 * argc * sizeof(*argvAll)); \
argv = (char **)malloc(argc * sizeof(*argv)); \
if (argv == NULL || argvAll == NULL) { \
FREE_ARGV() \
return 1; \
} \
for (int i = 0; i < argc; ++i) { \
argv[i] = argvAll + 1024 * i; \
int rc = WideCharToMultiByte(CP_UTF8, WC_ERR_INVALID_CHARS, wargv[i], -1, argv[i], 1024, NULL, NULL); \
if (rc == 0) { \
fprintf(stderr, "WideCharToMultiByte() failed\n"); \
FREE_ARGV() \
return 1; \
} \
}
#endif // __cplusplus
#else
#define INIT_ARGV()
#endif
#if defined(_WIN32) && defined(_UCRT) && !defined(__cplusplus)
#define FREE_ARGV() \
free(argv); \
free(argvAll);
#else
#define FREE_ARGV()
#endif

// The %z format specifier is not available in the old Windows CRT msvcrt,
// hence the %I format specifier must be used instead to print out `size_t`.
// The new Windows CRT UCRT, which is used by Visual Studio 2015 or later,
Expand Down
8 changes: 8 additions & 0 deletions apps/utf8.manifest
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
<assembly manifestVersion="1.0" xmlns="urn:schemas-microsoft-com:asm.v1">
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I copied this file from https://learn.microsoft.com/en-us/windows/apps/design/globalizing/use-utf8-code-page#examples. The original file has this line:

  <assemblyIdentity type="win32" name="..." version="6.0.0.0"/>

I noticed that the name "..." and version "6.0.0.0" in this line look like placeholders. I think we are supposed to replace them with the name and version of avifdec, avifenc, and are_images_equal.

According to Microsoft documentation, the assemblyIdentity element is required in an application manifest:
https://learn.microsoft.com/en-us/windows/win32/sbscs/application-manifests

However, I deleted this line and our tests still pass. So I suggest we try going without the assemblyIdentity element.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thx for clarifying

<application>
<windowsSettings>
<activeCodePage xmlns="http://schemas.microsoft.com/SMI/2019/WindowsSettings">UTF-8</activeCodePage>
</windowsSettings>
</application>
</assembly>
3 changes: 3 additions & 0 deletions apps/utf8.rc
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
#include <winuser.h>

CREATEPROCESS_MANIFEST_RESOURCE_ID RT_MANIFEST "utf8.manifest"
15 changes: 6 additions & 9 deletions tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -226,19 +226,16 @@ if(AVIF_BUILD_APPS)
# When building apps, test the avifenc/avifdec.
# 'are_images_equal' is used to make sure inputs/outputs are unchanged.
add_executable(are_images_equal gtest/are_images_equal.cc)
if(WIN32)
target_sources(are_images_equal PRIVATE ${CMAKE_SOURCE_DIR}/apps/utf8.rc)
if(NOT MINGW)
target_link_options(are_images_equal PRIVATE /MANIFEST:NO)
endif()
endif()
target_link_libraries(are_images_equal aviftest_helpers)
add_test(NAME test_cmd COMMAND bash ${CMAKE_CURRENT_SOURCE_DIR}/test_cmd.sh ${CMAKE_BINARY_DIR}
${CMAKE_CURRENT_SOURCE_DIR}/data
)
set_tests_properties(test_cmd PROPERTIES ENVIRONMENT "AVIF_TEST_UTF8=1")
if(MINGW)
if("$ENV{MSYSTEM}" MATCHES "(MINGW32|MINGW64)")
# Those legacy environments link against MSVCRT which does not support UTF8.
set_tests_properties(test_cmd PROPERTIES ENVIRONMENT "AVIF_TEST_UTF8=0")
else()
target_link_options(are_images_equal PRIVATE -municode)
endif()
endif()
add_test(NAME test_cmd_animation COMMAND bash ${CMAKE_CURRENT_SOURCE_DIR}/test_cmd_animation.sh ${CMAKE_BINARY_DIR}
${CMAKE_CURRENT_SOURCE_DIR}/data
)
Expand Down
12 changes: 1 addition & 11 deletions tests/gtest/are_images_equal.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,9 @@
#include "aviftest_helpers.h"
#include "avifutil.h"

#if defined(_WIN32)
#include <locale.h>
#define WIN32_LEAN_AND_MEAN
#include <windows.h>
#endif

using avif::ImagePtr;

MAIN() {
INIT_ARGV()

int main(int argc, char** argv) {
if (argc != 4 && argc != 5) {
std::cerr << "Wrong argument: " << argv[0]
<< " file1 file2 ignore_alpha_flag [psnr_threshold]" << std::endl;
Expand Down Expand Up @@ -79,7 +71,5 @@ MAIN() {
<< argv[2] << " are similar." << std::endl;
}

FREE_ARGV()

return 0;
}
16 changes: 7 additions & 9 deletions tests/test_cmd.sh
Original file line number Diff line number Diff line change
Expand Up @@ -71,15 +71,13 @@ pushd ${TMP_DIR}
"${AVIFENC}" -s 8 "${INPUT_Y4M}" -o "${ENCODED_FILE}"
"${AVIFDEC}" "${ENCODED_FILE}" "${DECODED_FILE}"
"${ARE_IMAGES_EQUAL}" "${INPUT_Y4M}" "${DECODED_FILE}" 0 && exit 1
if [[ "${AVIF_TEST_UTF8}" == "1" ]]; then
cp ${INPUT_Y4M} ${INPUT_UTF8_Y4M}
"${AVIFENC}" -s 8 "${INPUT_UTF8_Y4M}" -o "${ENCODED_UTF8_FILE}"
"${AVIFDEC}" "${ENCODED_UTF8_FILE}" "${DECODED_UTF8_FILE}"
RET=0
"${ARE_IMAGES_EQUAL}" "${INPUT_UTF8_Y4M}" "${DECODED_UTF8_FILE}" 0 || RET=$?
if [[ ${RET} -ne 1 ]]; then
exit 1
fi
cp ${INPUT_Y4M} ${INPUT_UTF8_Y4M}
"${AVIFENC}" -s 8 "${INPUT_UTF8_Y4M}" -o "${ENCODED_UTF8_FILE}"
"${AVIFDEC}" "${ENCODED_UTF8_FILE}" "${DECODED_UTF8_FILE}"
RET=0
"${ARE_IMAGES_EQUAL}" "${INPUT_UTF8_Y4M}" "${DECODED_UTF8_FILE}" 0 || RET=$?
if [[ ${RET} -ne 1 ]]; then
exit 1
fi

# Argument parsing test with filenames starting with a dash.
Expand Down
Loading