[FFmpeg-devel] Proposed change to NVDEC ulNumOutputSurfaces initialization

[FFmpeg-devel] Proposed change to NVDEC ulNumOutputSurfaces initialization

[Miller, Adrian]

Hi,

I'm considering a proposing a change to libavcodec/nvdec.c but wanted to run it by you first as I'm new to FFmpeg development to make sure I've gotten things right (this is not a patch).

We use the NVDEC decoder as part of our live transcoder. This means that new decoders may be spun up to accommodate switching sources but the frames_ctx->initial_pool_size is calculated based on values from the source's initial SPS and the decoder's "extra_hw_frames" option, so it's possible that a new source will result in an SPS+extra_hw_frames value greater than the max supported by the NVDEC decoder, 32.

The current behavior is to return if the NVDEC decoder fails to initialize and return a warning (nvdec.c:413). I'm sure this is fine in the interactive use case where the user can simply provide a smaller value for extra_hw_frames and try again. In the unattended case this isn't possible, and our application fails.

I was thinking something along the lines of in nvdec.c to handle this case:

-    params.ulNumDecodeSurfaces = frames_ctx->initial_pool_size;
-    params.ulNumOutputSurfaces = frames_ctx->initial_pool_size;
+    const int kMaxSurfaces = 32;
+    params.ulNumDecodeSurfaces = (frames_ctx->initial_pool_size <= kMaxSurfaces) ? frames_ctx->initial_pool_size : kMaxSurfaces;
+    params.ulNumOutputSurfaces = params.ulNumDecodeSurfaces;
+    if (frames_ctx->initial_pool_size > kMaxSurfaces) {
+        av_log(avctx, AV_LOG_WARNING, "Requested %d decode surfaces, which is more than %d. Condifuring decoder with %d surfaces.\n",
+               (int)frames_ctx->initial_pool_size, kMaxSurfaces, (int)params.ulNumDecodeSurfaces);
+    }

Plus, probably rewording the original warning that handles the case where the decoder still fails.

Thoughts? If this seems like a reasonable approach I'll put together a formal patch email after going through the rest of your submission process and the stuff I have to do for my employer.

Thanks,

Ade Miller


_______________________________________________
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] Proposed change to NVDEC ulNumOutputSurfaces initialization

[Timo Rothenpieler]

On 14.02.2023 18:27, Miller, Adrian wrote:
> 
> Hi,
> 
> I'm considering a proposing a change to libavcodec/nvdec.c but wanted to run it by you first as I'm new to FFmpeg development to make sure I've gotten things right (this is not a patch).
> 
> We use the NVDEC decoder as part of our live transcoder. This means that new decoders may be spun up to accommodate switching sources but the frames_ctx->initial_pool_size is calculated based on values from the source's initial SPS and the decoder's "extra_hw_frames" option, so it's possible that a new source will result in an SPS+extra_hw_frames value greater than the max supported by the NVDEC decoder, 32.
> 
> The current behavior is to return if the NVDEC decoder fails to initialize and return a warning (nvdec.c:413). I'm sure this is fine in the interactive use case where the user can simply provide a smaller value for extra_hw_frames and try again. In the unattended case this isn't possible, and our application fails.
> 
> I was thinking something along the lines of in nvdec.c to handle this case:
> 
> -    params.ulNumDecodeSurfaces = frames_ctx->initial_pool_size;
> -    params.ulNumOutputSurfaces = frames_ctx->initial_pool_size;
> +    const int kMaxSurfaces = 32;
> +    params.ulNumDecodeSurfaces = (frames_ctx->initial_pool_size <= kMaxSurfaces) ? frames_ctx->initial_pool_size : kMaxSurfaces;
> +    params.ulNumOutputSurfaces = params.ulNumDecodeSurfaces;
> +    if (frames_ctx->initial_pool_size > kMaxSurfaces) {
> +        av_log(avctx, AV_LOG_WARNING, "Requested %d decode surfaces, which is more than %d. Condifuring decoder with %d surfaces.\n",
> +               (int)frames_ctx->initial_pool_size, kMaxSurfaces, (int)params.ulNumDecodeSurfaces);
> +    }
> 
> Plus, probably rewording the original warning that handles the case where the decoder still fails.
> 
> Thoughts? If this seems like a reasonable approach I'll put together a formal patch email after going through the rest of your submission process and the stuff I have to do for my employer.

Something like that seems reasonable enough to me for sure.
Better than running into a failure right away.

Make sure to limit the number of threads in your setup.
Those are the main source for extra surfaces, specially on systems with 
high CPU/thread numbers.
If all you do is hwdecode/process/encode, you can safely just set the 
threads value to two and call it a day, and you'll then likely never run 
into the limit again.
_______________________________________________
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".