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

Use of preprocessor directives in function-like macros is undefined #5366

Open
Malcolmnixon opened this issue May 1, 2024 · 5 comments
Open
Assignees
Labels
os:windows MediaPipe issues on Windows platform:c++ Issues specific to C++ framework in mediapipe stat:awaiting googler Waiting for Google Engineer's Response task::all All tasks of MediaPipe type:build/install For Build and Installation issues

Comments

@Malcolmnixon
Copy link

Malcolmnixon commented May 1, 2024

OS Platform and Distribution

Windows 11

Compiler version

Visual Studio 2019

Programming Language and version

C++

Installed using virtualenv? pip? Conda?(if python)

No response

MediaPipe version

0.10.11

Bazel version

No response

XCode and Tulsi versions (if iOS)

No response

Android SDK and NDK versions (if android)

No response

Android AAR (if android)

None

OpenCV version (if running on desktop)

3.4.10

Describe the problem

Compile error when building

Complete Logs

This occurs after upgrading from MediaPip 0.10.9 to 0.10.11:
.\mediapipe/tasks/cc/core/task_api_factory.h(86): error C2121: '#': invalid character: possibly the result of a macro expansion

The problem is caused by the following code:

    MP_ASSIGN_OR_RETURN(
        auto runner,
#if !MEDIAPIPE_DISABLE_GPU
        core::TaskRunner::Create(std::move(graph_config), std::move(resolver),
                                 std::move(packets_callback),
                                 std::move(default_executor),
                                 std::move(input_side_packets),
                                 /*resources=*/nullptr, std::move(error_fn)));
#else
        core::TaskRunner::Create(
            std::move(graph_config), std::move(resolver),
            std::move(packets_callback), std::move(default_executor),
            std::move(input_side_packets), std::move(error_fn)));
#endif

MP_ASSIGN_OR_RETURN(...) is a variadic function-like macro; and this code attempts to use #if / #else / #endif inside the invocation of this function-like macro. The C++ specification states this results in undefined behavior.

If there are sequences of preprocessing tokens within the list of arguments that would otherwise act as preprocessing directives, the behavior is undefined.

As MediaPipe is a cross-platform library, it may be beneficial to limit code to that defined by the C++ specification for maximum portability.

@Malcolmnixon Malcolmnixon added the type:build/install For Build and Installation issues label May 1, 2024
@Malcolmnixon
Copy link
Author

The solution in this case should simply be to move the two lines of MP_ASSIGN_OR_RETURN inside the preprocessor conditionals as follows:

#if !MEDIAPIPE_DISABLE_GPU
    MP_ASSIGN_OR_RETURN(
        auto runner,
        core::TaskRunner::Create(std::move(graph_config), std::move(resolver),
                                 std::move(packets_callback),
                                 std::move(default_executor),
                                 std::move(input_side_packets),
                                 /*resources=*/nullptr, std::move(error_fn)));
#else
    MP_ASSIGN_OR_RETURN(
        auto runner,
        core::TaskRunner::Create(
            std::move(graph_config), std::move(resolver),
            std::move(packets_callback), std::move(default_executor),
            std::move(input_side_packets), std::move(error_fn)));
#endif

@GeorgeS2019
Copy link

GeorgeS2019 commented May 1, 2024

@andrechen
@kuaashish
@lu-wang-g

Progress Update: Godot 4 Mediapiple Addon : Windows build v.0.10.11

@Malcolmnixon is attempting to build for Windows using Github Action => Mediapipe v.0.10.11

Hope we can get some feedback how to build for Windows in reliable way and to coordinate CI/CD build pipeline for Windows

Potential related link

breaking MediaPipe's Python build for Windows

Progress Update for [.NET wrapper] for Mediapipe v0.10.09

Here is a tentative example

The Godot community is pushing for .NET wrapper for v0.10.11. We need your feedback.

Referenced Issues which can be closed now or had been closed

Future direction: v0.10.12

@GeorgeS2019
Copy link

GeorgeS2019 commented May 1, 2024

godot_mediapipe_module

@purgeme
godot_mediapipe_module

We want to upgrade Godot Mediapipe to the latest version.
Have a look to see if you could feedback how to address this block when building the Windows version
purgeme/mediapipe_cpp_lib#4 (comment)

@kuaashish kuaashish assigned kuaashish and unassigned ayushgdev May 2, 2024
@kuaashish kuaashish added os:windows MediaPipe issues on Windows platform:c++ Issues specific to C++ framework in mediapipe task::all All tasks of MediaPipe labels May 2, 2024
@GeorgeS2019
Copy link

GeorgeS2019 commented May 4, 2024

@kuaashish
The Godot community, top 8th in entire GitHub, wants to use the latest version seriously, by successfully setting up GitHub action to support the .Net access to the latest Mediapipe version

Do consider officially support windows ...removing the statement that it is experimental support and provide .Net examples through Godot

May 2024 Requests from Unity and Godot communities

#5392 (comment)

@GeorgeS2019
Copy link

GeorgeS2019 commented May 8, 2024

c09d38c

The needed change passed. Thx to Google team and Malcolm

Screenshot_2024-05-08-06-38-19-590-edit_com microsoft emmx

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
os:windows MediaPipe issues on Windows platform:c++ Issues specific to C++ framework in mediapipe stat:awaiting googler Waiting for Google Engineer's Response task::all All tasks of MediaPipe type:build/install For Build and Installation issues
Projects
None yet
Development

No branches or pull requests

5 participants