Make transfer/primaries conversion opt-in in reformat, fixes #2208

CHANGELOG.rst

@@ -46,6 +46,7 @@ Features:
 
 Fixes:
 
+- Fix ``VideoFrame.reformat`` (and ``to_ndarray``/``to_rgb``/``to_image``) raising ``OSError`` ``Operation not supported`` on frames tagged with reserved or otherwise unsupported ``color_primaries``/``color_trc`` values (e.g. VP9 and NVDEC output); a transfer/primaries conversion is now only performed when explicitly requested by :gh-user:`WyattBlue` (:issue:`2208`).
 - Fix ``add_mux_stream`` producing unwritable Matroska files by extracting codec extradata from the bitstream before the header is written by :gh-user:`WyattBlue` (:issue:`2198`).
 - Encode GPU frames (e.g. CUDA frames from DLPack) directly with ``pix_fmt="cuda"`` by adopting the frame's ``hw_frames_ctx`` before opening the encoder by :gh-user:`WyattBlue` (:issue:`2199`).
 

av/video/reformatter.py

@@ -224,11 +224,14 @@ def reformat(
         )
         c_src_color_range = _resolve_enum_value(src_color_range, ColorRange, 0)
         c_dst_color_range = _resolve_enum_value(dst_color_range, ColorRange, 0)
+        # Default to UNSPECIFIED (not the source's value) so that a transfer /
+        # primaries conversion is only performed when explicitly requested. See
+        # _reformat for why.
         c_dst_color_trc = _resolve_enum_value(
-            dst_color_trc, ColorTrc, frame.ptr.color_trc
+            dst_color_trc, ColorTrc, lib.AVCOL_TRC_UNSPECIFIED
         )
         c_dst_color_primaries = _resolve_enum_value(
-            dst_color_primaries, ColorPrimaries, frame.ptr.color_primaries
+            dst_color_primaries, ColorPrimaries, lib.AVCOL_PRI_UNSPECIFIED
         )
         c_threads: cython.int = threads if threads is not None else 0
         c_width: cython.int = width if width is not None else frame.ptr.width
