From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTPS id DFAAC4D42E for ; Fri, 21 Feb 2025 13:26:59 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id CFB3868C7B8; Fri, 21 Feb 2025 15:26:55 +0200 (EET) Received: from btbn.de (btbn.de [144.76.60.213]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id C81F668C360 for ; Fri, 21 Feb 2025 15:26:49 +0200 (EET) Received: from [authenticated] by btbn.de (Postfix) with ESMTPSA id 4BDE42819FDD0 for ; Fri, 21 Feb 2025 14:26:49 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=rothenpieler.org; s=mail; t=1740144409; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=ljZ/Y81YoLkzP20zoF7FJCxz31ebgN24D3yX4u0oNSU=; b=DM8irLclrZKsSuy7MuyRy5TuMmf8WqRtsNmzZk3+aWekxafSsnWBgoCA4h+HCv5BEbfrqx EJJ/EQUrJCRgTaFD+XdeK9oge+olWy5A6Ts0obImCaR3vZxTEMJey/Y71xcUDaGFfbZgij IMhRNVBaB9OwOThs9yQBhacNbt657X4W01b1ASXd8dWQ86JFp7ETg15Sm7hsuFr0isVlw/ qBw8pbRRTi0mzQH7Expf0pQp4DGWtfKcFXG2s4xHAMd1CRHwaqn9FR7/smA98rJdNkVVdd XszdBak147DyABfN2STA+uJIYPTLRvw98zu94k1tf9VUnRoJur57QHWRhJMnoA== Message-ID: <79d22978-6473-42d1-b004-31a8b0236f7e@rothenpieler.org> Date: Fri, 21 Feb 2025 14:26:49 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: ffmpeg-devel@ffmpeg.org References: <20250220203733.165702-1-scott.the.elm@gmail.com> Content-Language: en-US From: Timo Rothenpieler In-Reply-To: <20250220203733.165702-1-scott.the.elm@gmail.com> Subject: Re: [FFmpeg-devel] [PATCH] libavcodec/cuviddec.c: increase CUVID_DEFAULT_NUM_SURFACES X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: On 20.02.2025 21:37, Scott Theisen wrote: > The default value of CuvidContext::nb_surfaces was reduced from 25 to 5 (as > (CUVID_MAX_DISPLAY_DELAY + 1)) in 402d98c9d467dff6931d906ebb732b9a00334e0b. > > In cuvid_is_buffer_full() delay can be 2 * CUVID_MAX_DISPLAY_DELAY with double > rate deinterlacing. ctx->nb_surfaces is CUVID_DEFAULT_NUM_SURFACES = > (CUVID_MAX_DISPLAY_DELAY + 1) by default, in which case cuvid_is_buffer_full() > will always return true and cuvid_output_frame() will never read any data since > it will not call ff_decode_get_packet(). It's been way too long since I looked at all that code, and I didn't even write most of the code involved: > https://github.com/FFmpeg/FFmpeg/commit/bddb2343b6e594e312dadb5d21b408702929ae04 > https://github.com/FFmpeg/FFmpeg/commit/402d98c9d467dff6931d906ebb732b9a00334e0b But doesn't this instead mean that the logic in cuvid_is_buffer_full is flawed somehow? Just increasing the default number of surfaces does not seem like the correct fix or sensible, since it will increase VRAM usage by potentially quite a bit for all users. From looking at this a bit, the issue will only happen when deinterlacing, the logic in cuvid_is_buffer_full becomes stuck then, and will always claim the buffer is full. And from my understanding, it's correct in making that claim. Due to the display delay, it could in theory happen that the moment cuvid starts outputting frames, there will be more output available than what fits into ctx->frame_queue, since it delayed by 4 frames, which results in 8 surfaces, but the queue only fits 5. So to me it looks like that the correct fix would be to double the size of the frame_queue when deinterlacing, not unconditionally. nb_surfaces is also used to determine the size of the key_frame array, which would then also be pointlessly doubled. But not like a handful of extra ints would hurt that much though. Alternatively the size-doubling could not be reflected in nb_surfaces, but that would make the logic in various other places be more complicated. > --- > > I think part of the problem might be that cuvid_is_buffer_full() does not know > how many frames are actually in the driver's queue and assumes it is the > maximum, even if none have yet been added. > > This was preventing any frames from being decoded using NVDEC with MythTV for > some streams. See https://github.com/MythTV/mythtv/issues/1039 I'd highly recommend to not use cuviddec anymore, but instead use nvdec. cuviddec only still exists to sanity-check nvdec against it at times. > --- > libavcodec/cuviddec.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavcodec/cuviddec.c b/libavcodec/cuviddec.c > index 67076a1752..05dcafab6e 100644 > --- a/libavcodec/cuviddec.c > +++ b/libavcodec/cuviddec.c > @@ -120,7 +120,7 @@ typedef struct CuvidParsedFrame > #define CUVID_MAX_DISPLAY_DELAY (4) > > // Actual pool size will be determined by parser. > -#define CUVID_DEFAULT_NUM_SURFACES (CUVID_MAX_DISPLAY_DELAY + 1) > +#define CUVID_DEFAULT_NUM_SURFACES ((2 * CUVID_MAX_DISPLAY_DELAY) + 1) > > static int CUDAAPI cuvid_handle_video_sequence(void *opaque, CUVIDEOFORMAT* format) > { _______________________________________________ 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".