[FFmpeg-devel] [PATCH] avcodec/nvenc: Fix segfault with intra-only

[FFmpeg-devel] [PATCH] avcodec/nvenc: Fix segfault with intra-only

[Josh Allmann]

In intra-only mode, frameIntervalP is 0, which means the frame
data array is smaller than the number of surfaces. This causes a
crash when closing the encoder.

Fix this by making sure the frame data array is at least as big as
the number of surfaces.
---
 libavcodec/nvenc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/nvenc.c b/libavcodec/nvenc.c
index a9945355ba..93e87b21db 100644
--- a/libavcodec/nvenc.c
+++ b/libavcodec/nvenc.c
@@ -1021,7 +1021,7 @@ static av_cold int nvenc_recalc_surfaces(AVCodecContext *avctx)
 
     // Output in the worst case will only start when the surface buffer is completely full.
     // Hence we need to keep at least the max amount of surfaces plus the max reorder delay around.
-    ctx->frame_data_array_nb = ctx->nb_surfaces + ctx->encode_config.frameIntervalP - 1;
+    ctx->frame_data_array_nb = FFMAX(ctx->nb_surfaces, ctx->nb_surfaces + ctx->encode_config.frameIntervalP - 1);
 
     return 0;
 }
-- 
2.39.2

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avcodec/nvenc: Fix segfault with intra-only

[Josh Allmann]

On Thu, 20 Jun 2024 at 17:39, Josh Allmann <joshua.allmann@gmail.com> wrote:
>
> In intra-only mode, frameIntervalP is 0, which means the frame
> data array is smaller than the number of surfaces. This causes a
> crash when closing the encoder.
>
> Fix this by making sure the frame data array is at least as big as
> the number of surfaces.
> ---
>  libavcodec/nvenc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavcodec/nvenc.c b/libavcodec/nvenc.c
> index a9945355ba..93e87b21db 100644
> --- a/libavcodec/nvenc.c
> +++ b/libavcodec/nvenc.c
> @@ -1021,7 +1021,7 @@ static av_cold int nvenc_recalc_surfaces(AVCodecContext *avctx)
>
>      // Output in the worst case will only start when the surface buffer is completely full.
>      // Hence we need to keep at least the max amount of surfaces plus the max reorder delay around.
> -    ctx->frame_data_array_nb = ctx->nb_surfaces + ctx->encode_config.frameIntervalP - 1;
> +    ctx->frame_data_array_nb = FFMAX(ctx->nb_surfaces, ctx->nb_surfaces + ctx->encode_config.frameIntervalP - 1);
>
>      return 0;
>  }
> --
> 2.39.2
>

Hello,

Ping for review. This patch fixes an easily triggered crash with nvenc
in intra-only mode, eg

ffmpeg -i <in> -c:v h264_nvenc -g 0 <out>

Josh
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avcodec/nvenc: Fix segfault with intra-only

[Timo Rothenpieler]

On 01.07.2024 18:52, Josh Allmann wrote:
> On Thu, 20 Jun 2024 at 17:39, Josh Allmann <joshua.allmann@gmail.com> wrote:
>>
>> In intra-only mode, frameIntervalP is 0, which means the frame
>> data array is smaller than the number of surfaces. This causes a
>> crash when closing the encoder.
>>
>> Fix this by making sure the frame data array is at least as big as
>> the number of surfaces.
>> ---
>>   libavcodec/nvenc.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/libavcodec/nvenc.c b/libavcodec/nvenc.c
>> index a9945355ba..93e87b21db 100644
>> --- a/libavcodec/nvenc.c
>> +++ b/libavcodec/nvenc.c
>> @@ -1021,7 +1021,7 @@ static av_cold int nvenc_recalc_surfaces(AVCodecContext *avctx)
>>
>>       // Output in the worst case will only start when the surface buffer is completely full.
>>       // Hence we need to keep at least the max amount of surfaces plus the max reorder delay around.
>> -    ctx->frame_data_array_nb = ctx->nb_surfaces + ctx->encode_config.frameIntervalP - 1;
>> +    ctx->frame_data_array_nb = FFMAX(ctx->nb_surfaces, ctx->nb_surfaces + ctx->encode_config.frameIntervalP - 1);
>>
>>       return 0;
>>   }
>> --
>> 2.39.2
>>
> 
> Hello,
> 
> Ping for review. This patch fixes an easily triggered crash with nvenc
> in intra-only mode, eg
> 
> ffmpeg -i <in> -c:v h264_nvenc -g 0 <out>
> 
> Josh
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

