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

[Rogozhkin, Dmitry V]

On Sun, 2023-11-19 at 16:29 +0100, Hendrik Leppkes wrote:
> 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.

I am fine with that. Thank you for the text. Added with minor change in
title to shorten it. See v4 patch.

> 
> ---
> 
> 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
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".