Message ID | 20160918114607.25944-1-timo@rothenpieler.org |
---|---|
State | Changes Requested |
Headers | show |
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 [...]
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; #endif