It's the wrong fix for the crash, but definitely a sensible change in 
itself.
Will amend with the actual fix (using the right size when deallocating 
that array) but keep this change as well, since it makes sense.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avcodec/nvenc: Fix segfault with intra-only

[Dennis Mungai]

On Mon, 1 Jul 2024 at 21:01, Timo Rothenpieler <timo@rothenpieler.org>
wrote:

> On 01.07.2024 18:52, Josh Allmann wrote:
> > On Thu, 20 Jun 2024 at 17:39, Josh Allmann <joshua.allmann@gmail.com>
> wrote:
> >>
> >> In intra-only mode, frameIntervalP is 0, which means the frame
> >> data array is smaller than the number of surfaces. This causes a
> >> crash when closing the encoder.
> >>
> >> Fix this by making sure the frame data array is at least as big as
> >> the number of surfaces.
> >> ---
> >>   libavcodec/nvenc.c | 2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/libavcodec/nvenc.c b/libavcodec/nvenc.c
> >> index a9945355ba..93e87b21db 100644
> >> --- a/libavcodec/nvenc.c
> >> +++ b/libavcodec/nvenc.c
> >> @@ -1021,7 +1021,7 @@ static av_cold int
> nvenc_recalc_surfaces(AVCodecContext *avctx)
> >>
> >>       // Output in the worst case will only start when the surface
> buffer is completely full.
> >>       // Hence we need to keep at least the max amount of surfaces plus
> the max reorder delay around.
> >> -    ctx->frame_data_array_nb = ctx->nb_surfaces +
> ctx->encode_config.frameIntervalP - 1;
> >> +    ctx->frame_data_array_nb = FFMAX(ctx->nb_surfaces,
> ctx->nb_surfaces + ctx->encode_config.frameIntervalP - 1);
> >>
> >>       return 0;
> >>   }
> >> --
> >> 2.39.2
> >>
> >
> > Hello,
> >
> > Ping for review. This patch fixes an easily triggered crash with nvenc
> > in intra-only mode, eg
> >
> > ffmpeg -i <in> -c:v h264_nvenc -g 0 <out>
> >
> > Josh
>

Can this also be back-ported to FFmpeg 7.x upon merging?
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avcodec/nvenc: Fix segfault with intra-only

[Timo Rothenpieler]

On 01.07.2024 20:05, Dennis Mungai wrote:
> On Mon, 1 Jul 2024 at 21:01, Timo Rothenpieler <timo@rothenpieler.org>
> wrote:
> 
>> On 01.07.2024 18:52, Josh Allmann wrote:
>>> On Thu, 20 Jun 2024 at 17:39, Josh Allmann <joshua.allmann@gmail.com>
>> wrote:
>>>>
>>>> In intra-only mode, frameIntervalP is 0, which means the frame
>>>> data array is smaller than the number of surfaces. This causes a
>>>> crash when closing the encoder.
>>>>
>>>> Fix this by making sure the frame data array is at least as big as
>>>> the number of surfaces.
>>>> ---
>>>>    libavcodec/nvenc.c | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/libavcodec/nvenc.c b/libavcodec/nvenc.c
>>>> index a9945355ba..93e87b21db 100644
>>>> --- a/libavcodec/nvenc.c
>>>> +++ b/libavcodec/nvenc.c
>>>> @@ -1021,7 +1021,7 @@ static av_cold int
>> nvenc_recalc_surfaces(AVCodecContext *avctx)
>>>>
>>>>        // Output in the worst case will only start when the surface
>> buffer is completely full.
>>>>        // Hence we need to keep at least the max amount of surfaces plus
>> the max reorder delay around.
>>>> -    ctx->frame_data_array_nb = ctx->nb_surfaces +
>> ctx->encode_config.frameIntervalP - 1;
>>>> +    ctx->frame_data_array_nb = FFMAX(ctx->nb_surfaces,
>> ctx->nb_surfaces + ctx->encode_config.frameIntervalP - 1);
>>>>
>>>>        return 0;
>>>>    }
>>>> --
>>>> 2.39.2
>>>>
>>>
>>> Hello,
>>>
>>> Ping for review. This patch fixes an easily triggered crash with nvenc
>>> in intra-only mode, eg
>>>
>>> ffmpeg -i <in> -c:v h264_nvenc -g 0 <out>
>>>
>>> Josh
>>
> 
> Can this also be back-ported to FFmpeg 7.x upon merging?

Already is, applies to 7.0 and 6.x
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".