[{"id":31113,"web_url":"https://patchwork.libcamera.org/comment/31113/","msgid":"","date":"2024-09-09T07:33:07","subject":"Re: [PATCH v2] libcamera: Make FrameBuffer status default to\n\tFrameSuccess","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi\n\nOn Mon, Sep 09, 2024 at 05:34:19AM GMT, Harvey Yang wrote:\n> From: Han-Lin Chen \n>\n> To solve issues when platforms not following V4L2 spec and frame\n> buffers not going through V4L2VideoDevice, setting default values\n> to FrameMetadata's member variables.\n\nI still think this patch works around a faulty implementation, however\ninitializing class members to sane defaults certainly doesn't hurt.\n\n>\n> Signed-off-by: Han-Lin Chen \n> Co-developed-by: Harvey Yang \n> ---\n> include/libcamera/framebuffer.h | 6 +++---\n> 1 file changed, 3 insertions(+), 3 deletions(-)\n>\n> diff --git a/include/libcamera/framebuffer.h b/include/libcamera/framebuffer.h\n> index ff839243..8dae747c 100644\n> --- a/include/libcamera/framebuffer.h\n> +++ b/include/libcamera/framebuffer.h\n> @@ -32,9 +32,9 @@ struct FrameMetadata {\n> \t\tunsigned int bytesused;\n> \t};\n>\n> -\tStatus status;\n> -\tunsigned int sequence;\n> -\tuint64_t timestamp;\n> +\tStatus status = FrameSuccess;\n\nI wonder if this shouldn't be the other way around, make it Fail by\ndefault to make sure the component that handles the buffers correctly\nset their status.\n\n> +\tunsigned int sequence = 0;\n> +\tuint64_t timestamp = 0;\n\nThese ones are indeed sane\n\n>\n> \tSpan planes() { return planes_; }\n> \tSpan planes() const { return planes_; }\n> --\n> 2.46.0.469.g59c65b2a67-goog\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 2C0A0C324C\n\tfor ;\n\tMon, 9 Sep 2024 07:33:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 23BFB634EB;\n\tMon, 9 Sep 2024 09:33:15 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E76C3634E4\n\tfor ;\n\tMon, 9 Sep 2024 09:33:12 +0200 (CEST)","from ideasonboard.com (unknown [213.208.157.109])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 9B6D23EA;\n\tMon, 9 Sep 2024 09:31:56 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"OluTYlCv\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1725867116;\n\tbh=OqeFr8zmnFtF2N5qfjQTAv+cIcgFc3ZHPFV7OUYf+c0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=OluTYlCvVcl0OzsroKAmKnJXuqWhxkqEypTHImBIhmlPl/Idtq1wT0jWPrvRgSs7e\n\tlRrdVLzmLJ0ZKTX9CxFJbf8yZy++UcpR/q4roz1xN/59uSDEZe7k5rtNBPRhGNib5r\n\toilkrEUO3GIuwdr71Iw/cME43ePiTtyO8nd7Y2bA=","Date":"Mon, 9 Sep 2024 09:33:07 +0200","From":"Jacopo Mondi ","To":"Harvey Yang ","Cc":"libcamera-devel@lists.libcamera.org, \n\tHan-Lin Chen ","Subject":"Re: [PATCH v2] libcamera: Make FrameBuffer status default to\n\tFrameSuccess","Message-ID":"","References":"<20240909053425.3075699-1-chenghaoyang@google.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240909053425.3075699-1-chenghaoyang@google.com>","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":31120,"web_url":"https://patchwork.libcamera.org/comment/31120/","msgid":"","date":"2024-09-09T09:15:14","subject":"Re: [PATCH v2] libcamera: Make FrameBuffer status default to\n\tFrameSuccess","submitter":{"id":117,"url":"https://patchwork.libcamera.org/api/people/117/","name":"Cheng-Hao Yang","email":"chenghaoyang@chromium.org"},"content":"Thanks Jacopo,\n\nOn Mon, Sep 9, 2024 at 3:33 PM Jacopo Mondi \nwrote:\n\n> Hi\n>\n> On Mon, Sep 09, 2024 at 05:34:19AM GMT, Harvey Yang wrote:\n> > From: Han-Lin Chen \n> >\n> > To solve issues when platforms not following V4L2 spec and frame\n> > buffers not going through V4L2VideoDevice, setting default values\n> > to FrameMetadata's member variables.\n>\n> I still think this patch works around a faulty implementation, however\n> initializing class members to sane defaults certainly doesn't hurt.\n>\n> >\n> > Signed-off-by: Han-Lin Chen \n> > Co-developed-by: Harvey Yang \n> > ---\n> > include/libcamera/framebuffer.h | 6 +++---\n> > 1 file changed, 3 insertions(+), 3 deletions(-)\n> >\n> > diff --git a/include/libcamera/framebuffer.h\n> b/include/libcamera/framebuffer.h\n> > index ff839243..8dae747c 100644\n> > --- a/include/libcamera/framebuffer.h\n> > +++ b/include/libcamera/framebuffer.h\n> > @@ -32,9 +32,9 @@ struct FrameMetadata {\n> > unsigned int bytesused;\n> > };\n> >\n> > - Status status;\n> > - unsigned int sequence;\n> > - uint64_t timestamp;\n> > + Status status = FrameSuccess;\n>\n> I wonder if this shouldn't be the other way around, make it Fail by\n> default to make sure the component that handles the buffers correctly\n> set their status.\n>\n\nYes, that makes more sense actually. Setting it to FrameError by default\ninstead in the next version.\n\n\n>\n> > + unsigned int sequence = 0;\n> > + uint64_t timestamp = 0;\n>\n> These ones are indeed sane\n>\n> >\n> > Span planes() { return planes_; }\n> > Span planes() const { return planes_; }\n> > --\n> > 2.46.0.469.g59c65b2a67-goog\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 35865C324C\n\tfor ;\n\tMon, 9 Sep 2024 09:15:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D2FF7634F4;\n\tMon, 9 Sep 2024 11:15:27 +0200 (CEST)","from mail-lj1-x231.google.com (mail-lj1-x231.google.com\n\t[IPv6:2a00:1450:4864:20::231])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id F1204634E4\n\tfor ;\n\tMon, 9 Sep 2024 11:15:25 +0200 (CEST)","by mail-lj1-x231.google.com with SMTP id\n\t38308e7fff4ca-2f75de9a503so18890451fa.0\n\tfor ;\n\tMon, 09 Sep 2024 02:15:25 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"E5vditOV\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=chromium.org; s=google; t=1725873325; x=1726478125;\n\tdarn=lists.libcamera.org; \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=n2/u81yxM7zBVibDL4QI6FXZc2ph0rDFaGRvZbyuGIc=;\n\tb=E5vditOV3vdQ28Au3/xhYdUryM/UQdAA02K7D+oKv78MI2+3RoCGzii9ZeRqOHdUx4\n\t6pze0QnRAFtWHrnxKReE4N8a/Oc48fBCeHEEckRI7lIsUOihfy6FJC/BAv8+9HxGeIqZ\n\tFm0B/5wTfIxA81nM7fxifF5tYsZw4o5bkhpOs=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1725873325; x=1726478125;\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=n2/u81yxM7zBVibDL4QI6FXZc2ph0rDFaGRvZbyuGIc=;\n\tb=HEYdgCzhZpEcZB4PU73m33jGxw7vnv5f1HTzKPkvyJpKPW2EdxUvnJpiwzsEGHS0KM\n\tUfmltqgaox//eVIiOmdLxuhKxjE1e4mKKgtzh7MuQMPW7aod4XNyiQXUctE+pV+y2Z+i\n\tXD41HWQBzi++rJH+0LvGyJHYqQvjM505nirE4QN+o4uVO5tnWo80xvBtqJYB+VWMC85P\n\tydnDfbHF1mQ56tt7Loj1MmiSwJh2y64kA1KovHyZ0lkrpvNY6TfI3YKEL0gKx211zjJh\n\tfctoPtuKor3Ry0w1cBU6O1YOqA6c1PUqE2zc7PMpJCO5W5t8W8UvXCsPGW6dDEmhem7k\n\txRgA==","X-Gm-Message-State":"AOJu0Yx5DfNVY4WFVwTuVkmOQg7HPWDbKe4NrKWV8pJvy0FIjYqcvgwi\n\ttz51XBeaTuEV4P+R36iSE/RTULOuL+JA6AycA0ghHQj5E+HxQRU8RxIBpzXC0T4ZGAF+BDBprQI\n\tT4P91Q975UYxjAuvgtcMaMvfeI4mGebjIr1RV","X-Google-Smtp-Source":"AGHT+IEopzY/Do7t9oQo9Qa5khvx61De3W5xmX57VR7CfsEUJnHKwy4mMVe/zfmce3rNf56W563HGJhngzSEDgZ7Xu0=","X-Received":"by 2002:a2e:b8c7:0:b0:2ef:22ad:77b5 with SMTP id\n\t38308e7fff4ca-2f751f69abcmr81848051fa.29.1725873325023;\n\tMon, 09 Sep 2024 02:15:25 -0700 (PDT)","MIME-Version":"1.0","References":"<20240909053425.3075699-1-chenghaoyang@google.com>\n\t","In-Reply-To":"","From":"Cheng-Hao Yang ","Date":"Mon, 9 Sep 2024 17:15:14 +0800","Message-ID":"","Subject":"Re: [PATCH v2] libcamera: Make FrameBuffer status default to\n\tFrameSuccess","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org, \n\tHan-Lin Chen ","Content-Type":"multipart/alternative; boundary=\"0000000000007739870621ac32ae\"","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]