PyAV-Org / PyAV

Pythonic bindings for FFmpeg's libraries.
https://pyav.basswood-io.com/
BSD 3-Clause "New" or "Revised" License
2.43k stars 359 forks source link

Add S12M_TIMECODE to av.sidedata.sidedata.Type Enum #1359

Closed ivan94fi closed 4 months ago

ivan94fi commented 5 months ago

Overview

Adding S12M_TIMECODE field to the av.sidedata.sidedata.Type Enum would give the ability to read timecode information from SEI metadata.

Existing FFmpeg API

The existing FFmpeg enum is defined in libavutil/frame.h and is called AVFrameSideDataType.

The documentation is at https://ffmpeg.org/doxygen/trunk/group__lavu__frame.html#gae01fa7e427274293aacdf2adc17076bc.

The AV_FRAME_DATA_S12M_TIMECODE field is described as:

    /**
     * Timecode which conforms to SMPTE ST 12-1. The data is an array of 4 uint32_t
     * where the first uint32_t describes how many (1-3) of the other timecodes are used.
     * The timecode format is described in the documentation of av_timecode_get_smpte_from_framenum()
     * function in libavutil/timecode.h.
     */
    AV_FRAME_DATA_S12M_TIMECODE

This is the link to the AV_FRAME_DATA_S12M_TIMECODE field in the docs: https://ffmpeg.org/doxygen/trunk/group__lavu__frame.html#ggae01fa7e427274293aacdf2adc17076bca9f4e4ed5a874d1089ec07c384b81bb70

Expected PyAV API

PyAV should have a S12M_TIMECODE field in the av.sidedata.sidedata.Type Enum, which is currently defined as: https://github.com/PyAV-Org/PyAV/blob/1b27fc96ff3d1dc9eb7774f065a5e8e9c43475d8/av/sidedata/sidedata.pyx#L11-L29

Example:

import av

container = av.open("video.mkv")

for frame in container.decode(video=0):
    timecode_data = frame.side_data.get(av.sidedata.sidedata.Type.S12M_TIMECODE)
    # ... use timecode_data

Investigation

N/A

Reproduction

N/A

Versions

Additional context

I tried manually adding a S12M_TIMECODE field to av.sidedata.sidedata, with this edits on the latest commit on main branch (1dfe7763023102a98cf94416c456eaafe67fdb93):

diff --git a/av/sidedata/sidedata.pyi b/av/sidedata/sidedata.pyi
index ac28f0d..e814bb2 100644
--- a/av/sidedata/sidedata.pyi
+++ b/av/sidedata/sidedata.pyi
@@ -23,6 +23,7 @@ class Type(EnumItem):
     CONTENT_LIGHT_LEVEL: int
     ICC_PROFILE: int
     SEI_UNREGISTERED: int
+    S12M_TIMECODE: int

 class SideData(Buffer):
     type: Type
diff --git a/av/sidedata/sidedata.pyx b/av/sidedata/sidedata.pyx
index 05ed002..1e62dff 100644
--- a/av/sidedata/sidedata.pyx
+++ b/av/sidedata/sidedata.pyx
@@ -26,6 +26,7 @@ Type = define_enum("Type", __name__, (
     ("CONTENT_LIGHT_LEVEL", lib.AV_FRAME_DATA_CONTENT_LIGHT_LEVEL),
     ("ICC_PROFILE", lib.AV_FRAME_DATA_ICC_PROFILE),
     ("SEI_UNREGISTERED", lib.AV_FRAME_DATA_SEI_UNREGISTERED) if lib.AV_FRAME_DATA_SEI_UNREGISTERED != -1 else None,
+    ("S12M_TIMECODE", lib.AV_FRAME_DATA_S12M_TIMECODE) if lib.AV_FRAME_DATA_S12M_TIMECODE != -1 else None,
 ))

But the compilation fails with this message:

Compiling av/sidedata/sidedata.pyx because it changed.
[1/1] Cythonizing av/sidedata/sidedata.pyx

