[FFmpeg-devel] [PATCH v3] avcodec/decode: guard against NULL hw_frames_ctx

[Hendrik Leppkes]

On Fri, Nov 17, 2023 at 6:04 PM Dmitry Rogozhkin
<dmitry.v.rogozhkin-at-intel.com at ffmpeg.org> wrote:
>
> Guard against segfault running VLC decoding under msys2 [1]:
>
> Thread 33 received signal SIGSEGV, Segmentation fault.
> [Switching to Thread 37728.0xadd0]
> ff_hwaccel_frame_priv_alloc (avctx=0x6447b00, hwaccel_picture_private=0x65dfd00)
>     at libavcodec/decode.c:1848
> 1848        frames_ctx = (AVHWFramesContext *)avctx->hw_frames_ctx->data;
> (gdb) bt
>     at libavcodec/decode.c:1848
>     at libavcodec/h264_slice.c:208
>     first_slice=1) at libavcodec/h264_slice.c:1599
>     at libavcodec/h264_slice.c:2130
>     at libavcodec/h264dec.c:652
>     got_frame=0x646e4b0, avpkt=0x64522c0) at libavcodec/h264dec.c:1048
>
> (gdb) p avctx
> $1 = (AVCodecContext *) 0x6447b00
> (gdb) p avctx->hw_frames_ctx
> $2 = (AVBufferRef *) 0x0
>
> v2: check for free_frame_priv (Hendrik)
> v3: return EINVAL (Christoph Reiter)
>
> See[1]: https://github.com/msys2/MINGW-packages/pull/19050
> Fixes: be07145109 ("avcodec: add AVHWAccel.free_frame_priv callback")
> CC: Lynne <dev at lynne.ee>
> CC: Christoph Reiter <reiter.christoph at gmail.com>
> Signed-off-by: Dmitry Rogozhkin <dmitry.v.rogozhkin at intel.com>
> ---

The latest change itself looks fine to me, I would however prefer if
the commit message would contain a bit more text and a bit less gdb
dump.

Maybe something like this (quick suggestion, for title and body):

avcodec: validate hw_frames_ctx is available when
AVHWAccel.free_frame_priv is used

Validate that a hw_frames_ctx is available before using it for the
AVHWAccel.free_frame_priv callback, and don't require it to be present when
the callback is not in use by the HWAccel.

---

Feel free to augment and reword, but I don't think the gdb dump offers
much info that couldn't be conveyed more clearly in a few words that
mention the absence of hw_frame_ctx.

- Hendrik