Fix color space conversion filters

Author: shrimbly

sharp_frames/processing/colorspace.py

@@ -300,9 +300,10 @@ def _build_hdr_to_sdr_filter(color_info: VideoColorInfo) -> str:
             "format=yuv420p"                     # Standard output format
         )
     else:
-        # Fallback: basic colorspace conversion (won't tone map properly)
-        # This is better than nothing but may clip highlights
-        filter_chain = f"colorspace=all=bt709:iall={primaries_in}:fast=0"
+        raise RuntimeError(
+            "HDR-to-SDR conversion requires FFmpeg's zscale filter. "
+            "Install an FFmpeg build with libzimg support."
+        )
 
     return filter_chain
 
@@ -311,18 +312,31 @@ def _build_wide_gamut_to_srgb_filter(color_info: VideoColorInfo) -> str:
     """
     Build wide gamut SDR (Display P3/BT.2020) to sRGB/BT.709 filter.
     """
-    # Determine input color space for the filter
-    if color_info.color_primaries == ColorPrimaries.DISPLAY_P3:
-        input_primaries = "smpte432"
-    elif color_info.color_primaries == ColorPrimaries.BT2020:
-        input_primaries = "bt2020"
-    else:
-        input_primaries = "bt709"  # Assume BT.709 if unknown
-
-    # Use colorspace filter for gamut conversion (no tone mapping needed)
-    filter_chain = f"colorspace=all=bt709:iall={input_primaries}:fast=0"
-
-    return filter_chain
+    input_primaries = {
+        ColorPrimaries.DISPLAY_P3: "smpte432",
+        ColorPrimaries.BT2020: "bt2020",
+    }.get(color_info.color_primaries, "bt709")
+    input_transfer = {
+        TransferFunction.SRGB: "iec61966-2-1",
+        TransferFunction.BT709: "bt709",
+    }.get(color_info.transfer_function, "bt709")
+    default_matrix = (
+        "bt2020nc"
+        if color_info.color_primaries == ColorPrimaries.BT2020
+        else "bt709"
+    )
+    input_matrix = {
+        ColorMatrix.BT709: "bt709",
+        ColorMatrix.BT2020_NCL: "bt2020nc",
+        ColorMatrix.BT2020_CL: "bt2020c",
+    }.get(color_info.color_matrix, default_matrix)
+
+    return (
+        "colorspace=all=bt709:"
+        f"ispace={input_matrix}:"
+        f"iprimaries={input_primaries}:"
+        f"itrc={input_transfer}:fast=0"
+    )
 
 
 def get_color_info_description(color_info: VideoColorInfo) -> str:

tests/test_colorspace.py

@@ -1,8 +1,11 @@
 """Tests for the colorspace detection and conversion module."""
 
-import pytest
+import shutil
+import subprocess
 from unittest.mock import patch, MagicMock
 
+import pytest
+
 from sharp_frames.processing.colorspace import (
     VideoColorInfo,
     ColorPrimaries,
@@ -198,6 +201,8 @@ def test_filter_for_display_p3(self):
         assert 'colorspace' in filter_str
         assert 'bt709' in filter_str
         assert 'smpte432' in filter_str
+        assert 'iprimaries=smpte432' in filter_str
+        assert 'iall=smpte432' not in filter_str
 
     @patch('sharp_frames.processing.colorspace.is_zscale_available', return_value=True)
     def test_hdr_filter_with_zscale(self, mock_zscale):
@@ -219,17 +224,77 @@ def test_hdr_filter_with_zscale(self, mock_zscale):
 
     @patch('sharp_frames.processing.colorspace.is_zscale_available', return_value=False)
     def test_hdr_filter_fallback_without_zscale(self, mock_zscale):
-        """HDR filter falls back to colorspace when zscale unavailable."""
+        """HDR conversion fails clearly when tone mapping is unavailable."""
         info = VideoColorInfo(
             color_primaries=ColorPrimaries.BT2020,
             transfer_function=TransferFunction.PQ,
             color_matrix=ColorMatrix.BT2020_NCL,
             is_hdr=True
         )
-        filter_str = build_colorspace_filter(info)
-        assert filter_str is not None
-        assert 'colorspace' in filter_str
-        assert 'zscale' not in filter_str
+        with pytest.raises(RuntimeError, match='zscale'):
+            build_colorspace_filter(info)
+
+
+@pytest.mark.skipif(shutil.which('ffmpeg') is None, reason='FFmpeg is not installed')
+class TestColorspaceFilterIntegration:
+    """Validate generated filters with a real FFmpeg process."""
+
+    @staticmethod
+    def _run_filter(source_params, filter_string):
+        source = (
+            "testsrc2=size=64x64:rate=1,format=yuv420p,"
+            f"setparams={source_params}"
+        )
+        result = subprocess.run(
+            [
+                'ffmpeg', '-hide_banner', '-loglevel', 'error',
+                '-f', 'lavfi', '-i', source,
+                '-frames:v', '1', '-vf', filter_string,
+                '-f', 'null', '-'
+            ],
+            capture_output=True,
+            text=True,
+            timeout=30,
+        )
+        assert result.returncode == 0, result.stderr
+
+    def test_display_p3_filter_executes(self):
+        info = VideoColorInfo(
+            color_primaries=ColorPrimaries.DISPLAY_P3,
+            transfer_function=TransferFunction.BT709,
+            color_matrix=ColorMatrix.BT709,
+            is_hdr=False,
+        )
+        self._run_filter(
+            'color_primaries=smpte432:color_trc=bt709:colorspace=bt709',
+            build_colorspace_filter(info),
+        )
+
+    def test_bt2020_sdr_filter_executes(self):
+        info = VideoColorInfo(
+            color_primaries=ColorPrimaries.BT2020,
+            transfer_function=TransferFunction.BT709,
+            color_matrix=ColorMatrix.BT2020_NCL,
+            is_hdr=False,
+        )
+        self._run_filter(
+            'color_primaries=bt2020:color_trc=bt709:colorspace=bt2020nc',
+            build_colorspace_filter(info),
+        )
+
+    def test_hdr_filter_executes(self):
+        if not is_zscale_available():
+            pytest.skip("FFmpeg's zscale filter is not available")
+        info = VideoColorInfo(
+            color_primaries=ColorPrimaries.BT2020,
+            transfer_function=TransferFunction.PQ,
+            color_matrix=ColorMatrix.BT2020_NCL,
+            is_hdr=True,
+        )
+        self._run_filter(
+            'color_primaries=bt2020:color_trc=smpte2084:colorspace=bt2020nc',
+            build_colorspace_filter(info),
+        )
 
 
 class TestGetColorInfoDescription: