Fix RGB24↔BGR24 conversion issues and AVX2 crash

src/ccap_convert_avx2.cpp

@@ -133,8 +133,8 @@ AVX2_TARGET void colorShuffle_avx2(const uint8_t* src, int srcStride, uint8_t* d
     }
 
     alignas(32) uint8_t shuffleData[32];
-    constexpr uint32_t inputPatchSize = inputChannels == 4 ? 8 : 10;
-    constexpr uint32_t outputPatchSize = outputChannels == 4 ? 8 : 10;
+    constexpr uint32_t inputPatchSize = inputChannels == 4 ? 8 : (inputChannels == 3 && outputChannels == 3 ? 5 : 10);
+    constexpr uint32_t outputPatchSize = outputChannels == 4 ? 8 : (inputChannels == 3 && outputChannels == 3 ? 5 : 10);
     constexpr uint32_t patchSize = inputPatchSize < outputPatchSize ? inputPatchSize : outputPatchSize;
 
     for (int i = 0; i < patchSize; ++i) {
@@ -218,17 +218,12 @@ AVX2_TARGET void colorShuffle_avx2(const uint8_t* src, int srcStride, uint8_t* d
                 _mm_store_si128((__m128i*)remainBuffer, result_hi);           // Temporarily store, 16 bytes
                 memcpy(dstRow + x * outputChannels + 12, remainBuffer, 12);   // Manual alignment, overwrite extra 4 bytes, fill remaining 12 bytes, exactly 24 bytes
             } else if constexpr (inputChannels == 3 && outputChannels == 3) { // 3 -> 3
-                /// Split into 15 + 15, reading 30 bytes each time
-                __m128i pixels_lo = _mm_loadu_si128((__m128i*)(srcRow + x * inputChannels));
-                __m128i pixels_hi = _mm_loadu_si128((__m128i*)(srcRow + x * inputChannels + 15));
+                /// Process 5 pixels at a time (15 bytes), reading 16 bytes each time
+                __m128i pixels = _mm_loadu_si128((__m128i*)(srcRow + x * inputChannels));
 
-                __m128i result_lo = _mm_shuffle_epi8(pixels_lo, shuffle128); // Only the first 15 bytes are useful
-                __m128i result_hi = _mm_shuffle_epi8(pixels_hi, shuffle128); // Only the first 15 bytes are useful
+                __m128i result = _mm_shuffle_epi8(pixels, shuffle128); // Only the first 15 bytes are useful
 
-                _mm_storeu_si128((__m128i*)(dstRow + x * outputChannels), result_lo); // Write 16 bytes, but only the first 15 bytes are useful
-                alignas(16) uint8_t remainBuffer[16];
-                _mm_store_si128((__m128i*)remainBuffer, result_hi);         // Temporarily store, 15 bytes
-                memcpy(dstRow + x * outputChannels + 15, remainBuffer, 15); // Manual alignment, overwrite extra 1 byte, fill remaining 15 bytes, exactly 30 bytes
+                _mm_storeu_si128((__m128i*)(dstRow + x * outputChannels), result); // Write 16 bytes, but only the first 15 bytes are useful
             } else {                                                        // 4 -> 4
                 __m256i pixels = _mm256_loadu_si256((const __m256i*)(srcRow + x * inputChannels));
                 __m256i result = _mm256_shuffle_epi8(pixels, shuffle256);

src/ccap_convert_frame.cpp

@@ -19,6 +19,12 @@ bool inplaceConvertFrameYUV2RGBColor(VideoFrame* frame, PixelFormat toFormat, bo
 
     /// TODO: Fix toFormat here, only support YUV -> (BGR24/BGRA32). Simplify SDK design. Will improve later.
 
+    // ASSERTION: Ensure frame->data[0] points to EXTERNAL memory, not allocator->data()
+    // This validates the design constraint: VideoFrame should only be converted once
+    assert(frame->allocator == nullptr || frame->data[0] != frame->allocator->data() && 
+           "DESIGN VIOLATION: frame->data[0] must point to external memory (e.g., camera buffer), not allocator memory. "
+           "Each VideoFrame should only be converted ONCE using inplaceConvertFrame*() functions.");
+
     auto inputFormat = frame->pixelFormat;
     assert((inputFormat & kPixelFormatYUVColorBit) != 0 && (toFormat & kPixelFormatYUVColorBit) == 0);
     bool isInputNV12 = pixelFormatInclude(inputFormat, PixelFormat::NV12);
@@ -122,6 +128,12 @@ bool inplaceConvertFrameYUV2RGBColor(VideoFrame* frame, PixelFormat toFormat, bo
 
 bool inplaceConvertFrameRGB(VideoFrame* frame, PixelFormat toFormat, bool verticalFlip) {
     // RGB(A) interconversion
+
+    // ASSERTION: Ensure frame->data[0] points to EXTERNAL memory, not allocator->data()
+    // This validates the design constraint: VideoFrame should only be converted once
+    assert(frame->allocator == nullptr || frame->data[0] != frame->allocator->data() && 
+           "DESIGN VIOLATION: frame->data[0] must point to external memory (e.g., camera buffer), not allocator memory. "
+           "Each VideoFrame should only be converted ONCE using inplaceConvertFrame*() functions.");
 
     uint8_t* inputBytes = frame->data[0];
     int inputLineSize = frame->stride[0];
@@ -157,7 +169,7 @@ bool inplaceConvertFrameRGB(VideoFrame* frame, PixelFormat toFormat, bool vertic
 #endif
         } else // RGB <-> BGR
         {
-            rgbaToBgra(inputBytes, inputLineSize, outputBytes, newLineSize, frame->width, height);
+            rgbToBgr(inputBytes, inputLineSize, outputBytes, newLineSize, frame->width, height);
         }
     } else /// Different number of channels, only 4 channels <-> 3 channels
     {
@@ -181,6 +193,12 @@ bool inplaceConvertFrameRGB(VideoFrame* frame, PixelFormat toFormat, bool vertic
 }
 
 inline bool inplaceConvertFrameImp(VideoFrame* frame, PixelFormat toFormat, bool verticalFlip) {
+    // ASSERTION: Ensure frame->data[0] points to EXTERNAL memory, not allocator->data()
+    // This validates the design constraint: VideoFrame should only be converted once
+    assert(frame->allocator == nullptr || frame->data[0] != frame->allocator->data() && 
+           "DESIGN VIOLATION: frame->data[0] must point to external memory (e.g., camera buffer), not allocator memory. "
+           "Each VideoFrame should only be converted ONCE using inplaceConvertFrame*() functions.");
+
     if (frame->pixelFormat == toFormat) {
         if (verticalFlip && (toFormat & kPixelFormatRGBColorBit)) { // flip upside down
             int srcStride = (int)frame->stride[0];

src/ccap_convert_frame.h

@@ -12,14 +12,36 @@
 
 #include "ccap_def.h"
 
-/// The methods here require that the data field of frame is not allocated with an allocator.
-/// This method will use an allocator to allocate memory and convert to a new data format.
+/// @brief Inplace frame conversion functions
+/// 
+/// IMPORTANT CONSTRAINTS:
+/// - These methods require that frame->data[0] points to EXTERNAL memory (e.g., camera buffer)
+///   and NOT to frame->allocator->data()
+/// - Each VideoFrame should only be converted ONCE using these functions
+/// - The functions will allocate new memory via frame->allocator and update frame->data[0]
+/// 
+/// TYPICAL USAGE:
+/// 1. Capture frame from camera: frame->data[0] points to camera's buffer
+/// 2. Call inplaceConvertFrame*() ONCE: converts and moves data to allocator
+/// 3. After conversion: frame->data[0] == frame->allocator->data()
+/// 
+/// VIOLATION will cause:
+/// - Data corruption (reading freed memory)
+/// - Assertion failure in debug builds
+/// - Undefined behavior
+
+// Export internal functions only when building tests
+#ifdef CCAP_BUILD_TESTS
+    #define CCAP_TEST_EXPORT CCAP_EXPORT
+#else
+    #define CCAP_TEST_EXPORT
+#endif
 
 namespace ccap {
 
-bool inplaceConvertFrame(VideoFrame* frame, PixelFormat toFormat, bool verticalFlip);
-bool inplaceConvertFrameRGB(VideoFrame* frame, PixelFormat toFormat, bool verticalFlip);
-bool inplaceConvertFrameYUV2RGBColor(VideoFrame* frame, PixelFormat toFormat, bool verticalFlip);
+CCAP_TEST_EXPORT bool inplaceConvertFrame(VideoFrame* frame, PixelFormat toFormat, bool verticalFlip);
+CCAP_TEST_EXPORT bool inplaceConvertFrameRGB(VideoFrame* frame, PixelFormat toFormat, bool verticalFlip);
+CCAP_TEST_EXPORT bool inplaceConvertFrameYUV2RGBColor(VideoFrame* frame, PixelFormat toFormat, bool verticalFlip);
 
 } // namespace ccap
 

tests/CMakeLists.txt

@@ -72,6 +72,7 @@ add_executable(
     test_color_conversions.cpp
     test_yuv_conversions.cpp
     test_platform_features.cpp
+    test_frame_conversions.cpp
 )
 
 target_link_libraries(
@@ -80,6 +81,13 @@ target_link_libraries(
     gtest_main
 )
 
+# Add src directory to include path for internal API testing
+target_include_directories(
+    ccap_convert_test
+    PRIVATE
+    ${CMAKE_SOURCE_DIR}/src
+)
+
 # GUID definition verification test (Windows only)
 # This test verifies that locally defined GUIDs match strmiids.lib values
 if(WIN32)