Prevent codec reconfiguration after opening (fixes #2232)

Author: WyattBlue

CHANGELOG.rst

@@ -40,6 +40,7 @@ Features:
 
 Fixes:
 
+- Prevent crashes and corrupted output when structural codec properties are changed after an output stream has been opened by :gh-user:`WyattBlue`, reported by :gh-user:`oakaigh` (:issue:`2232`).
 - Fix a crash when using a stream that has no ``CodecContext`` (a demuxed stream with no available decoder, such as one from a truncated file, or a stream created by ``add_mux_stream``); decoding now raises ``DecoderNotFoundError``, encoding now raises ``EncoderNotFoundError``, and ``BitStreamFilterContext`` accepts such a stream as ``out_stream`` by :gh-user:`WyattBlue`, reported by :gh-user:`justinrmiller` (:issue:`2344`).
 
 v18.0.0

av/audio/codeccontext.py

@@ -62,6 +62,7 @@ def sample_rate(self):
 
     @sample_rate.setter
     def sample_rate(self, value: cython.int):
+        self._assert_not_open("sample_rate")
         self.ptr.sample_rate = value
 
     @property
@@ -88,6 +89,7 @@ def layout(self):
 
     @layout.setter
     def layout(self, value):
+        self._assert_not_open("layout")
         layout: AudioLayout = AudioLayout(value)
         self.ptr.ch_layout = layout.layout
 
@@ -102,5 +104,6 @@ def format(self):
 
     @format.setter
     def format(self, value):
+        self._assert_not_open("format")
         format: AudioFormat = AudioFormat(value)
         self.ptr.sample_fmt = format.sample_fmt

av/codec/context.pxd

@@ -17,6 +17,7 @@ cdef class CodecContext:
 
     cdef lib.AVCodecParserContext *parser
     cdef _init(self, lib.AVCodecContext *ptr, const lib.AVCodec *codec, HWAccel hwaccel)
+    cdef _assert_not_open(self, name)
 
     # Public API.
     cdef readonly bint is_open

av/codec/context.py

@@ -287,6 +287,11 @@ def _init(
         self.ptr.thread_count = 0  # use as many threads as there are CPUs.
         self.ptr.thread_type = 0x02  # thread within a frame. Does not change the API.
 
+    @cython.cfunc
+    def _assert_not_open(self, name):
+        if self.is_open:
+            raise RuntimeError(f"Cannot change {name} after codec is open.")
+
     @property
     def flags(self):
         """

av/container/output.py

@@ -103,6 +103,12 @@ def add_stream(
         :param \\**kwargs: Set attributes for the stream.
         :rtype: The new :class:`~av.stream.Stream`.
 
+        .. warning::
+
+            Configure every output stream before muxing the first packet. Writing
+            the file header opens all stream codec contexts, after which structural
+            properties such as format, dimensions, layout, and rate cannot change.
+
         """
 
         codec_obj: Codec = Codec(codec_name, "w")

av/video/codeccontext.py

@@ -193,6 +193,7 @@ def format(self):
 
     @format.setter
     def format(self, format: VideoFormat):
+        self._assert_not_open("format")
         self.ptr.pix_fmt = format.pix_fmt
         self.ptr.width = format.width
         self.ptr.height = format.height
@@ -205,6 +206,7 @@ def width(self):
 
     @width.setter
     def width(self, value: cython.uint):
+        self._assert_not_open("width")
         self.ptr.width = value
 
     @property
@@ -215,6 +217,7 @@ def height(self):
 
     @height.setter
     def height(self, value: cython.uint):
+        self._assert_not_open("height")
         self.ptr.height = value
 
     @property
@@ -251,6 +254,7 @@ def pix_fmt(self):
 
     @pix_fmt.setter
     def pix_fmt(self, value):
+        self._assert_not_open("pix_fmt")
         self.ptr.pix_fmt = get_pix_fmt(value)
 
     @property
@@ -277,6 +281,7 @@ def sw_format(self):
 
     @sw_format.setter
     def sw_format(self, value):
+        self._assert_not_open("sw_format")
         self.ptr.sw_pix_fmt = get_pix_fmt(value)
 
     @property
@@ -290,6 +295,7 @@ def framerate(self):
 
     @framerate.setter
     def framerate(self, value):
+        self._assert_not_open("framerate")
         to_avrational(value, cython.address(self.ptr.framerate))
 
     @property

tests/test_encode.py

@@ -4,6 +4,7 @@
 import math
 import os
 from fractions import Fraction
+from typing import cast
 
 import numpy as np
 import pytest
@@ -268,6 +269,30 @@ def test_subtitle_muxing(self) -> None:
 
 
 class TestEncodeStreamSemantics(TestCase):
+    def test_reconfigure_stream_after_mux(self) -> None:
+        output_bytes = io.BytesIO()
+        with av.open(output_bytes, "w", format="mp4") as output:
+            first = cast(VideoStream, output.add_stream("ffv1", rate=30))
+            second = cast(VideoStream, output.add_stream("ffv1", rate=30))
+
+            first.format = av.VideoFormat("bgr0", width=16, height=16)
+            frame = VideoFrame(16, 16, "bgr0")
+            frame.pts = 0
+            frame.time_base = Fraction(1, 30)
+            output.mux(first.encode(frame))
+
+            # Muxing the first packet writes the header and opens every stream.
+            # Changing the second encoder now used to corrupt FFV1 state and crash.
+            assert second.codec_context.is_open
+            with pytest.raises(RuntimeError, match="Cannot change format"):
+                second.format = av.VideoFormat("bgr0", width=16, height=16)
+            with pytest.raises(RuntimeError, match="Cannot change width"):
+                second.width = 16
+            with pytest.raises(RuntimeError, match="Cannot change height"):
+                second.height = 16
+            with pytest.raises(RuntimeError, match="Cannot change pix_fmt"):
+                second.pix_fmt = "bgr0"
+
     def test_stream_index(self) -> None:
         with av.open(self.sandboxed("output.mov"), "w") as output:
             vstream = output.add_stream("mpeg4", 24)