google-ai-edge / mediapipe

Cross-platform, customizable ML solutions for live and streaming media.
https://ai.google.dev/edge/mediapipe
Apache License 2.0
27.72k stars 5.18k forks source link

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

Open Malcolmnixon opened 6 months ago

Malcolmnixon commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

@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 commented 6 months ago

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 https://github.com/purgeme/mediapipe_cpp_lib/issues/4#issuecomment-1520447063

GeorgeS2019 commented 6 months ago

@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

https://github.com/google/mediapipe/pull/5392#issuecomment-2104098525

GeorgeS2019 commented 6 months ago

https://github.com/google/mediapipe/commit/c09d38cc496f5d22fb9d23b0eadc21fbc2658b63

The needed change passed. Thx to Google team and Malcolm

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