fix(build): replace im2d.hpp with im2d.h to fix RGA compilation on RK3588#334
fix(build): replace im2d.hpp with im2d.h to fix RGA compilation on RK3588#334flyw wants to merge 1 commit intoHyperInspire:masterfrom
Conversation
Compile failed when enabling RGA on Android (RK3588). The issue was caused by including `im2d.hpp` which could not be resolved. Changing the include to `im2d.h` fixes the build issue. Tested on: Android NDK for Rockchip 3588.
There was a problem hiding this comment.
PR Summary:
This PR attempts to fix a compilation error when building the Android NDK version for RK3588 with RGA enabled by changing the include from im2d.hpp (C++ header) to im2d.h (C header). The author reports that the RGA library in the Android NDK environment for RK3588 only provides the C header rather than the C++ header.
Review Summary:
I've identified a critical API compatibility issue with this change. The existing code on line 129 calls wrapbuffer_handle(handle, w, h, RK_FORMAT_RGB_888) with only 4 arguments, which relies on C++ default parameters available in im2d.hpp. The C header im2d.h requires all 6 parameters explicitly: wrapbuffer_handle(handle, width, height, format, wstride, hstride).
My research of the Rockchip librga library confirms that while both headers are standard in the upstream repositories, the C++ version provides default arguments for stride parameters that the C version does not. This change should cause a compilation error unless the function call on line 129 is also updated to include the stride parameters, or the precompiled librga in the build environment has non-standard function signatures.
Knowledge utilized: Rockchip RGA library API documentation, librga repository structure, CMake build system configuration for RK3588 platform, and hardware acceleration backend integration patterns from the InspireFace codebase.
Follow-up suggestions:
@devloaiverify the compilation status and update line 129 if needed@devloaicheck if im2d.hpp can be added to the precompiled librga package to avoid this workaround
| #include <memory> | ||
| #include <unordered_map> | ||
| #include "im2d.hpp" | ||
| #include "im2d.h" |
There was a problem hiding this comment.
Critical Issue: This change from im2d.hpp (C++) to im2d.h (C) introduces an API compatibility problem that should cause compilation failure.
Root Problem: Line 129 in this file uses:
buffer = wrapbuffer_handle(handle, w, h, RK_FORMAT_RGB_888);This 4-argument call requires C++ default parameters from im2d.hpp. The C header im2d.h requires all 6 parameters: wrapbuffer_handle(handle, width, height, format, wstride, hstride).
Research findings:
- Standard Rockchip librga repositories (rockchip-linux/librga, airockchip/librga) provide both headers
- For C++ projects,
im2d.hppis the recommended choice - The C++ header offers default arguments and better type safety
Recommended Solution:
Option A (Preferred): Fix the build environment to include im2d.hpp
- Revert this change back to
#include "im2d.hpp" - Ensure the librga package in your Android NDK includes C++ headers
- This maintains the intended API usage
Option B: If im2d.hpp is truly unavailable, fix the function call:
- Keep
#include "im2d.h" - Update line 129 to:
buffer = wrapbuffer_handle(handle, w, h, RK_FORMAT_RGB_888, w, h); - Add a comment explaining the environment constraint
Verification Request:
Please confirm that the build actually passes with this change alone. The code should fail to compile without fixing line 129. If it compiles, your librga version may have non-standard function signatures.
actions
Feedback: Rate this comment to help me improve future code reviews:
- 👍 Good - Helpful and accurate
- 👎 Poor - Wrong, unclear, or unhelpful
- Skip if you don't have any strong opinions either way.
Description
This PR fixes a compilation error encountered when building the Android NDK version for RK3588 with RGA enabled.
Context
When
IS_RGAis enabled, the build fails becauseim2d.hppis included inimage_processor_rga.h. It seems that the RGA library (librga) available in the NDK environment for RK3588 providesim2d.hinstead of the C++ headerim2d.hpp.Changes
cpp/inspireface/image_process/nexus_processor/image_processor_rga.h: Changed#include "im2d.hpp"to#include "im2d.h".Verification