Fix RGB24↔BGR24 conversion issues and AVX2 crash

.github/workflows/linux-build.yml

@@ -140,6 +140,7 @@ jobs:
         if [ "${{ matrix.library_type }}" = "shared" ]; then
           export LD_LIBRARY_PATH="$PWD/../build/${{ matrix.build_type }}-${{ matrix.library_type }}:$LD_LIBRARY_PATH"
         fi
+        # Run tests with AddressSanitizer enabled by default (improves memory error detection)
         ./run_tests.sh --functional --exit-when-failed
 
     - name: Upload artifacts
@@ -239,6 +240,8 @@ jobs:
         if [ "${{ matrix.library_type }}" = "shared" ]; then
           export LD_LIBRARY_PATH="$PWD/../build/${{ matrix.build_type }}-${{ matrix.library_type }}:$LD_LIBRARY_PATH"
         fi
+        # Run tests with AddressSanitizer enabled by default
+        # This helps catch memory errors early in CI
         ./run_tests.sh --functional --exit-when-failed
 
     - name: Upload artifacts
@@ -448,6 +451,8 @@ jobs:
         if [ "${{ matrix.library_type }}" = "shared" ]; then
           export LD_LIBRARY_PATH="$PWD/../build/${{ matrix.build_type }}-${{ matrix.library_type }}:$LD_LIBRARY_PATH"
         fi
+        # Run tests with AddressSanitizer enabled by default
+        # This helps catch memory errors early in CI
         ./run_tests.sh --functional --exit-when-failed
 
     - name: Upload artifacts

.github/workflows/macos-build.yml

@@ -105,6 +105,7 @@ jobs:
           if [ "${{ matrix.library_type }}" = "shared" ]; then
             export DYLD_LIBRARY_PATH="$PWD/../build/${{ matrix.config }}-${{ matrix.library_type }}:$DYLD_LIBRARY_PATH"
           fi
+          # Run tests with AddressSanitizer enabled by default (improves memory error detection)
           ./run_tests.sh --functional --skip-build
         else
           # Set library path for shared library tests

scripts/run_tests.sh

@@ -8,12 +8,19 @@
 # - Windows (MSVC): Uses single build directory, configs specified during build
 # - Linux/Mac: Uses separate build/Debug and build/Release directories
 #
+# Memory Sanitizer (ASAN):
+# - Functional tests: ENABLED by default (can be disabled with --no-sanitize)
+# - Performance tests: DISABLED by default (can be enabled with --sanitize-all)
+# - ASAN helps detect memory errors like buffer overflows, use-after-free, etc.
+#
 # Usage:
-#   ./run_tests.sh                    # Run all tests
-#   ./run_tests.sh --functional       # Run only functional tests
-#   ./run_tests.sh --performance      # Run only performance tests
+#   ./run_tests.sh                    # Run all tests (functional with ASAN, performance without)
+#   ./run_tests.sh --functional       # Run only functional tests (with ASAN)
+#   ./run_tests.sh --performance      # Run only performance tests (without ASAN)
 #   ./run_tests.sh --avx2             # Run only AVX2 performance tests
 #   ./run_tests.sh --shuffle          # Run only tests with names containing 'shuffle' (case-insensitive) in functional tests
+#   ./run_tests.sh --no-sanitize      # Disable ASAN for all tests
+#   ./run_tests.sh --sanitize-all     # Enable ASAN for all tests (including performance)
 #   ./run_tests.sh --help             # Show help
 
 set -e # Exit on any error
@@ -70,6 +77,9 @@ EXIT_WHEN_FAILED=false
 GTEST_FAIL_FAST_PARAM=""
 FILTER=""
 SHUFFLE_ONLY=false
