Deep copy AVChannelLayout to fix double free, fixes #2444

CHANGELOG.rst

@@ -36,6 +36,7 @@ Fixes:
 
 - Audio encoding now correctly passes the codec's ``thread_count`` to its internal :class:`.AudioResampler`, which takes a new ``threads`` argument, as video encoding does for its reformatter. Audio codecs without threading (e.g. MP3) resolve ``thread_count`` to ``1``, but the resampler ignored it and started an idle thread per CPU. By :gh-user:`tonyay163`.
 - Output containers now pass their options to ``avio_open2()`` before ``avformat_write_header()``, as the ``ffmpeg`` program does, so protocol options are used by the protocol. By :gh-user:`ANDYwyLEE`.
+- Fix memory corruption bug when decoding audio with custom channel layouts: :class:`.AudioLayout`, :class:`.AudioFrame`, and :class:`.AudioCodecContext` shallow-copied channel layouts, so custom-order layouts (e.g. ``"FR+FL"``) were double freed. Fixes (:issue:`2444`) by :gh-user:`WyattBlue`.
 
 
 v19.0.1

av/audio/codeccontext.py

@@ -3,6 +3,7 @@
 from cython.cimports.av.audio.format import AudioFormat, get_audio_format
 from cython.cimports.av.audio.frame import AudioFrame, alloc_audio_frame
 from cython.cimports.av.audio.layout import AudioLayout, get_audio_layout
+from cython.cimports.av.error import err_check
 from cython.cimports.av.frame import Frame
 from cython.cimports.av.packet import Packet
 
@@ -94,7 +95,11 @@ def layout(self):
     def layout(self, value):
         self._assert_not_open("layout")
         layout: AudioLayout = AudioLayout(value)
-        self.ptr.ch_layout = layout.layout
+        err_check(
+            lib.av_channel_layout_copy(
+                cython.address(self.ptr.ch_layout), cython.address(layout.layout)
+            )
+        )
 
     @property
     def format(self):

av/audio/frame.py

@@ -50,7 +50,11 @@ def _init(
     ) -> cython.void:
         self.ptr.nb_samples = nb_samples
         self.ptr.format = format
-        self.ptr.ch_layout = layout
+        err_check(
+            lib.av_channel_layout_copy(
+                cython.address(self.ptr.ch_layout), cython.address(layout)
+            )
+        )
 
         # Sometimes this is called twice. Oh well.
         self._init_user_attributes()

av/audio/layout.py

@@ -2,6 +2,7 @@
 
 import cython
 from cython.cimports import libav as lib
+from cython.cimports.av.error import err_check
 from cython.cimports.cpython.bytes import PyBytes_FromStringAndSize
 
 
@@ -21,7 +22,11 @@ def __repr__(self):
 def get_audio_layout(c_layout: lib.AVChannelLayout) -> AudioLayout:
     """Get an AudioLayout from Cython land."""
     layout: AudioLayout = AudioLayout(_cinit_bypass_sentinel)
-    layout.layout = c_layout
+    err_check(
+        lib.av_channel_layout_copy(
+            cython.address(layout.layout), cython.address(c_layout)
+        )
+    )
     return layout
 
 
@@ -36,18 +41,21 @@ def __cinit__(self, layout):
             return
 
         if type(layout) is str:
-            ret = lib.av_channel_layout_from_string(cython.address(c_layout), layout)
+            ret = lib.av_channel_layout_from_string(cython.address(self.layout), layout)
             if ret != 0:
                 raise ValueError(f"Invalid layout: {layout}")
         elif isinstance(layout, AudioLayout):
-            c_layout = cython.cast(AudioLayout, layout).layout
+            err_check(
+                lib.av_channel_layout_copy(
+                    cython.address(self.layout),
+                    cython.address(cython.cast(AudioLayout, layout).layout),
+                )
+            )
         else:
             raise TypeError(
                 f"layout must be of type: string | av.AudioLayout, got {type(layout)}"
             )
 
-        self.layout = c_layout
-
     def __repr__(self):
         return f"<av.{self.__class__.__name__} {self.name!r}>"
 

av/frame.py

@@ -41,7 +41,12 @@ def _copy_internal_attributes(
             self.ptr.format = source.ptr.format
             self.ptr.width = source.ptr.width
             self.ptr.height = source.ptr.height
-            self.ptr.ch_layout = source.ptr.ch_layout
+            err_check(
+                lib.av_channel_layout_copy(
+                    cython.address(self.ptr.ch_layout),
+                    cython.address(source.ptr.ch_layout),
+                )
+            )
 
     @cython.cfunc
     def _init_user_attributes(self) -> cython.void:

include/avcodec.pxd

@@ -13,6 +13,7 @@ cdef extern from "libavutil/channel_layout.h" nogil:
     int av_channel_layout_compare(const AVChannelLayout *chl, const AVChannelLayout *chl1)
     AVChannel av_channel_layout_channel_from_index(const AVChannelLayout *channel_layout, unsigned int idx)
     void av_channel_layout_uninit(AVChannelLayout *channel_layout)
+    int av_channel_layout_copy(AVChannelLayout *dst, const AVChannelLayout *src)
 
 cdef extern from "libavcodec/avcodec.h" nogil:
     cdef unsigned int avcodec_version()

tests/test_audiolayout.py

@@ -1,3 +1,4 @@
+import av
 from av import AudioLayout
 
 
@@ -24,3 +25,23 @@ def test_stereo_from_str() -> None:
 def test_stereo_from_layout() -> None:
     layout2 = AudioLayout(AudioLayout("stereo"))
     _test_stereo(layout2)
+
+
+def test_custom_order_copies() -> None:
+    # Custom-order layouts own a heap-allocated channel map; copies must not share it.
+    a = AudioLayout("FR+FL")
+    b = AudioLayout(a)
+    del a
+    assert b.name == "2 channels (FR+FL)"
+    del b
+
+    frame = av.AudioFrame(format="s16", layout="FR+FL", samples=4)
+    layout = frame.layout
+    del frame
+    assert layout.nb_channels == 2
+
+    ctx = av.CodecContext.create("pcm_s16le", "w")
+    assert isinstance(ctx, av.AudioCodecContext)
+    ctx.layout = "FR+FL"
+    for _ in range(3):
+        assert ctx.layout.name == "2 channels (FR+FL)"