[{"id":25750,"web_url":"https://patchwork.libcamera.org/comment/25750/","msgid":"","date":"2022-11-09T10:28:52","subject":"Re: [libcamera-devel] [PATCH] Revert \"pipeline: raspberrypi: Do not\n\tunconditionally free buffers on close\"","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi David,\n\nThank you for the patch!\n\nOn Wed, 9 Nov 2022 at 10:16, David Plowman via libcamera-devel <\nlibcamera-devel@lists.libcamera.org> wrote:\n\n> This reverts commit 30d704732badc675f72fe73d14749669cb645c23.\n>\n> It turns out that this commit causes some regressions and is in fact\n> unnecessary because the related commit \"libcamera: v4l2_videodevice:\n> Guard against releasing unallocated buffers\"\n> (a2bdff6d0b67475492ac7cf9318866b6d89a28fd) fixes the problem\n> completely (if the buffers were never allocated, the video device\n> avoids trying to free them even if the pipeline handler asks).\n>\n> The reason for the regressions is that in this new (broken) scheme we\n> would never call clearBuffers() on all the streams if the internal\n> buffers were never allocated (i.e. buffersAllocated_ is never\n> set). This causes the stream's bufferMap_ list to get longer and\n> longer if there are multiple back-to-back calls to configure, and\n> dev_->importBuffers() will ultimately to fail.\n>\n> So either we need to think more carefully about how to stop the\n> pipeline handler from freeing buffers that it doesn't own, or we just\n> leave it as the other commit resolves the problem on its own. In the\n> interim, simply reverting this commit certainly seems like the best\n> solution.\n>\n> Signed-off-by: David Plowman \n>\n\nProbably worth adding a Fixes tag here.\n\nReviewed-by: Naushir Patuck \n\n\n> ---\n> src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 3 ---\n> 1 file changed, 3 deletions(-)\n>\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index 1b599fcc..343f8cb2 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -1506,9 +1506,6 @@ void PipelineHandlerRPi::mapBuffers(Camera *camera,\n> const RPi::BufferMap &buffer\n>\n> void RPiCameraData::freeBuffers()\n> {\n> - if (!buffersAllocated_)\n> - return;\n> -\n> if (ipa_) {\n> /*\n> * Copy the buffer ids from the unordered_set to a vector\n> to\n> --\n> 2.30.2\n>\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id B25C5BD16B\n\tfor ;\n\tWed, 9 Nov 2022 10:29:11 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 698B463084;\n\tWed, 9 Nov 2022 11:29:11 +0100 (CET)","from mail-qv1-xf32.google.com (mail-qv1-xf32.google.com\n\t[IPv6:2607:f8b0:4864:20::f32])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3B70161F3F\n\tfor ;\n\tWed, 9 Nov 2022 11:29:09 +0100 (CET)","by mail-qv1-xf32.google.com with SMTP id c8so11976394qvn.10\n\tfor ;\n\tWed, 09 Nov 2022 02:29:09 -0800 (PST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1667989751;\n\tbh=6hgp9Ks8qymKzP0w0QuTpT/Zj/m5dAHUMGACHQLfJYs=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=EJ7ZWt3a+DWZpMnv6shxrYvbMBp7o39IxfMZobVjRMx52oBjhEock4plca3OXUFQv\n\tugUPoAczaJaFrgEFgV6WnZ4CyN/BNH+24ZDyXqy/Q4ufof1cwT3Ky5WUNZr+4RcT0c\n\t0MwS8XdfI2/CXIYDouV3QvcZSnJ89Fjzlo8sWbNHCMSe4GAdaG1qYRz2/rTDwV2pXy\n\tW8m7/2w7CY0bmcvHoPhkf0yNJHehgNvyQmvxdcah5dksVHZx/rlibDTkOrqLiZqbD9\n\t0Nen26N0DKsvQHWwsKPkZ1YYPwZs/1TOtltZQem/QrX137v+kPR8tXuWJRIsr8GZRb\n\t7qUlfHDSDyoZw==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=BXYExfVmya+dHzzxEPolQ6SyeLWIZd6Qmf3hBVyshEA=;\n\tb=QZMpT8SAH3Q0BylaEreWLwdIEyL2hmb7uK5IqJMu3krcJIb6QXwKp1eAth4uMVujID\n\t8Rx0aHkBUgmmmQyCUad7Bc6EBKubnEkSjSqKkt8V7/S8z9Ryiv6rg1NboJu+nCqu+Qei\n\tUHQK2d6RdNz14ojG+tNMVn0B+Y68+kZPlsvNrejXLy6U+gBTLN3h6KpXtPVtxyOl0grB\n\tD+0+KLolsmSn/KfxU8TX/rtD1Vn7vqC4TYw7/jhDatFGObJuePCN0LCTIsCuju0mlUjZ\n\tfTarMYmT79vdQsSauhWFhc9ekzqbMtlW+0UhDFQrzcblSVV9vTjMp/CyPH9Q6X1e4HeU\n\t9T+Q=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"QZMpT8SA\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=BXYExfVmya+dHzzxEPolQ6SyeLWIZd6Qmf3hBVyshEA=;\n\tb=vWx6RjDEjx+3pIWeBbRBPOsGZe4bxKXlQpVmgm+jkTdvREahEORKNQqU/dR5lPW+jg\n\tN0o3uGSLTnCKrdsWXl91m3iasGRb4dJgZ7/j+u/lncjgN6rsqp3L1k2rIsgu3ZG+zqYL\n\tPfqUcf7v0vxM//Z2nUHDE0MaC9XJUQqPXEllYj5J6n1FlNz4+93k9CZYeGY20ngVS5Qw\n\tTP6tcgWEQt66o1WcHZdLpZ2oJIq8eF9tez+WxFMdqlNCCLVz/6xbsXQ+wBBXZnbTVU6W\n\tTJ4kYhz9UBvvL7fNNcmOeV3Y7hzVODgFqrFmcIofVyOEcB8ebOj0giWSHiuKVrINL2L3\n\tK5Tw==","X-Gm-Message-State":"ACrzQf3ypYEDdzwf3KCSJnP2Y8gJcq+vUwkvuROcyJ20/mJ5MtWnfXgx\n\tGBq0C+Iiw+YEdOG6UaKYRzg6+QyS3gOULo1/68o8ZDiThks=","X-Google-Smtp-Source":"AMsMyM5dGKOYZm3ayhffqGPMiEfXv5MYaPs0yFjCOTZFzA1nQwooaBlxlZPJPLynR/mM3cZfm7dLiqznMhItGWIWN0A=","X-Received":"by 2002:a05:6214:2264:b0:4bb:792d:e391 with SMTP id\n\tgs4-20020a056214226400b004bb792de391mr54693624qvb.100.1667989748288;\n\tWed, 09 Nov 2022 02:29:08 -0800 (PST)","MIME-Version":"1.0","References":"<20221109101642.4957-1-david.plowman@raspberrypi.com>","In-Reply-To":"<20221109101642.4957-1-david.plowman@raspberrypi.com>","Date":"Wed, 9 Nov 2022 10:28:52 +0000","Message-ID":"","To":"David Plowman ","Content-Type":"multipart/alternative; boundary=\"0000000000006fa43b05ed07204e\"","Subject":"Re: [libcamera-devel] [PATCH] Revert \"pipeline: raspberrypi: Do not\n\tunconditionally free buffers on close\"","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Naushir Patuck via libcamera-devel\n\t","Reply-To":"Naushir Patuck ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]