[FFmpeg-devel] avformat/utils: only call h264 decoder private function if h264 decoder is in use

Fixes a crash when decoding with for example h264_cuvid, as
avpriv_h264_has_num_reorder_frames assumes the AVCodecContext->priv_data
to be a H264Context.
---
 libavformat/utils.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

On Sun, Sep 18, 2016 at 01:46:07PM +0200, Timo Rothenpieler wrote:
> Fixes a crash when decoding with for example h264_cuvid, as
> avpriv_h264_has_num_reorder_frames assumes the AVCodecContext->priv_data
> to be a H264Context.
> ---
>  libavformat/utils.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Relevant IRC discussion:

<michaelni> BtbN, JEEB why is st->internal->avctx not the libavcodec software decoder ?
<BtbN> because it's the cuvid h264 decoder.
<michaelni> I mean if this is the hw decoder you would be running the hw decoder twice
<michaelni> once in the application and once in libavformat
<michaelni> is that intended ?
<BtbN> hm?
<michaelni> or am i misunderstanding?
<BtbN> the cuvid h264 decoder is completely inside of lavc, completely independend from the outside
<michaelni> sure but libavformat opens a decoder to get some information about a stream, if thats the hw decoder then there would be 2 as the user app would also have a decoder independat of libavformat
<BtbN> It indeed seems to open it twice
<BtbN> But I don't see anything I could possibly do about that from inside of the cuvid decoder, and it also works just fine.
<michaelni> not from the inside but the code setting up st->internal->avctx should maybe not open a hw decoder
<michaelni> or if we want it to do that then indeed the avpriv_h264_has_num_reorder_frames() call is garbage
<BtbN> Forcing it to use the software decoder for the initial parsing seems like the better solution. As it returns way more information about the stream.

[...]

On Sun, Sep 18, 2016 at 01:46:07PM +0200, Timo Rothenpieler wrote:
> Fixes a crash when decoding with for example h264_cuvid, as
> avpriv_h264_has_num_reorder_frames assumes the AVCodecContext->priv_data
> to be a H264Context.
> ---
>  libavformat/utils.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index d605a96..06003dd 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -935,7 +935,7 @@ static int has_decode_delay_been_guessed(AVStream *st)
>      if (!st->info) // if we have left find_stream_info then nb_decoded_frames won't increase anymore for stream copy
>          return 1;
>  #if CONFIG_H264_DECODER
> -    if (st->internal->avctx->has_b_frames &&
> +    if (st->internal->avctx->has_b_frames && !strcmp(st->internal->avctx->codec->name, "h264") &&
>         avpriv_h264_has_num_reorder_frames(st->internal->avctx) == st->internal->avctx->has_b_frames)
>          return 1;

has the issue been fixed for all branches an cases or is something
missing that needs this ?
(if so this needs a null pointer check i think)


[...]

> has the issue been fixed for all branches an cases or is something
> missing that needs this ?
> (if so this needs a null pointer check i think)

This was fixed in a diffrent way in 6d9a46e884d090a68069112a3b0436aa8b563967

It forces the h264 decoder to be used, so the assumption it is in use is
allways correct.

On Sun, Oct 30, 2016 at 12:04:14PM +0100, Timo Rothenpieler wrote:
> > has the issue been fixed for all branches an cases or is something
> > missing that needs this ?
> > (if so this needs a null pointer check i think)
> 
> This was fixed in a diffrent way in 6d9a46e884d090a68069112a3b0436aa8b563967
> 
> It forces the h264 decoder to be used, so the assumption it is in use is
> allways correct.

My question is, does either patch need to be backported to some
branch ?
6d9a46e884d090a68069112a3b0436aa8b563967 is not in 3.1 for example

[...]