Read and write metadata as UTF-8 with surrogateescape

CHANGELOG.rst

@@ -37,6 +37,7 @@ Major:
 - Remove the undocumented ``CodecContext.hwaccel`` attribute. It held the ``HWAccel`` settings object passed in, not the live device context; use ``CodecContext.is_hwaccel`` to check whether hardware acceleration is in use.
 - Rational attributes (``time_base``, ``average_rate``, ``base_rate``, ``guessed_rate``, ``framerate``, ``rate``, ``sample_aspect_ratio``, and ``display_aspect_ratio``) now return :class:`av.AVRational` rather than ``fractions.Fraction``, and are never ``None``: an unset value is the falsy ``AVRational(0, 1)``. Test them with ``if not stream.time_base:`` instead of ``is None``. Setters still accept a ``fractions.Fraction``.
 - Remove ``Capabilities.hwaccel``, ``Capabilities.hwaccel_vdpau``, and ``Capabilities.neg_linesizes``, none of which FFmpeg defines any more.
+- Remove the ``metadata_encoding`` and ``metadata_errors`` arguments to :func:`av.open`, and the matching attributes. Metadata is now always read and written as UTF-8 with ``surrogateescape``, which is byte exact: a tag in another encoding survives as surrogates and is recovered per key with ``value.encode("utf-8", "surrogateescape").decode("cp1251")``. Previously one encoding had to be chosen for a whole container, so a file mixing encodings across tags could not be represented at all.
 - Remove the ``stream_options`` argument to :func:`av.open` and the matching attribute. They only ever reached ``avformat_find_stream_info()``, and only for formats that expose their streams before it runs, so they raised for MPEG and friends; output containers rejected them outright. Pass ``options`` for every stream, set ``stream.codec_context.options`` for one, and ``Container.add_stream(..., options={})`` when writing.
 
 Features:

av/container/core.pxd

@@ -17,8 +17,6 @@ ctypedef struct timeout_info:
 cdef class Container:
     cdef lib.AVFormatContext *ptr
     cdef readonly str name
-    cdef readonly str metadata_encoding
-    cdef readonly str metadata_errors
     cdef readonly PyIOFile file
     cdef int buffer_size
     cdef readonly object io_open

av/container/core.py

@@ -89,9 +89,7 @@ def pyav_io_open_gil(
                     cython.cast(
                         cython.pointer[cython.pointer[lib.AVDictionary]], options
                     )
-                ),
-                encoding=container.metadata_encoding,
-                errors=container.metadata_errors,
+                )
             )
         else:
             options_dict = {}
@@ -235,8 +233,6 @@ def __cinit__(
         options,
         container_options,
         hwaccel,
-        metadata_encoding,
-        metadata_errors,
         buffer_size,
         open_timeout,
         read_timeout,
@@ -257,8 +253,6 @@ def __cinit__(
         self.options = dict(options or ())
         self.container_options = dict(container_options or ())
         self.hwaccel = hwaccel
-        self.metadata_encoding = metadata_encoding
-        self.metadata_errors = metadata_errors
         self.open_timeout = open_timeout
         self.read_timeout = read_timeout
         self.buffer_size = buffer_size
@@ -428,9 +422,7 @@ def chapters(self):
                     "start": ch.start,
                     "end": ch.end,
                     "time_base": from_avrational(ch.time_base),
-                    "metadata": avdict_to_dict(
-                        ch.metadata, self.metadata_encoding, self.metadata_errors
-                    ),
+                    "metadata": avdict_to_dict(ch.metadata),
                 }
             )
         return result
@@ -473,12 +465,7 @@ def set_chapters(self, chapters):
             to_avrational(entry["time_base"], cython.address(ch.time_base))
             ch.metadata = cython.NULL
             if "metadata" in entry:
-                dict_to_avdict(
-                    cython.address(ch.metadata),
-                    entry["metadata"],
-                    self.metadata_encoding,
-                    self.metadata_errors,
-                )
+                dict_to_avdict(cython.address(ch.metadata), entry["metadata"])
             ch_array[i] = ch
 
         self.ptr.nb_chapters = cython.cast(cython.uint, count)
