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 ESMTP id 3FF9542934 for ; Mon, 10 Jan 2022 08:17:49 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 1335568AE7A; Mon, 10 Jan 2022 10:17:47 +0200 (EET) Received: from mail-ot1-f46.google.com (mail-ot1-f46.google.com [209.85.210.46]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 6DF6368A744 for ; Mon, 10 Jan 2022 10:17:41 +0200 (EET) Received: by mail-ot1-f46.google.com with SMTP id s8-20020a0568301e0800b00590a1c8cc08so8373522otr.9 for ; Mon, 10 Jan 2022 00:17:41 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=from:content-transfer-encoding:mime-version:subject:date:references :to:in-reply-to:message-id; bh=dO9EvyafWqhrG96csTsjsgqJhbH6JxK4IlS7FkVJ4YY=; b=CYfROVLzvpCgws3+Xg6Xbd31aXSlzQxzvClu1uwN6SCna7Nri0spYp8kwKRC3Y7+Y9 lGoVa3iyT8l1CWqvvOwnjXcVa5bC1IC2NmXVZiAYtrVgSR303C9SSYfyVlmUxOgoqMgl j8jtCruPviS6AnsQo3DYLJX+XHkYQyo787Usd9tdNa7zR/+x5kb45dAnBpdf4RzIerJf Xtbg2j48JsDFJ70eBuVZ/2BJjJTDkjYAyuiJIm7NOqMVvkgJkYDTw11WgXVlr0NG6REN tOgLtX2Kg3gKAMCkmCd4PHqabqU8t6bIXygI3Fa0kKKzdQZI8lGz39T6qfUH4ZWH05UC QhDw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:content-transfer-encoding:mime-version :subject:date:references:to:in-reply-to:message-id; bh=dO9EvyafWqhrG96csTsjsgqJhbH6JxK4IlS7FkVJ4YY=; b=pEbIjJbrFUWKa4Bt+jhQH3zZNrDW9/C8hL5r8UiagiZ7dEcqY/IEBpbQgY51nUUuV+ jLnk4MaI3aFyW+Cs9allQHdqslFJQ60SEo2WzSI0IOvblKgyn8JaUccijX9v/+va7Ksz +bc8/l12cW47dewnz1M7cgz7ht0xp0RZ9JdWw7jg33i6RlyBQC00GT9Rlkjn+NzaCOKl zb6BKCI6PzVM3CGdYEDwhgfd2MN/kYqEH+rcuG+K5mF07yMAn4V61ywfRjC9nDydUtc+ EB50IIZvwhAShSnVwxhdUJxrdJ/EnCoSJfKnVWarDZD1aLTugsPE7874vOwnSzwHQvDd tO2g== X-Gm-Message-State: AOAM533S4fdftOglqY6pX7Cd/CtWoNLIjk8/pDkjGwr3VvAbc4hYbscl C6LvGtswil1HTHNO54oUoiD7a/Hw0jRCww== X-Google-Smtp-Source: ABdhPJy+EEOp2yARdvw2yrFGvCwNbr4rmHrjZ3WO3oo103fHjjPJJmDh+avNgRmQA31qT3Qd8IZ0Qw== X-Received: by 2002:a9d:2d81:: with SMTP id g1mr53667782otb.25.1641802659214; Mon, 10 Jan 2022 00:17:39 -0800 (PST) Received: from smtpclient.apple ([2600:1700:4830:3f7f:7d7a:d4ca:2b00:f712]) by smtp.gmail.com with ESMTPSA id ay40sm1086971oib.1.2022.01.10.00.17.38 for (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Mon, 10 Jan 2022 00:17:38 -0800 (PST) From: Cameron Gutman Mime-Version: 1.0 (Mac OS X Mail 15.0 \(3693.40.0.1.81\)) Date: Mon, 10 Jan 2022 02:17:37 -0600 References: <20220103003318.442892-1-aicommander@gmail.com> <164172029148.2375.17275383230465998544@lain.red.khirnov.net> To: FFmpeg development discussions and patches In-Reply-To: <164172029148.2375.17275383230465998544@lain.red.khirnov.net> Message-Id: X-Mailer: Apple Mail (2.3693.40.0.1.81) Subject: Re: [FFmpeg-devel] [PATCH] lavu/videotoolbox: add support for memory mapping frames 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-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: > On Jan 9, 2022, at 3:24 AM, Anton Khirnov wrote: > > Quoting Cameron Gutman (2022-01-03 01:33:19) >> Signed-off-by: Cameron Gutman >> --- >> libavutil/hwcontext_videotoolbox.c | 25 +++++++++++++++++++++++++ >> 1 file changed, 25 insertions(+) >> >> diff --git a/libavutil/hwcontext_videotoolbox.c b/libavutil/hwcontext_videotoolbox.c >> index 0a8dbe9f33..026127d412 100644 >> --- a/libavutil/hwcontext_videotoolbox.c >> +++ b/libavutil/hwcontext_videotoolbox.c >> @@ -711,6 +711,30 @@ fail: >> return err; >> } >> >> +static int vt_map_from(AVHWFramesContext *hwfc, AVFrame *dst, >> + const AVFrame *src, int flags) >> +{ >> + int err; >> + >> + if (dst->format == AV_PIX_FMT_NONE) >> + dst->format = hwfc->sw_format; >> + else if (dst->format != hwfc->sw_format) >> + return AVERROR(ENOSYS); >> + >> + err = vt_map_frame(hwfc, dst, src, flags); >> + if (err) >> + return err; >> + >> + dst->width = src->width; >> + dst->height = src->height; >> + >> + err = av_frame_copy_props(dst, src); >> + if (err) >> + return err; > > Don't you need to unmap the frame in this error path? > Maybe? It's complicated... The mapping is still written to dst and it will be unmapped when av_frame_unref() is called on dst. However, if the caller wants to try again with same dst frame for some reason, then it looks like it will leak the first "failed" mapping. AFAICT, another call to ff_hwframe_map_create() will overwrite dst->buf[0] without unrefing it first, but that makes sense given that the docs say "dst should be blank" (arguably that "should" ought to be a "must" in this case). However, this isn't the full story. None of the existing map_from() implementations (VAAPI, DRM, DXVA2) actually unmap when av_frame_copy_props() fails. The only ones that don't have this bug are OpenCL and Vulkan, and that's only because they forgot to call av_frame_copy_props() entirely! Ideally, you'd have nothing modified in dst when av_hwframe_map() fails, but that isn't the case in practice. Much of the mapping process involves irreversible changes to the dst frame, including overwriting dst->format, dst->width, dst->height, dst->data, dst->linesize, even partially copied frame properties prior to av_frame_copy_props() failing. Given these limitations, it seems like the only sane thing to do with a dst frame after failure of av_hwframe_map() (other than ENOSYS) is to unref/free. The frame is basically in an undefined state after such a failure. If that's the case, then we don't actually have a leak here since av_frame_unref() will do the right thing and free the old mapping. Cam > -- > Anton Khirnov _______________________________________________ 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".