Error compiling Cython file:
------------------------------------------------------------
...
    ("GOP_TIMECODE", lib.AV_FRAME_DATA_GOP_TIMECODE),
    ("SPHERICAL", lib.AV_FRAME_DATA_SPHERICAL),
    ("CONTENT_LIGHT_LEVEL", lib.AV_FRAME_DATA_CONTENT_LIGHT_LEVEL),
    ("ICC_PROFILE", lib.AV_FRAME_DATA_ICC_PROFILE),
    ("SEI_UNREGISTERED", lib.AV_FRAME_DATA_SEI_UNREGISTERED) if lib.AV_FRAME_DATA_SEI_UNREGISTERED != -1 else None,
    ("S12M_TIMECODE", lib.AV_FRAME_DATA_S12M_TIMECODE) if lib.AV_FRAME_DATA_S12M_TIMECODE != -1 else None,
                                                             ^
------------------------------------------------------------

av/sidedata/sidedata.pyx:29:61: cimported module has no attribute 'AV_FRAME_DATA_S12M_TIMECODE'

Error compiling Cython file:
------------------------------------------------------------
...
    ("GOP_TIMECODE", lib.AV_FRAME_DATA_GOP_TIMECODE),
    ("SPHERICAL", lib.AV_FRAME_DATA_SPHERICAL),
    ("CONTENT_LIGHT_LEVEL", lib.AV_FRAME_DATA_CONTENT_LIGHT_LEVEL),
    ("ICC_PROFILE", lib.AV_FRAME_DATA_ICC_PROFILE),
    ("SEI_UNREGISTERED", lib.AV_FRAME_DATA_SEI_UNREGISTERED) if lib.AV_FRAME_DATA_SEI_UNREGISTERED != -1 else None,
    ("S12M_TIMECODE", lib.AV_FRAME_DATA_S12M_TIMECODE) if lib.AV_FRAME_DATA_S12M_TIMECODE != -1 else None,
                         ^
------------------------------------------------------------

av/sidedata/sidedata.pyx:29:25: cimported module has no attribute 'AV_FRAME_DATA_S12M_TIMECODE'
Traceback (most recent call last):
  File "/code/PyAV/setup.py", line 156, in <module>
    ext_modules += cythonize(
  File "/code/PyAV/venvs/Linux.6.5.0-21-generic.cpython3.10/lib/python3.10/site-packages/Cython/Build/Dependencies.py", line 1154, in cythonize
    cythonize_one(*args)
  File "/code/PyAV/venvs/Linux.6.5.0-21-generic.cpython3.10/lib/python3.10/site-packages/Cython/Build/Dependencies.py", line 1321, in cythonize_one
    raise CompileError(None, pyx_file)
Cython.Compiler.Errors.CompileError: av/sidedata/sidedata.pyx
make: *** [Makefile:14: build] Error 1

From FFmpeg sources, I can see that AV_FRAME_DATA_S12M_TIMECODE was added in libavutil version 56.20.100:

https://github.com/FFmpeg/FFmpeg/blob/89215237dd6ac64f94e14aa20a000e0440a00aac/doc/APIchanges#L894-L895

And I have installed the package libavutil-dev version 7:4.4.2-0ubuntu0.22.04.1 and libavutil56, which for Ubuntu22.04 provide libavutil version 56.70.100 as can be seen in the following lines:

$ dpkg -L libavutil-dev
# ....
/usr/lib/x86_64-linux-gnu/pkgconfig/libavutil.pc
# ....
$ cat /usr/lib/x86_64-linux-gnu/pkgconfig/libavutil.pc
prefix=/usr
exec_prefix=${prefix}
libdir=/usr/lib/x86_64-linux-gnu
includedir=/usr/include/x86_64-linux-gnu

Name: libavutil
Description: FFmpeg utility library
Version: 56.70.100
Requires:
Requires.private:
Conflicts:
Libs: -L${libdir}  -lavutil
Libs.private: -pthread -lva-drm -lva -lva-x11 -lva -lvdpau -lX11 -lm -ldrm -lmfx -lstdc++ -ldl -lOpenCL -lva -lXv -lX11 -lXext
Cflags: -I${includedir}
$ dpkg -L libavutil56
# ...
/usr/lib/x86_64-linux-gnu/libavutil.so.56.70.100
# ...

So the libavutil version should be sufficient, as far as I understand, but maybe I need something else to make this work, I do not have sufficient experience.