@@ -491,8 +478,6 @@ def open(
     format=None,
     options=None,
     container_options=None,
-    metadata_encoding="utf-8",
-    metadata_errors="strict",
     buffer_size=32768,
     timeout=None,
     io_open=None,
@@ -507,10 +492,6 @@ def open(
     :param str format: Specific format to use. Defaults to autodect.
     :param dict options: Options to pass to the container and all streams.
     :param dict container_options: Options to pass to the container.
-    :param str metadata_encoding: Encoding to use when reading or writing file metadata.
-        Defaults to ``"utf-8"``.
-    :param str metadata_errors: Specifies how to handle encoding errors; behaves like
-        ``str.encode`` parameter. Defaults to ``"strict"``.
     :param int buffer_size: Size of buffer for Python input/output operations in bytes.
         Honored only when ``file`` is a file-like object. Defaults to 32768 (32k).
     :param timeout: How many seconds to wait for data before giving up, as a float, or a
@@ -574,8 +555,6 @@ def open(
             options,
             container_options,
             hwaccel,
-            metadata_encoding,
-            metadata_errors,
             buffer_size,
             open_timeout,
             read_timeout,
@@ -589,8 +568,6 @@ def open(
         options,
         container_options,
         None,
-        metadata_encoding,
-        metadata_errors,
         buffer_size,
         open_timeout,
         read_timeout,

av/container/core.pyi

@@ -78,8 +78,6 @@ class Chapter(TypedDict):
 
 class Container:
     name: str
-    metadata_encoding: str
-    metadata_errors: str
     file: Any
     buffer_size: int
     io_open: Any
@@ -113,8 +111,6 @@ def open(
     format: str | None = None,
     options: dict[str, str] | None = None,
     container_options: dict[str, str] | None = None,
-    metadata_encoding: str = "utf-8",
-    metadata_errors: str = "strict",
     buffer_size: int = 32768,
     timeout: Real | None | tuple[Real | None, Real | None] = None,
     io_open: Callable[..., Any] | None = None,
@@ -127,8 +123,6 @@ def open(
     format: str | None = None,
     options: dict[str, str] | None = None,
     container_options: dict[str, str] | None = None,
-    metadata_encoding: str = "utf-8",
-    metadata_errors: str = "strict",
     buffer_size: int = 32768,
     timeout: Real | None | tuple[Real | None, Real | None] = None,
     io_open: Callable[..., Any] | None = None,
@@ -141,8 +135,6 @@ def open(
     format: str | None = None,
     options: dict[str, str] | None = None,
     container_options: dict[str, str] | None = None,
-    metadata_encoding: str = "utf-8",
-    metadata_errors: str = "strict",
     buffer_size: int = 32768,
     timeout: Real | None | tuple[Real | None, Real | None] = None,
     io_open: Callable[..., Any] | None = None,
@@ -155,8 +147,6 @@ def open(
     format: str | None = None,
     options: dict[str, str] | None = None,
     container_options: dict[str, str] | None = None,
-    metadata_encoding: str = "utf-8",
-    metadata_errors: str = "strict",
     buffer_size: int = 32768,
     timeout: Real | None | tuple[Real | None, Real | None] = None,
     io_open: Callable[..., Any] | None = None,

av/container/input.py

@@ -89,9 +89,7 @@ def __cinit__(self, *args, **kwargs):
                 "Hardware accelerated decode requested but no stream is compatible"
             )
 
-        self._metadata = avdict_to_dict(
-            self.ptr.metadata, self.metadata_encoding, self.metadata_errors
-        )
+        self._metadata = avdict_to_dict(self.ptr.metadata)
 
     def __dealloc__(self):
         close_input(self)

av/container/output.py

@@ -523,12 +523,7 @@ def start_encoding(self):
             )
 
         # Copy the metadata dict.
-        dict_to_avdict(
-            cython.address(self.ptr.metadata),
-            self.metadata,
-            encoding=self.metadata_encoding,
-            errors=self.metadata_errors,
-        )
+        dict_to_avdict(cython.address(self.ptr.metadata), self.metadata)
 
         all_options: Dictionary = Dictionary(self.options, self.container_options)
         options: Dictionary = all_options.copy()

av/frame.py

@@ -187,7 +187,7 @@ def side_data(self):
     @property
     def metadata(self):
         """Metadata attached to the frame by FFmpeg."""
-        return avdict_to_dict(self.ptr.metadata, "utf-8", "strict")
+        return avdict_to_dict(self.ptr.metadata)
 
     def make_writable(self):
         """

av/stream.py

@@ -120,11 +120,7 @@ def _init(
 
         self.codec_context = codec_context
 
-        self.metadata = avdict_to_dict(
-            stream.metadata,
-            encoding=self.container.metadata_encoding,
-            errors=self.container.metadata_errors,
-        )
+        self.metadata = avdict_to_dict(stream.metadata)
 
     @cython.cfunc
     def _is_open(self) -> cython.bint:
@@ -176,12 +172,7 @@ def __setattr__(self, name, value):
 
     @cython.cfunc
     def _finalize_for_output(self) -> cython.void:
-        dict_to_avdict(
-            cython.address(self.ptr.metadata),
-            self.metadata,
-            encoding=self.container.metadata_encoding,
-            errors=self.container.metadata_errors,
-        )
+        dict_to_avdict(cython.address(self.ptr.metadata), self.metadata)
 
         if self.codec_context is None:
             return

av/utils.pxd

@@ -1,8 +1,8 @@
 cimport libav as lib
 
 
-cdef dict avdict_to_dict(lib.AVDictionary *input, str encoding, str errors)
-cdef void dict_to_avdict(lib.AVDictionary **dst, dict src, str encoding, str errors)
+cdef dict avdict_to_dict(lib.AVDictionary *input)
+cdef void dict_to_avdict(lib.AVDictionary **dst, dict src)
 
 cdef void to_avrational(object frac, lib.AVRational *input)
 cdef void check_ndarray(object array, object dtype, int ndim)

av/utils.py

@@ -5,39 +5,35 @@
 
 
 @cython.cfunc
-def _decode(s: cython.pointer[cython.char], encoding, errors) -> str:
-    return cython.cast(bytes, s).decode(encoding, errors)
+def _decode(s: cython.pointer[cython.char]) -> str:
+    return cython.cast(bytes, s).decode("utf-8", "surrogateescape")
 
 
 @cython.cfunc
-def avdict_to_dict(
-    input: cython.pointer[lib.AVDictionary], encoding: str, errors: str
-) -> dict:
+def avdict_to_dict(input: cython.pointer[lib.AVDictionary]) -> dict:
     element: cython.pointer[lib.AVDictionaryEntry] = cython.NULL
     output: dict = {}
     while True:
         element = lib.av_dict_get(input, "", element, lib.AV_DICT_IGNORE_SUFFIX)
         if element == cython.NULL:
             break
-        output[_decode(element.key, encoding, errors)] = _decode(
-            element.value, encoding, errors
-        )
+        output[_decode(element.key)] = _decode(element.value)
 
     return output
 
 
 @cython.cfunc
 def dict_to_avdict(
-    dst: cython.pointer[cython.pointer[lib.AVDictionary]],
-    src: dict,
-    encoding: str,
-    errors: str,
+    dst: cython.pointer[cython.pointer[lib.AVDictionary]], src: dict
 ) -> cython.void:
     lib.av_dict_free(dst)
     for key, value in src.items():
         err_check(
             lib.av_dict_set(
-                dst, key.encode(encoding, errors), value.encode(encoding, errors), 0
+                dst,
+                key.encode("utf-8", "surrogateescape"),
+                value.encode("utf-8", "surrogateescape"),
+                0,
             )
         )
 

docs/api/container.rst

@@ -14,8 +14,6 @@ Generic
 
     .. attribute:: options
     .. attribute:: container_options
-    .. attribute:: metadata_encoding
-    .. attribute:: metadata_errors
     .. attribute:: open_timeout
     .. attribute:: read_timeout
 

tests/test_encode.py

@@ -662,3 +662,31 @@ def test_hardware_encode_honors_sw_format() -> None:
     for packet in stream.encode():
         container.mux(packet)
     container.close()
+
+
+def test_metadata_survives_non_utf8_bytes(tmp_path) -> None:
+    # FFmpeg hands tags back as bytes with no declared encoding, so PyAV reads
+    # them as UTF-8 with surrogateescape. That has to be byte exact both ways,
+    # or a tag written in some other encoding is destroyed by a round trip.
+    raw = "café".encode("latin-1")
+    tag = raw.decode("utf-8", "surrogateescape")
+    path = str(tmp_path / "metadata.mkv")
+
+    with av.open(path, "w") as output:
+        stream = output.add_stream("mpeg4", rate=24)
+        assert isinstance(stream, VideoStream)
+        stream.width = stream.height = 64
+        stream.pix_fmt = "yuv420p"
+        output.metadata["title"] = tag
+        stream.metadata["note"] = tag
+        output.mux(stream.encode(VideoFrame(64, 64, "yuv420p")))
+        output.mux(stream.encode(None))
+
+    with av.open(path) as input_:
+        # Matroska upper cases the keys it does not know.
+        title = input_.metadata["title"]
+        note = input_.streams[0].metadata["NOTE"]
+
+    assert title.encode("utf-8", "surrogateescape") == raw
+    assert note.encode("utf-8", "surrogateescape") == raw
+    assert title.encode("utf-8", "surrogateescape").decode("latin-1") == "café"