@@ -277,12 +280,37 @@ def _reformat(
         new_frame.ptr.format = dst_format
         new_frame.ptr.width = width
         new_frame.ptr.height = height
-        new_frame.ptr.color_trc = cython.cast(
-            lib.AVColorTransferCharacteristic, dst_color_trc
-        )
-        new_frame.ptr.color_primaries = cython.cast(
-            lib.AVColorPrimaries, dst_color_primaries
+
+        # A transfer-characteristic / primaries conversion is opt-in. Unlike the
+        # pre-17.0 sws_scale, sws_scale_frame inspects color_trc/color_primaries
+        # and rejects RESERVED (and other unsupported) values with EOPNOTSUPP,
+        # which regressed plain reformats of e.g. VP9 / NVDEC frames (#2208). So
+        # only feed these fields to swscale when the caller explicitly requested a
+        # destination value; otherwise neutralize them for the scale (as the old
+        # sws_scale effectively did) while still preserving the source's tags on
+        # the returned frame's metadata.
+        convert_trc: cython.bint = dst_color_trc != lib.AVCOL_TRC_UNSPECIFIED
+        convert_primaries: cython.bint = (
+            dst_color_primaries != lib.AVCOL_PRI_UNSPECIFIED
         )
+        frame_src_color_trc: lib.AVColorTransferCharacteristic = frame.ptr.color_trc
+        frame_src_color_primaries: lib.AVColorPrimaries = frame.ptr.color_primaries
+
+        if convert_trc:
+            new_frame.ptr.color_trc = cython.cast(
+                lib.AVColorTransferCharacteristic, dst_color_trc
+            )
+        else:
+            frame.ptr.color_trc = lib.AVCOL_TRC_UNSPECIFIED
+            new_frame.ptr.color_trc = lib.AVCOL_TRC_UNSPECIFIED
+
+        if convert_primaries:
+            new_frame.ptr.color_primaries = cython.cast(
+                lib.AVColorPrimaries, dst_color_primaries
+            )
+        else:
+            frame.ptr.color_primaries = lib.AVCOL_PRI_UNSPECIFIED
+            new_frame.ptr.color_primaries = lib.AVCOL_PRI_UNSPECIFIED
 
         # Translate source and destination colorspace/range from SWS_CS_* to AVCOL_*
         # so sws_is_noop and sws_scale_frame understand them
@@ -294,9 +322,11 @@ def _reformat(
         # Shortcut if sws_scale_frame would be a no-op
         is_noop: cython.bint = sws_is_noop(new_frame.ptr, frame.ptr) != 0
         if is_noop:
-            # Restore source frame colorspace/range to avoid side effects
+            # Restore source frame metadata to avoid side effects
             frame.ptr.colorspace = frame_src_colorspace
             frame.ptr.color_range = frame_src_color_range
+            frame.ptr.color_trc = frame_src_color_trc
+            frame.ptr.color_primaries = frame_src_color_primaries
             return frame
 
         if self.ptr == cython.NULL:
@@ -311,9 +341,18 @@ def _reformat(
         with cython.nogil:
             ret = sws_scale_frame(self.ptr, new_frame.ptr, frame.ptr)
 
-        # Restore source frame colorspace/range to avoid side effects
+        # Restore source frame metadata to avoid side effects
         frame.ptr.colorspace = frame_src_colorspace
         frame.ptr.color_range = frame_src_color_range
+        frame.ptr.color_trc = frame_src_color_trc
+        frame.ptr.color_primaries = frame_src_color_primaries
+
+        # Preserve the source's transfer/primaries on the output when no explicit
+        # conversion was requested (the scale ran with neutralized tags).
+        if not convert_trc:
+            new_frame.ptr.color_trc = frame_src_color_trc
+        if not convert_primaries:
+            new_frame.ptr.color_primaries = frame_src_color_primaries
 
         err_check(ret)
 

tests/test_colorspace.py

@@ -121,3 +121,50 @@ def test_reformat_dst_colorspace_metadata(
     frame = av.VideoFrame(width=64, height=64, format="yuv420p")
     rgb = frame.reformat(format="rgb24", dst_colorspace=colorspace)
     assert rgb.colorspace == expected
+
+
+# RESERVED0 (0) and RESERVED (3) primaries/transfer values, plus a couple of
+# transfer functions swscale can't handle (LOG / LOG_SQRT). Real VP9 and NVDEC
+# streams routinely tag frames with these. sws_scale_frame (used since 17.0.0)
+# validates these fields and rejects them with EOPNOTSUPP, which regressed a
+# plain reformat/to_ndarray to "rgb24" (#2208). The pre-17.0 sws_scale ignored
+# them, and a transfer/primaries conversion should stay opt-in.
+@pytest.mark.parametrize(
+    ("color_primaries", "color_trc"),
+    [
+        (3, 3),  # RESERVED / RESERVED
+        (0, 0),  # RESERVED0 / RESERVED0
+        (3, 2),  # reserved primaries only
+        (2, 3),  # reserved transfer only
+        (2, 9),  # AVCOL_TRC_LOG (unsupported by swscale)
+        (2, 10),  # AVCOL_TRC_LOG_SQRT (unsupported by swscale)
+    ],
+)
+def test_reformat_unsupported_color_metadata(
+    color_primaries: int, color_trc: int
+) -> None:
+    frame = av.VideoFrame(width=64, height=64, format="yuv420p")
+    frame.colorspace = Colorspace.ITU709
+    frame.color_primaries = color_primaries
+    frame.color_trc = color_trc
+
+    # Neither of these should raise OSError(EOPNOTSUPP).
+    rgb = frame.reformat(format="rgb24")
+    assert rgb.format.name == "rgb24"
+    array = frame.to_ndarray(format="rgb24")
+    assert array.shape == (64, 64, 3)
+
+    # The reformat must not mutate the source frame's metadata.
+    assert frame.color_primaries == color_primaries
+    assert frame.color_trc == color_trc
+
+    # The BT.709 matrix is still applied even though the transfer/primaries are
+    # unsupported: a neutral gray must stay gray.
+    gray = av.VideoFrame(width=64, height=64, format="yuv420p")
+    gray.colorspace = Colorspace.ITU709
+    gray.color_primaries = color_primaries
+    gray.color_trc = color_trc
+    for plane, value in zip(gray.planes, (128, 128, 128)):
+        plane.update(bytes([value]) * plane.buffer_size)
+    out = gray.to_ndarray(format="rgb24")
+    assert out.min() == out.max() == out[0, 0, 0]