Setter for VideoCodecContext.pix_fmt is not validated, application segfaults

moi90 commented 2 years ago

Overview

VideoCodecContext.pix_fmt can be set to an illegal value. Later, the application segfaults.

Expected behavior

If VideoCodecContext.pix_fmt is set to an illegal value, a ValueError should be raised.

Actual behavior

If VideoCodecContext.pix_fmt is set to an illegal value, it is silently unset.

Traceback:

Specified pixel format -1 is invalid or not supported
Speicherzugriffsfehler (Speicherabzug geschrieben)

Reproduction

import numpy as np

import av

duration = 4
fps = 24
total_frames = duration * fps

with av.open('test.mp4', mode='w') as container:

    stream = container.add_stream('mpeg4', rate=fps)
    stream.width = 480
    stream.height = 320
    # stream.pix_fmt = 'yuv420p' # This works
    stream.pix_fmt = 'yuv240p' # Illegal value

    print("stream", stream)

    for frame_i in range(total_frames):

        img = np.empty((320, 480, 3))
        img[:, :, 0] = 0.5 + 0.5 * np.sin(2 * np.pi * (0 / 3 + frame_i / total_frames))
        img[:, :, 1] = 0.5 + 0.5 * np.sin(2 * np.pi * (1 / 3 + frame_i / total_frames))
        img[:, :, 2] = 0.5 + 0.5 * np.sin(2 * np.pi * (2 / 3 + frame_i / total_frames))

        img = np.round(255 * img).astype(np.uint8)
        img = np.clip(img, 0, 255)

        print("img", img.shape, img.dtype)

        frame = av.VideoFrame.from_ndarray(img, format='rgb24')

        print("frame", frame)

        # Here, the Segfault happens:
        for packet in stream.encode(frame):
            container.mux(packet)
        break

    # Flush stream
    for packet in stream.encode():
        container.mux(packet)

Versions

OS: Ubuntu 20.04.3 LTS

PyAV runtime:

PyAV v8.0.4.dev0
git origin: git@github.com:PyAV-Org/PyAV
git commit: v8.0.3-11-g9ac05d9
library configuration: --prefix=/home/moi/Work/21-Dimitrii/.venv --cc=/home/conda/feedstock_root/build_artifacts/ffmpeg_1627813612080/_build_env/bin/x86_64-conda-linux-gnu-cc --disable-doc --disable-openssl --enable-avresample --enable-gnutls --enable-gpl --enable-hardcoded-tables --enable-libfreetype --enable-libopenh264 --enable-libx264 --enable-pic --enable-pthreads --enable-shared --enable-static --enable-version3 --enable-zlib --enable-libmp3lame --pkg-config=/home/conda/feedstock_root/build_artifacts/ffmpeg_1627813612080/_build_env/bin/pkg-config
library license: GPL version 3 or later
libavcodec     58. 91.100
libavdevice    58. 10.100
libavfilter     7. 85.100
libavformat    58. 45.100
libavutil      56. 51.100
libswresample   3.  7.100
libswscale      5.  7.100

PyAV build:

PyAV: 8.0.4.dev0 v8.0.3-11-g9ac05d9
Python: 3.9.7 | packaged by conda-forge | (default, Sep 14 2021, 01:17:55) \n[GCC 9.4.0]
platform: Linux-5.4.0-86-generic-x86_64-with-glibc2.31
extension_extra:
    include_dirs: [b'include', b'/usr/include/x86_64-linux-gnu']
    libraries: [b'avformat', b'avcodec', b'avdevice', b'avutil', b'avfilter', b'swscale', b'swresample']
    library_dirs: []
    define_macros: []
    runtime_library_dirs: []
config_macros:
    PYAV_COMMIT_STR="v8.0.3-11-g9ac05d9"
    PYAV_VERSION=8.0.4.dev0
    PYAV_VERSION_STR="8.0.4.dev0"

FFmpeg:

ffmpeg version 9c33b2f Copyright (c) 2000-2021 the FFmpeg developers
built with gcc 9.3.0 (crosstool-NG 1.24.0.133_b0863d8_dirty)
configuration: --prefix=/home/moi/Work/21-Dimitrii/.venv --cc=/home/conda/feedstock_root/build_artifacts/ffmpeg_1627813612080/_build_env/bin/x86_64-conda-linux-gnu-cc --disable-doc --disable-openssl --enable-avresample --enable-gnutls --enable-gpl --enable-hardcoded-tables --enable-libfreetype --enable-libopenh264 --enable-libx264 --enable-pic --enable-pthreads --enable-shared --enable-static --enable-version3 --enable-zlib --enable-libmp3lame --pkg-config=/home/conda/feedstock_root/build_artifacts/ffmpeg_1627813612080/_build_env/bin/pkg-config
libavutil      56. 51.100 / 56. 51.100
libavcodec     58. 91.100 / 58. 91.100
libavformat    58. 45.100 / 58. 45.100
libavdevice    58. 10.100 / 58. 10.100
libavfilter     7. 85.100 /  7. 85.100
libavresample   4.  0.  0 /  4.  0.  0
libswscale      5.  7.100 /  5.  7.100
libswresample   3.  7.100 /  3.  7.100
libpostproc    55.  7.100 / 55.  7.100

Research

I have done the following:

[ ] Checked the PyAV documentation
[ ] Searched on Google
[ ] Searched on Stack Overflow
[ ] Looked through old GitHub issues
[ ] Asked on PyAV Gitter
[ ] ... and waited 72 hours for a response.

moi90 commented 2 years ago

This can be fixed by implementing pix_fmt with format, like hinted in the source: https://github.com/PyAV-Org/PyAV/blob/9ac05d9ac902d71ecb2fe80f04dcae454008378c/av/video/codeccontext.pyx#L96

i3587616 commented 2 years ago

pix_fmt = 'yuv420p' like this ,you should see source code.it will help you

moi90 commented 2 years ago

@i3587616 Your comment is not helpful. It is absolutely unacceptable that a user of the library is able to set an illegal value and that the application crashes with a SEGFAULT at a later point.

jlaine commented 2 years ago

This is a good catch but I seem to have missed the corresponding PR?

moi90 commented 2 years ago

I'm not able to provide a PR in my limited time, but the fix mentioned above should be easy. Everyone is welcome to take credit.

jlaine commented 2 years ago

Ah right, but maintainers magically have unlimited time.

moi90 commented 2 years ago

Of cause! ;)

(I'd be happy to contribute, but I have zero experience with this library. Regular contributors, on the other hand, don't have the overhead of setting up a building and testing environment and digging into an unknown source. Eventually, someone will find the time. It might even be me, but not now.)

jlaine commented 2 years ago

An alternative would be to remove the pix_fmt setter, AFAICT it doesn't add anything over format?

moi90 commented 2 years ago

I wouldn't remove fix_fmt because with only format, a user would need to specify all three properties (pix_fmt, width, height) when setting.

PyAV-Org / PyAV