+# Memory sanitizer flags: auto-detect based on test type, can be overridden
+SANITIZE_FUNCTIONAL="auto"  # Default: enabled for functional tests
+SANITIZE_PERFORMANCE="auto" # Default: disabled for performance tests
 
 while [[ $# -gt 0 ]]; do
     case $1 in
@@ -105,19 +115,37 @@ while [[ $# -gt 0 ]]; do
         GTEST_FAIL_FAST_PARAM="--gtest_fail_fast"
         shift
         ;;
+    --no-sanitize)
+        SANITIZE_FUNCTIONAL="no"
+        SANITIZE_PERFORMANCE="no"
+        shift
+        ;;
+    --sanitize-all)
+        SANITIZE_FUNCTIONAL="yes"
+        SANITIZE_PERFORMANCE="yes"
+        shift
+        ;;
     --help)
         echo "CCAP Unit Tests Runner"
         echo ""
         echo "Usage:"
-        echo "  $0                    # Run all tests"
-        echo "  $0 --functional       # Run only functional tests (Debug mode)"
-        echo "  $0 --performance      # Run only performance tests (Release mode)"
+        echo "  $0                    # Run all tests (functional with ASAN, performance without)"
+        echo "  $0 --functional       # Run only functional tests (Debug mode, with ASAN)"
+        echo "  $0 --performance      # Run only performance tests (Release mode, without ASAN)"
         echo "  $0 --avx2             # Run only AVX2 performance tests (Release mode)"
         echo "  $0 --shuffle          # Run only tests whose names contain '*shuffle*' or '*Shuffle*' in functional tests"
         echo "  $0 --skip-build       # Skip build step, run tests only"
         echo "  $0 --exit-when-failed # Stop at first test failure (gtest fail fast mode)"
+        echo "  $0 --no-sanitize      # Disable AddressSanitizer (ASAN) for all tests"
+        echo "  $0 --sanitize-all     # Enable AddressSanitizer (ASAN) for all tests (including performance)"
         echo "  $0 --help             # Show this help"
         echo ""
+        echo "Memory Sanitizer (ASAN):"
+        echo "  - Functional tests: ENABLED by default (detects memory errors)"
+        echo "  - Performance tests: DISABLED by default (would affect performance measurements)"
+        echo "  - Use --no-sanitize to disable ASAN completely"
+        echo "  - Use --sanitize-all to enable ASAN for performance tests too"
+        echo ""
         echo "Note: Performance tests are automatically run in Release mode for accurate results"
         exit 0
         ;;
@@ -162,6 +190,48 @@ fi
 TEST_RESULT=0
 PERF_RESULT=0
 
+# Determine ASAN usage
+# Functional tests: default enabled, Performance tests: default disabled
+USE_ASAN_FUNCTIONAL=false
+USE_ASAN_PERFORMANCE=false
+
+if [ "$SANITIZE_FUNCTIONAL" = "auto" ]; then
+    USE_ASAN_FUNCTIONAL=true # Default: enable ASAN for functional tests
+elif [ "$SANITIZE_FUNCTIONAL" = "yes" ]; then
+    USE_ASAN_FUNCTIONAL=true
+fi
+
+if [ "$SANITIZE_PERFORMANCE" = "auto" ]; then
+    USE_ASAN_PERFORMANCE=false # Default: disable ASAN for performance tests
+elif [ "$SANITIZE_PERFORMANCE" = "yes" ]; then
+    USE_ASAN_PERFORMANCE=true
+fi
+
+# Function to check if ASAN is supported
+function checkAsanSupport() {
+    # ASAN is well supported on Linux and macOS with GCC/Clang
+    # Windows MSVC support is limited and not used here
+    if isWindows; then
+        return 1 # Disable ASAN on Windows for now
+    fi
+    return 0
+}
+
+# Check ASAN support
+ASAN_SUPPORTED=false
+if checkAsanSupport; then
+    ASAN_SUPPORTED=true
+fi
+
+# Disable ASAN if not supported
+if [ "$ASAN_SUPPORTED" = false ]; then
+    if [ "$USE_ASAN_FUNCTIONAL" = true ] || [ "$USE_ASAN_PERFORMANCE" = true ]; then
+        echo -e "${YELLOW}⚠ AddressSanitizer not supported on this platform, disabling ASAN${NC}"
+        USE_ASAN_FUNCTIONAL=false
+        USE_ASAN_PERFORMANCE=false
+    fi
+fi
+
 # Build Debug version for functional tests
 if [ "$RUN_FUNCTIONAL" = true ]; then
     echo ""
@@ -170,15 +240,24 @@ if [ "$RUN_FUNCTIONAL" = true ]; then
         echo -e "${BLUE}Skipping build, using existing Debug binaries${NC}"
     else
         echo -e "${BLUE}Building Debug version (for functional tests)${NC}"
+        if [ "$USE_ASAN_FUNCTIONAL" = true ]; then
+            echo -e "${GREEN}🛡️  AddressSanitizer (ASAN) ENABLED for memory error detection${NC}"
+        fi
     fi
     echo -e "${PURPLE}===============================================${NC}"
 
     if [ "$SKIP_BUILD" = false ]; then
+        # Prepare ASAN flags if enabled
+        ASAN_FLAGS=""
+        if [ "$USE_ASAN_FUNCTIONAL" = true ]; then
+            ASAN_FLAGS="-DCMAKE_CXX_FLAGS=\"-fsanitize=address -g\" -DCMAKE_C_FLAGS=\"-fsanitize=address -g\" -DCMAKE_EXE_LINKER_FLAGS=\"-fsanitize=address\" -DCMAKE_SHARED_LINKER_FLAGS=\"-fsanitize=address\""
+        fi
+
         if isWindows; then
             # Windows MSVC: use single build directory, specify config during build
             cd build
             echo -e "${BLUE}Configuring CMake (Windows MSVC)...${NC}"
-            cmake .. -DCCAP_BUILD_TESTS=ON -DCMAKE_POLICY_VERSION_MINIMUM=3.5
+            eval cmake .. -DCCAP_BUILD_TESTS=ON -DCMAKE_POLICY_VERSION_MINIMUM=3.5 $ASAN_FLAGS
 
             echo -e "${BLUE}Building Debug project...${NC}"
             cmake --build . --config Debug --parallel $(detectCores)
@@ -191,7 +270,7 @@ if [ "$RUN_FUNCTIONAL" = true ]; then
             # Linux/Mac: use separate Debug directory
             cd build/Debug
             echo -e "${BLUE}Configuring CMake (Debug)...${NC}"
-            cmake ../.. -DCMAKE_BUILD_TYPE=Debug -DCCAP_BUILD_TESTS=ON -DCMAKE_POLICY_VERSION_MINIMUM=3.5
+            eval cmake ../.. -DCMAKE_BUILD_TYPE=Debug -DCCAP_BUILD_TESTS=ON -DCMAKE_POLICY_VERSION_MINIMUM=3.5 $ASAN_FLAGS
 
             echo -e "${BLUE}Building Debug project...${NC}"
             cmake --build . --config Debug --parallel $(detectCores)
@@ -211,17 +290,27 @@ if [ "$RUN_PERFORMANCE" = true ]; then
         echo -e "${BLUE}Skipping build, using existing Release binaries${NC}"
     else
         echo -e "${BLUE}Building Release version (for performance tests)${NC}"
+        if [ "$USE_ASAN_PERFORMANCE" = true ]; then
+            echo -e "${GREEN}🛡️  AddressSanitizer (ASAN) ENABLED${NC}"
+            echo -e "${YELLOW}⚠  Note: ASAN affects performance measurements${NC}"
+        fi
     fi
     echo -e "${PURPLE}===============================================${NC}"
 
     if [ "$SKIP_BUILD" = false ]; then
+        # Prepare ASAN flags if enabled
+        ASAN_FLAGS=""
+        if [ "$USE_ASAN_PERFORMANCE" = true ]; then
+            ASAN_FLAGS="-DCMAKE_CXX_FLAGS=\"-fsanitize=address -g\" -DCMAKE_C_FLAGS=\"-fsanitize=address -g\" -DCMAKE_EXE_LINKER_FLAGS=\"-fsanitize=address\" -DCMAKE_SHARED_LINKER_FLAGS=\"-fsanitize=address\""
+        fi
+
         if isWindows; then
             # Windows MSVC: use single build directory, specify config during build
             cd build
             # Only configure if not already configured
             if [ ! -f "CMakeCache.txt" ]; then
                 echo -e "${BLUE}Configuring CMake (Windows MSVC)...${NC}"
-                cmake .. -DCCAP_BUILD_TESTS=ON -DCMAKE_POLICY_VERSION_MINIMUM=3.5
+                eval cmake .. -DCCAP_BUILD_TESTS=ON -DCMAKE_POLICY_VERSION_MINIMUM=3.5 $ASAN_FLAGS
             fi
 
             echo -e "${BLUE}Building Release project...${NC}"
@@ -234,7 +323,7 @@ if [ "$RUN_PERFORMANCE" = true ]; then
             # Linux/Mac: use separate Release directory
             cd build/Release
             echo -e "${BLUE}Configuring CMake (Release)...${NC}"
-            cmake ../.. -DCMAKE_BUILD_TYPE=Release -DCCAP_BUILD_TESTS=ON -DCMAKE_POLICY_VERSION_MINIMUM=3.5
+            eval cmake ../.. -DCMAKE_BUILD_TYPE=Release -DCCAP_BUILD_TESTS=ON -DCMAKE_POLICY_VERSION_MINIMUM=3.5 $ASAN_FLAGS
 
             echo -e "${BLUE}Building Release project...${NC}"
             cmake --build . --config Release --parallel $(detectCores)
@@ -265,6 +354,14 @@ if [ "$RUN_FUNCTIONAL" = true ]; then
     if [ -f "$TEST_EXECUTABLE" ]; then
         echo -e "${YELLOW}Running functional tests in Debug mode...${NC}"
 
+        # Set ASAN options if enabled
+        if [ "$USE_ASAN_FUNCTIONAL" = true ]; then
+            echo -e "${GREEN}🛡️  Running with AddressSanitizer enabled${NC}"
+            # Disable memory leak detection in ASAN (can cause false positives in tests)
+            # Enable detailed error reporting
+            export ASAN_OPTIONS="detect_leaks=0:halt_on_error=1:allocator_may_return_null=1"
+        fi
+
         if [ "$SHUFFLE_ONLY" = true ]; then
             echo -e "${BLUE}Filtering tests to names containing '*shuffle*' or '*Shuffle*'...${NC}"
             "$TEST_EXECUTABLE" --gtest_filter='*shuffle*:*Shuffle*:*SHUFFLE*' $GTEST_FAIL_FAST_PARAM --gtest_output=xml:test_results_debug.xml
@@ -326,6 +423,13 @@ if [ "$RUN_PERFORMANCE" = true ]; then
         echo -e "${YELLOW}Running performance benchmarks in Release mode...${NC}"
         echo -e "${BLUE}Note: Release mode provides accurate performance measurements${NC}"
 
+        # Set ASAN options if enabled
+        if [ "$USE_ASAN_PERFORMANCE" = true ]; then
+            echo -e "${GREEN}🛡️  Running with AddressSanitizer enabled${NC}"
+            echo -e "${YELLOW}⚠  Performance results may be affected by ASAN overhead${NC}"
+            export ASAN_OPTIONS="detect_leaks=0:halt_on_error=1:allocator_may_return_null=1"
+        fi
+
         if [ -n "$FILTER" ]; then
             echo -e "${BLUE}Filter: $FILTER${NC}"
             "$PERF_EXECUTABLE" $FILTER $GTEST_FAIL_FAST_PARAM --gtest_output=xml:build/performance_results_release.xml

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) {
@@ -181,11 +181,21 @@ AVX2_TARGET void colorShuffle_avx2(const uint8_t* src, int srcStride, uint8_t* d
         shuffle128 = _mm_load_si128((__m128i*)shuffleData);
     }
 
+    // Different cases require different boundary conditions to avoid reading beyond allocated memory:
+    // - 3->4: reads 16 bytes from x*3+12, needs x*3+27 < width*3, i.e., x+9 < width
+    // - 3->3: reads 16 bytes from x*3, needs x*3+15 < width*3, i.e., x+5 < width  
+    // - 4->3: reads 16 bytes from x*4+16, needs x*4+31 < width*4, i.e., x+8 <= width
+    // - 4->4: reads 32 bytes from x*4, needs x*4+31 < width*4, i.e., x+8 <= width
+    constexpr uint32_t loopBoundary = (inputChannels == 3 && outputChannels == 4) ? (patchSize + 2) :
+                                      (inputChannels == 3 && outputChannels == 3) ? (patchSize + 1) :
+                                                                                    patchSize;
+
     for (int y = 0; y < height; ++y) {
         const uint8_t* srcRow = src + y * srcStride;
         uint8_t* dstRow = dst + y * dstStride;
         uint32_t x = 0;
-        while (x + patchSize <= (uint32_t)width) {
+
+        while (x + loopBoundary <= (uint32_t)width) {
             // _mm256_shuffle_epi8 can’t move these bytes across 16-byte lanes of the vector.
             // @see issue <https://stackoverflow.com/questions/77149094/how-to-use-mm256-shuffle-epi8-to-order-elements>
             if constexpr (outputChannels == 4 && inputChannels == 3) { // 3 -> 4, need to split channels
@@ -218,17 +228,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];