[{"id":34272,"web_url":"https://patchwork.libcamera.org/comment/34272/","msgid":"","date":"2025-05-19T09:54:19","subject":"Re: [PATCH v1 1/6] libcamera: framebuffer: Add\n\tFrameMetadata::Status::FrameStartup","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Naush\n\nThanks for the patch.\n\nOn Mon, 19 May 2025 at 10:22, Naushir Patuck wrote:\n>\n> Add a new status enum, FrameStartup, used to denote that even though\n> the frame has been successfully captured, the IQ parameters set by the\n> IPA will cause the frame to be unusable and applications are advised to\n> not consume this frame. An example of this would be on a cold-start of\n> the 3A algorithms, and there will be large oscillations to converge to\n> a stable state quickly.\n>\n> Additional, update the definition of the FrameError state to include its\n> usage when the sensor is known to produce a number of invalid/error\n> frames after stream-on.\n>\n> Signed-off-by: Naushir Patuck \n\nThis looks fine to me!\n\nReviewed-by: David Plowman \n\nThanks\nDavid\n\n> ---\n> include/libcamera/framebuffer.h | 1 +\n> src/libcamera/framebuffer.cpp | 11 +++++++++--\n> 2 files changed, 10 insertions(+), 2 deletions(-)\n>\n> diff --git a/include/libcamera/framebuffer.h b/include/libcamera/framebuffer.h\n> index ff83924300ac..e83825b466aa 100644\n> --- a/include/libcamera/framebuffer.h\n> +++ b/include/libcamera/framebuffer.h\n> @@ -26,6 +26,7 @@ struct FrameMetadata {\n> FrameSuccess,\n> FrameError,\n> FrameCancelled,\n> + FrameStartup,\n> };\n>\n> struct Plane {\n> diff --git a/src/libcamera/framebuffer.cpp b/src/libcamera/framebuffer.cpp\n> index 826848f75a56..36e56d593bc3 100644\n> --- a/src/libcamera/framebuffer.cpp\n> +++ b/src/libcamera/framebuffer.cpp\n> @@ -44,11 +44,18 @@ LOG_DEFINE_CATEGORY(Buffer)\n> * of the FrameMetadata structure are valid.\n> * \\var FrameMetadata::FrameError\n> * An error occurred during capture of the frame. The frame data may be partly\n> - * or fully invalid. The sequence and timestamp fields of the FrameMetadata\n> - * structure is valid, the other fields may be invalid.\n> + * or fully invalid. This status may also indicate an invalid frame produced by\n> + * the sensor during its startup or restart phase. The sequence and timestamp\n> + * fields of the FrameMetadata structure is valid, the other fields may be\n> + * invalid.\n> * \\var FrameMetadata::FrameCancelled\n> * Capture stopped before the frame completed. The frame data is not valid. All\n> * fields of the FrameMetadata structure but the status field are invalid.\n> + * \\var FrameMetadata::FrameStartup The frame has been successfully captured.\n> + * However, the IPA is in a cold-start or reset phase and will result in image\n> + * quality parameters producing unusable images. Applications are recommended to\n> + * not consume these frames. All fields of the FrameMetadata structure are\n> + * valid.\n> */\n>\n> /**\n> --\n> 2.43.0\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 9DECCBD78E\n\tfor ;\n\tMon, 19 May 2025 09:54:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DA3C568D7A;\n\tMon, 19 May 2025 11:54:33 +0200 (CEST)","from mail-qt1-x833.google.com (mail-qt1-x833.google.com\n\t[IPv6:2607:f8b0:4864:20::833])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 700A3616A3\n\tfor ;\n\tMon, 19 May 2025 11:54:31 +0200 (CEST)","by mail-qt1-x833.google.com with SMTP id\n\td75a77b69052e-476ab588f32so64403481cf.2\n\tfor ;\n\tMon, 19 May 2025 02:54:31 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"ODoKn3Bh\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1747648470; x=1748253270;\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=GGc8vo8RMszhWO7r00oLeJtVEYtN3l4PaVYnAdG62hI=;\n\tb=ODoKn3BhcqzzdlOmEPcjFg+JQPCl3wHUmj7LC4wwBZFJ+FY23Uh88Qq/eMJ0kdei+Z\n\tltPbA1DauOC6OEHG5i/97uBDzqCkR7nw5DqHNlVmuSoZHCANvBiBiCvQolKAoignovOW\n\tF0Tjk86JtLRJvbEPbaXPz8wk0+un+SEv2K58OzExMgTUZkvpBRfMQKQL9rv6oZpKl4nJ\n\tu32MZjtUB5pejoHykYuY6ZZIpo/FBlEq4I16AXD2LSpVOKunTXwiLmUimWxhf/3hOYlW\n\tNAQoSOmirWAEtQnce7F4SKXrIaqFNwnwvb6UYwykKjv3QIPrsiprHIR/xSfNKu3HHoD2\n\tJN0w==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1747648470; x=1748253270;\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=GGc8vo8RMszhWO7r00oLeJtVEYtN3l4PaVYnAdG62hI=;\n\tb=s6S/TfG4Losc92gAuLe0DTjbQwKtzCJZidBJHoVqiPtiTd07EmY2n+/JuTW6TnHYM4\n\tc34RVJt7Rca34d01pDeeghPQvOLh83OSJHFDmH1CSmFHuu9YolnPMCu8vgU463IALvzs\n\tYY1WczFIdoptSu5qOGc+IxXVcdwPzIEwNG171nmcVjFJpaXyKEm/8dtIRBu5zaDfzR/0\n\t8AlxmkweNcwXddjmY4UIU2MNdf7H4I79LBEP6P5IJMPGeWrPZLboMyci2hcfopcQDS9x\n\ttT1tYMHDXJJObcfQx9ie687qUGlWTziq2pV9M5n4w9pzBrtGe1tLpJWTNqIzjDqWwUpu\n\t8xoA==","X-Gm-Message-State":"AOJu0YyzX8Psecfs+TcavDrrYuG3o2IzgLcpvnmH6cgm+XKqRPY+b8fd\n\tu8WEn2Kej7xwQ0sRgpa4XQo/p7twkrRA2Gznj1YN5jBsdJRtBcp7cmzUnfSU5CJn8bT0ciKCMmg\n\tBr0a1h4+gwbAqUseO+jn1pH3P/uhk2FUfGso3+Iq6sQ==","X-Gm-Gg":"ASbGncuhcMpVbuWgLfYKcZSduDgMqd8cl1BXzyN15WSaIgIEWaeXKirqkCl3N68vqPc\n\tNgkNewQhc4ju9lH0Ba3yQfe5VnY+nrvnFhrUPyAJYmXQVL9hUusaV1dhR/COkg7hNfmQSkPjXuJ\n\tD/FSSRhN8iM1rPnY+oxR5DnMjQBniCAxr/vQ6lzDoBt+1/L9SNjejgY7NYHFu3QKk=","X-Google-Smtp-Source":"AGHT+IGyFoQgf2Gi/Iz06JYNj8DLB9c/ddzBe6zWGZx2KRXppJoTrQ+vKuHIQeTBHXPXXwMHSXRjSGV5AphY16koEJY=","X-Received":"by 2002:a05:622a:5812:b0:477:6f6d:607a with SMTP id\n\td75a77b69052e-494b0745a86mr174555411cf.7.1747648470101;\n\tMon, 19 May 2025 02:54:30 -0700 (PDT)","MIME-Version":"1.0","References":"<20250519092245.269048-1-naush@raspberrypi.com>\n\t<20250519092245.269048-2-naush@raspberrypi.com>","In-Reply-To":"<20250519092245.269048-2-naush@raspberrypi.com>","From":"David Plowman ","Date":"Mon, 19 May 2025 10:54:19 +0100","X-Gm-Features":"AX0GCFvAWAeN27jXxw6qYwMWN0O5lhVEE40tlGCd0DqFcUJeJbwpp90hAyuTSac","Message-ID":"","Subject":"Re: [PATCH v1 1/6] libcamera: framebuffer: Add\n\tFrameMetadata::Status::FrameStartup","To":"Naushir Patuck ","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"UTF-8\"","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":34280,"web_url":"https://patchwork.libcamera.org/comment/34280/","msgid":"","date":"2025-05-20T06:52:35","subject":"Re: [PATCH v1 1/6] libcamera: framebuffer: Add\n\tFrameMetadata::Status::FrameStartup","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Naush\n\nOn Mon, May 19, 2025 at 10:20:49AM +0100, Naushir Patuck wrote:\n> Add a new status enum, FrameStartup, used to denote that even though\n> the frame has been successfully captured, the IQ parameters set by the\n> IPA will cause the frame to be unusable and applications are advised to\n> not consume this frame. An example of this would be on a cold-start of\n> the 3A algorithms, and there will be large oscillations to converge to\n> a stable state quickly.\n>\n> Additional, update the definition of the FrameError state to include its\n> usage when the sensor is known to produce a number of invalid/error\n> frames after stream-on.\n>\n> Signed-off-by: Naushir Patuck \n> ---\n> include/libcamera/framebuffer.h | 1 +\n> src/libcamera/framebuffer.cpp | 11 +++++++++--\n> 2 files changed, 10 insertions(+), 2 deletions(-)\n>\n> diff --git a/include/libcamera/framebuffer.h b/include/libcamera/framebuffer.h\n> index ff83924300ac..e83825b466aa 100644\n> --- a/include/libcamera/framebuffer.h\n> +++ b/include/libcamera/framebuffer.h\n> @@ -26,6 +26,7 @@ struct FrameMetadata {\n> \t\tFrameSuccess,\n> \t\tFrameError,\n> \t\tFrameCancelled,\n> +\t\tFrameStartup,\n> \t};\n>\n> \tstruct Plane {\n> diff --git a/src/libcamera/framebuffer.cpp b/src/libcamera/framebuffer.cpp\n> index 826848f75a56..36e56d593bc3 100644\n> --- a/src/libcamera/framebuffer.cpp\n> +++ b/src/libcamera/framebuffer.cpp\n> @@ -44,11 +44,18 @@ LOG_DEFINE_CATEGORY(Buffer)\n> * of the FrameMetadata structure are valid.\n> * \\var FrameMetadata::FrameError\n> * An error occurred during capture of the frame. The frame data may be partly\n> - * or fully invalid. The sequence and timestamp fields of the FrameMetadata\n> - * structure is valid, the other fields may be invalid.\n> + * or fully invalid. This status may also indicate an invalid frame produced by\n> + * the sensor during its startup or restart phase. The sequence and timestamp\n> + * fields of the FrameMetadata structure is valid, the other fields may be\n> + * invalid.\n\nThis suggests to me that either you expect pipelines to set\n\n FrameError | FrameStartup\n\nto indicate a 'sensor startup' frame, which I don't think it's the\nintention here.\n\nI wonder if we actually add anything here.\n\nOr maybe just change the definition to:\n\n * The frame data may be partly or fully invalid. The sequence and\n * timestamp fields of the FrameMetadata structure is valid, the other\n * fields may be invalid.\n\nto remove the \"An error occurred\" part, as \"sensor startup\" frames are\nnot errors, but contain known-to-be-invalid data.\n\nWhat matters for applications, is, after all, knowing if frames can be\nconsumed or not, so I think we can drop mentioning \"errors\" or more\nprecise failure conditions.\n\n\n\n> * \\var FrameMetadata::FrameCancelled\n> * Capture stopped before the frame completed. The frame data is not valid. All\n> * fields of the FrameMetadata structure but the status field are invalid.\n> + * \\var FrameMetadata::FrameStartup The frame has been successfully captured.\n\nFollowing the above I would say that\n\n \"The frame data is valid, however the...\n\n> + * However, the IPA is in a cold-start or reset phase and will result in image\n\nDo we need to mention IPA ?\n\n \".. however the capture pipeline is in a cold-start or reset\n phase\"\n\n\n> + * quality parameters producing unusable images. Applications are recommended to\n> + * not consume these frames. All fields of the FrameMetadata structure are\n\nAll other fields ? Or are you ok with how it is ?\n\nThanks\n j\n\n> + * valid.\n> */\n>\n> /**\n> --\n> 2.43.0\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 87EDBC31E9\n\tfor ;\n\tTue, 20 May 2025 06:52:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1D8EC68D7D;\n\tTue, 20 May 2025 08:52:41 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 29FDC614DD\n\tfor ;\n\tTue, 20 May 2025 08:52:39 +0200 (CEST)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 5C899446;\n\tTue, 20 May 2025 08:52:18 +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=\"bJrQG2B9\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1747723938;\n\tbh=z/ze43UIKweGwKDm9Aj4dXX67shY7CiX0ORV8F4GAcI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=bJrQG2B9lBmUryf5iuAyRUSP0MpqXUnhp1yeGueamWSJRMDIvRdNdppjlu7+fDI0U\n\tRNZ19Fqycwp7L1GQtPZ1OLln3kmUUDVUq1gDtjRczLEpyypG3DD+YGtv+fqeE/xMPb\n\ti8NbLluK+juxwW4Q2a/G5DePPKV/YoVGw9YIQoHU=","Date":"Tue, 20 May 2025 08:52:35 +0200","From":"Jacopo Mondi ","To":"Naushir Patuck ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v1 1/6] libcamera: framebuffer: Add\n\tFrameMetadata::Status::FrameStartup","Message-ID":"","References":"<20250519092245.269048-1-naush@raspberrypi.com>\n\t<20250519092245.269048-2-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20250519092245.269048-2-naush@raspberrypi.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":34283,"web_url":"https://patchwork.libcamera.org/comment/34283/","msgid":"","date":"2025-05-20T08:05:33","subject":"Re: [PATCH v1 1/6] libcamera: framebuffer: Add\n\tFrameMetadata::Status::FrameStartup","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Jacopo,\n\nOn Tue, 20 May 2025 at 07:52, Jacopo Mondi \nwrote:\n\n> Hi Naush\n>\n> On Mon, May 19, 2025 at 10:20:49AM +0100, Naushir Patuck wrote:\n> > Add a new status enum, FrameStartup, used to denote that even though\n> > the frame has been successfully captured, the IQ parameters set by the\n> > IPA will cause the frame to be unusable and applications are advised to\n> > not consume this frame. An example of this would be on a cold-start of\n> > the 3A algorithms, and there will be large oscillations to converge to\n> > a stable state quickly.\n> >\n> > Additional, update the definition of the FrameError state to include its\n> > usage when the sensor is known to produce a number of invalid/error\n> > frames after stream-on.\n> >\n> > Signed-off-by: Naushir Patuck \n> > ---\n> > include/libcamera/framebuffer.h | 1 +\n> > src/libcamera/framebuffer.cpp | 11 +++++++++--\n> > 2 files changed, 10 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/include/libcamera/framebuffer.h\n> b/include/libcamera/framebuffer.h\n> > index ff83924300ac..e83825b466aa 100644\n> > --- a/include/libcamera/framebuffer.h\n> > +++ b/include/libcamera/framebuffer.h\n> > @@ -26,6 +26,7 @@ struct FrameMetadata {\n> > FrameSuccess,\n> > FrameError,\n> > FrameCancelled,\n> > + FrameStartup,\n> > };\n> >\n> > struct Plane {\n> > diff --git a/src/libcamera/framebuffer.cpp\n> b/src/libcamera/framebuffer.cpp\n> > index 826848f75a56..36e56d593bc3 100644\n> > --- a/src/libcamera/framebuffer.cpp\n> > +++ b/src/libcamera/framebuffer.cpp\n> > @@ -44,11 +44,18 @@ LOG_DEFINE_CATEGORY(Buffer)\n> > * of the FrameMetadata structure are valid.\n> > * \\var FrameMetadata::FrameError\n> > * An error occurred during capture of the frame. The frame data may be\n> partly\n> > - * or fully invalid. The sequence and timestamp fields of the\n> FrameMetadata\n> > - * structure is valid, the other fields may be invalid.\n> > + * or fully invalid. This status may also indicate an invalid frame\n> produced by\n> > + * the sensor during its startup or restart phase. The sequence and\n> timestamp\n> > + * fields of the FrameMetadata structure is valid, the other fields may\n> be\n> > + * invalid.\n>\n> This suggests to me that either you expect pipelines to set\n>\n> FrameError | FrameStartup\n>\n> to indicate a 'sensor startup' frame, which I don't think it's the\n> intention here.\n>\n> I wonder if we actually add anything here.\n>\n\nThe intention is for the enum flags to be exclusive, i.e. either FrameError\nor FrameStartup.\n\nHere, I've overloaded the FrameError flag to capture the existing usage\nlike random transmission errors plus the bad frames produced by the sensor\ndevice on startup.\n\n\n> Or maybe just change the definition to:\n>\n> * The frame data may be partly or fully invalid. The sequence and\n> * timestamp fields of the FrameMetadata structure is valid, the other\n> * fields may be invalid.\n>\n> to remove the \"An error occurred\" part, as \"sensor startup\" frames are\n> not errors, but contain known-to-be-invalid data.\n>\n\nMaybe I should replace the word \"invalid\" with \"corrupt\" in the new\ndefinition?\n\n\"\nThis status may also indicate corrupt frames produced by\n the sensor during its startup or restart phase.\n\"\n\n\n> What matters for applications, is, after all, knowing if frames can be\n> consumed or not, so I think we can drop mentioning \"errors\" or more\n> precise failure conditions.\n>\n\nHaving 2 flags does allow the application to know what's going on under the\nhood, i.e. bad frames from the device, or bad IQ due to software. But I\ncould be convinced that these could be folded into one error flag if that's\na better way.\n\n\n>\n>\n> > * \\var FrameMetadata::FrameCancelled\n> > * Capture stopped before the frame completed. The frame data is not\n> valid. All\n> > * fields of the FrameMetadata structure but the status field are\n> invalid.\n> > + * \\var FrameMetadata::FrameStartup The frame has been successfully\n> captured.\n>\n> Following the above I would say that\n>\n> \"The frame data is valid, however the...\n>\n> > + * However, the IPA is in a cold-start or reset phase and will result\n> in image\n>\n> Do we need to mention IPA ?\n>\n\nMaybe s/IPA/imaging algorithms (3A)/ ?\n\n\n>\n> \".. however the capture pipeline is in a cold-start or reset\n> phase\"\n>\n>\n> > + * quality parameters producing unusable images. Applications are\n> recommended to\n> > + * not consume these frames. All fields of the FrameMetadata structure\n> are\n>\n> All other fields ? Or are you ok with how it is ?\n>\n\nYes, I think all other metadata fields ought to report valid data in these\ncases.\n\nRegards,\nNaush\n\n\n>\n> Thanks\n> j\n>\n> > + * valid.\n> > */\n> >\n> > /**\n> > --\n> > 2.43.0\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 12972C31E9\n\tfor ;\n\tTue, 20 May 2025 08:06:14 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2998868D7D;\n\tTue, 20 May 2025 10:06:12 +0200 (CEST)","from mail-ua1-x930.google.com (mail-ua1-x930.google.com\n\t[IPv6:2607:f8b0:4864:20::930])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8BC7E614DD\n\tfor ;\n\tTue, 20 May 2025 10:06:09 +0200 (CEST)","by mail-ua1-x930.google.com with SMTP id\n\ta1e0cc1a2514c-87bfddd879bso107039241.3\n\tfor ;\n\tTue, 20 May 2025 01:06:09 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"S1DjOn0r\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1747728368; x=1748333168;\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=96+GA3BK2B8HvbeBLlcv2y6+3agVvnMU5X8NShZH28Q=;\n\tb=S1DjOn0r0yzko0zG4IO69LrqF/u6VWVW4V/4GBan6ojk96nRzAQlXHghj1PGMC5tli\n\tsw2M6vlbXIowyPUkZd8+nf+Ckkv2stXgMIz28fv+jW4tRp5lVtbhfWxacDw2RuD2P7W6\n\tavkJkY/K9MujBuIIgQKfMcK//uZTjH6+M4SXOeQmykSOjP5+gE5QdZNzbQ38mMEJbaZa\n\ttjhysxVnfG+MTPY8ZcVRMC+R6cvHYjFCWN95tBAc0124RG4uDpSmc/wL0q4SZUuJGfgZ\n\tDLe3YMaWSJcIdikH42NUrpurzpJxh7ia0/5PBGf8evXlVchuNBQEuDWViavNo1XP0oS9\n\twMcA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1747728368; x=1748333168;\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=96+GA3BK2B8HvbeBLlcv2y6+3agVvnMU5X8NShZH28Q=;\n\tb=fOdVxAfznNsL0lF9ZRpi1gme9Bn5N8tmlajoQ+CDMXufAt7qwb7cmHA+rmdF5FQyR4\n\tVSPiQyQYME1GRQoUEBZEIsUDvoeSd01kmfKdE9Fdo8cORef7YXEqdfTGYK54Affq0res\n\t0VjmFsDe26FR7uxuBD1qz3XRcK1se3GBLOwnWgpsK968WiQQNSPSQeNjUCaft3y4kk1G\n\tGCNgBofy+RU/mXdGRGLuehXh6bcMeohaqo8DwlAzh/VJBqmmTouxG+oG5EqDFWKmy9PJ\n\tY2JUC6ZaZtDGRYGdrvg2QrIBK37ZlL9J2CtzLPVviopW6L0JybS4UiCkC/BnTjJo6YxN\n\tvh8A==","X-Gm-Message-State":"AOJu0YyzDPR6sly3jl20B9fP4SR/GBmJlraaYpFqUCK3EemAfaBipTCC\n\t9gskeASMhPajHMEBUeEBKGFowP5QlViBPwN5+J3ndetgp4pt7HKrrktDdVfKtTCXKjhS+5JuSGB\n\t+yIZPZDae78mGh4A7gljqAi7tB8xdAjgwXdR6Hl/IpfSyTdgdYzjK","X-Gm-Gg":"ASbGncvTQkMYmovEes5tfSHIFVBOxpB5C00VtvNv6S3dlUZmIuLKGRseyGXb5hYenOX\n\t12N9Ou6DI3Qlq1RTHhX5UEpkmB5gD430QhM7rEFYQNz58nwPPo5aR3dN7VOje1W+Sqj2tYRwh0c\n\taiMdUCFtpmhq8rz+iymvcMzyCRwGHw5pdG","X-Google-Smtp-Source":"AGHT+IEwtk9V7xYYtFhcMztLcvJYiNv6H+gKS+ZlAZIQm4p9Em5WQHIj4VkyiAZH6g1nODQEopkIWx+2/IjIHFdSK8Q=","X-Received":"by 2002:a05:6102:3048:b0:4e2:8232:9c3c with SMTP id\n\tada2fe7eead31-4e282329dbdmr2179285137.8.1747728368091;\n\tTue, 20 May 2025 01:06:08 -0700 (PDT)","MIME-Version":"1.0","References":"<20250519092245.269048-1-naush@raspberrypi.com>\n\t<20250519092245.269048-2-naush@raspberrypi.com>\n\t","In-Reply-To":"","From":"Naushir Patuck ","Date":"Tue, 20 May 2025 09:05:33 +0100","X-Gm-Features":"AX0GCFukFfhe-97KzxrhdQikTSie9QllJAlh_yeeR0g7V3vl22RQqy_gKRPahnw","Message-ID":"","Subject":"Re: [PATCH v1 1/6] libcamera: framebuffer: Add\n\tFrameMetadata::Status::FrameStartup","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"multipart/alternative; boundary=\"0000000000008b5ef606358cb889\"","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":34288,"web_url":"https://patchwork.libcamera.org/comment/34288/","msgid":"","date":"2025-05-20T10:16:58","subject":"Re: [PATCH v1 1/6] libcamera: framebuffer: Add\n\tFrameMetadata::Status::FrameStartup","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Naush\n\nOn Tue, May 20, 2025 at 09:05:33AM +0100, Naushir Patuck wrote:\n> Hi Jacopo,\n>\n> On Tue, 20 May 2025 at 07:52, Jacopo Mondi \n> wrote:\n>\n> > Hi Naush\n> >\n> > On Mon, May 19, 2025 at 10:20:49AM +0100, Naushir Patuck wrote:\n> > > Add a new status enum, FrameStartup, used to denote that even though\n> > > the frame has been successfully captured, the IQ parameters set by the\n> > > IPA will cause the frame to be unusable and applications are advised to\n> > > not consume this frame. An example of this would be on a cold-start of\n> > > the 3A algorithms, and there will be large oscillations to converge to\n> > > a stable state quickly.\n> > >\n> > > Additional, update the definition of the FrameError state to include its\n> > > usage when the sensor is known to produce a number of invalid/error\n> > > frames after stream-on.\n> > >\n> > > Signed-off-by: Naushir Patuck \n> > > ---\n> > > include/libcamera/framebuffer.h | 1 +\n> > > src/libcamera/framebuffer.cpp | 11 +++++++++--\n> > > 2 files changed, 10 insertions(+), 2 deletions(-)\n> > >\n> > > diff --git a/include/libcamera/framebuffer.h\n> > b/include/libcamera/framebuffer.h\n> > > index ff83924300ac..e83825b466aa 100644\n> > > --- a/include/libcamera/framebuffer.h\n> > > +++ b/include/libcamera/framebuffer.h\n> > > @@ -26,6 +26,7 @@ struct FrameMetadata {\n> > > FrameSuccess,\n> > > FrameError,\n> > > FrameCancelled,\n> > > + FrameStartup,\n> > > };\n> > >\n> > > struct Plane {\n> > > diff --git a/src/libcamera/framebuffer.cpp\n> > b/src/libcamera/framebuffer.cpp\n> > > index 826848f75a56..36e56d593bc3 100644\n> > > --- a/src/libcamera/framebuffer.cpp\n> > > +++ b/src/libcamera/framebuffer.cpp\n> > > @@ -44,11 +44,18 @@ LOG_DEFINE_CATEGORY(Buffer)\n> > > * of the FrameMetadata structure are valid.\n> > > * \\var FrameMetadata::FrameError\n> > > * An error occurred during capture of the frame. The frame data may be\n> > partly\n> > > - * or fully invalid. The sequence and timestamp fields of the\n> > FrameMetadata\n> > > - * structure is valid, the other fields may be invalid.\n> > > + * or fully invalid. This status may also indicate an invalid frame\n> > produced by\n> > > + * the sensor during its startup or restart phase. The sequence and\n> > timestamp\n> > > + * fields of the FrameMetadata structure is valid, the other fields may\n> > be\n> > > + * invalid.\n> >\n> > This suggests to me that either you expect pipelines to set\n> >\n> > FrameError | FrameStartup\n> >\n> > to indicate a 'sensor startup' frame, which I don't think it's the\n> > intention here.\n> >\n> > I wonder if we actually add anything here.\n> >\n>\n> The intention is for the enum flags to be exclusive, i.e. either FrameError\n> or FrameStartup.\n\nAck, this was my understanding as well\n\n>\n> Here, I've overloaded the FrameError flag to capture the existing usage\n> like random transmission errors plus the bad frames produced by the sensor\n> device on startup.\n>\n>\n> > Or maybe just change the definition to:\n> >\n> > * The frame data may be partly or fully invalid. The sequence and\n> > * timestamp fields of the FrameMetadata structure is valid, the other\n> > * fields may be invalid.\n> >\n> > to remove the \"An error occurred\" part, as \"sensor startup\" frames are\n> > not errors, but contain known-to-be-invalid data.\n> >\n>\n> Maybe I should replace the word \"invalid\" with \"corrupt\" in the new\n> definition?\n>\n\nIdk, \"invalid\" seems more generic than \"corrupt\", and since we use FrameError\nto report frames whose post-processing failed, I would still use\n\"Invalid\"\n\n> \"\n> This status may also indicate corrupt frames produced by\n> the sensor during its startup or restart phase.\n> \"\n>\n\nI would rather not mention specific cases here, otherwise we should do\nthe same for all sources of invalid/corrupted frames.\n\n>\n> > What matters for applications, is, after all, knowing if frames can be\n> > consumed or not, so I think we can drop mentioning \"errors\" or more\n> > precise failure conditions.\n> >\n>\n> Having 2 flags does allow the application to know what's going on under the\n> hood, i.e. bad frames from the device, or bad IQ due to software. But I\n> could be convinced that these could be folded into one error flag if that's\n> a better way.\n>\n\nNo no, I'm not against 2 flags, I would simply avoid mentioning sensor\ncorrupted frames in the FrameError definition.\n\n>\n> >\n> >\n> > > * \\var FrameMetadata::FrameCancelled\n> > > * Capture stopped before the frame completed. The frame data is not\n> > valid. All\n> > > * fields of the FrameMetadata structure but the status field are\n> > invalid.\n> > > + * \\var FrameMetadata::FrameStartup The frame has been successfully\n> > captured.\n> >\n> > Following the above I would say that\n> >\n> > \"The frame data is valid, however the...\n> >\n> > > + * However, the IPA is in a cold-start or reset phase and will result\n> > in image\n> >\n> > Do we need to mention IPA ?\n> >\n>\n> Maybe s/IPA/imaging algorithms (3A)/ ?\n>\n>\n> >\n> > \".. however the capture pipeline is in a cold-start or reset\n> > phase\"\n> >\n> >\n> > > + * quality parameters producing unusable images. Applications are\n> > recommended to\n> > > + * not consume these frames. All fields of the FrameMetadata structure\n> > are\n> >\n> > All other fields ? Or are you ok with how it is ?\n> >\n>\n> Yes, I think all other metadata fields ought to report valid data in these\n> cases.\n\nYeah, my question was only about:\n\n\"All other fields\" vs\n\"All fields\"\n\nUp to you\n\n>\n> Regards,\n> Naush\n>\n>\n> >\n> > Thanks\n> > j\n> >\n> > > + * valid.\n> > > */\n> > >\n> > > /**\n> > > --\n> > > 2.43.0\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 68827BD78E\n\tfor ;\n\tTue, 20 May 2025 10:17:04 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2E57568D85;\n\tTue, 20 May 2025 12:17:03 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E136E614DE\n\tfor ;\n\tTue, 20 May 2025 12:17:01 +0200 (CEST)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2225F74C;\n\tTue, 20 May 2025 12:16:41 +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=\"I+Wewb7+\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1747736201;\n\tbh=1XbMWQZywaalk57UNKjGmmUX1g3xE0e1v+xIgTGqSpM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=I+Wewb7+LU0XKVjRqyiIQQITRJgl6Fk4cB3gK5B0d3y0egWWbGbubUlZED4cIniPz\n\tKDof2FHcoiF1AhTazRbt4koeBOEidaewvyG7m25XjMtlNTXynTl7BLvBC05l9F5hLf\n\tzfVHPFkd1yKTKV1H2smLNnBDvrpzdEhWCRpugquA=","Date":"Tue, 20 May 2025 12:16:58 +0200","From":"Jacopo Mondi ","To":"Naushir Patuck ","Cc":"Jacopo Mondi , \n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v1 1/6] libcamera: framebuffer: Add\n\tFrameMetadata::Status::FrameStartup","Message-ID":"","References":"<20250519092245.269048-1-naush@raspberrypi.com>\n\t<20250519092245.269048-2-naush@raspberrypi.com>\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","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":34295,"web_url":"https://patchwork.libcamera.org/comment/34295/","msgid":"","date":"2025-05-20T11:57:19","subject":"Re: [PATCH v1 1/6] libcamera: framebuffer: Add\n\tFrameMetadata::Status::FrameStartup","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Jacopo,\n\n\nOn Tue, 20 May 2025 at 11:17, Jacopo Mondi \nwrote:\n\n> Hi Naush\n>\n> On Tue, May 20, 2025 at 09:05:33AM +0100, Naushir Patuck wrote:\n> > Hi Jacopo,\n> >\n> > On Tue, 20 May 2025 at 07:52, Jacopo Mondi <\n> jacopo.mondi@ideasonboard.com>\n> > wrote:\n> >\n> > > Hi Naush\n> > >\n> > > On Mon, May 19, 2025 at 10:20:49AM +0100, Naushir Patuck wrote:\n> > > > Add a new status enum, FrameStartup, used to denote that even though\n> > > > the frame has been successfully captured, the IQ parameters set by\n> the\n> > > > IPA will cause the frame to be unusable and applications are advised\n> to\n> > > > not consume this frame. An example of this would be on a cold-start\n> of\n> > > > the 3A algorithms, and there will be large oscillations to converge\n> to\n> > > > a stable state quickly.\n> > > >\n> > > > Additional, update the definition of the FrameError state to include\n> its\n> > > > usage when the sensor is known to produce a number of invalid/error\n> > > > frames after stream-on.\n> > > >\n> > > > Signed-off-by: Naushir Patuck \n> > > > ---\n> > > > include/libcamera/framebuffer.h | 1 +\n> > > > src/libcamera/framebuffer.cpp | 11 +++++++++--\n> > > > 2 files changed, 10 insertions(+), 2 deletions(-)\n> > > >\n> > > > diff --git a/include/libcamera/framebuffer.h\n> > > b/include/libcamera/framebuffer.h\n> > > > index ff83924300ac..e83825b466aa 100644\n> > > > --- a/include/libcamera/framebuffer.h\n> > > > +++ b/include/libcamera/framebuffer.h\n> > > > @@ -26,6 +26,7 @@ struct FrameMetadata {\n> > > > FrameSuccess,\n> > > > FrameError,\n> > > > FrameCancelled,\n> > > > + FrameStartup,\n> > > > };\n> > > >\n> > > > struct Plane {\n> > > > diff --git a/src/libcamera/framebuffer.cpp\n> > > b/src/libcamera/framebuffer.cpp\n> > > > index 826848f75a56..36e56d593bc3 100644\n> > > > --- a/src/libcamera/framebuffer.cpp\n> > > > +++ b/src/libcamera/framebuffer.cpp\n> > > > @@ -44,11 +44,18 @@ LOG_DEFINE_CATEGORY(Buffer)\n> > > > * of the FrameMetadata structure are valid.\n> > > > * \\var FrameMetadata::FrameError\n> > > > * An error occurred during capture of the frame. The frame data\n> may be\n> > > partly\n> > > > - * or fully invalid. The sequence and timestamp fields of the\n> > > FrameMetadata\n> > > > - * structure is valid, the other fields may be invalid.\n> > > > + * or fully invalid. This status may also indicate an invalid frame\n> > > produced by\n> > > > + * the sensor during its startup or restart phase. The sequence and\n> > > timestamp\n> > > > + * fields of the FrameMetadata structure is valid, the other fields\n> may\n> > > be\n> > > > + * invalid.\n> > >\n> > > This suggests to me that either you expect pipelines to set\n> > >\n> > > FrameError | FrameStartup\n> > >\n> > > to indicate a 'sensor startup' frame, which I don't think it's the\n> > > intention here.\n> > >\n> > > I wonder if we actually add anything here.\n> > >\n> >\n> > The intention is for the enum flags to be exclusive, i.e. either\n> FrameError\n> > or FrameStartup.\n>\n> Ack, this was my understanding as well\n>\n> >\n> > Here, I've overloaded the FrameError flag to capture the existing usage\n> > like random transmission errors plus the bad frames produced by the\n> sensor\n> > device on startup.\n> >\n> >\n> > > Or maybe just change the definition to:\n> > >\n> > > * The frame data may be partly or fully invalid. The sequence and\n> > > * timestamp fields of the FrameMetadata structure is valid, the other\n> > > * fields may be invalid.\n> > >\n> > > to remove the \"An error occurred\" part, as \"sensor startup\" frames are\n> > > not errors, but contain known-to-be-invalid data.\n> > >\n> >\n> > Maybe I should replace the word \"invalid\" with \"corrupt\" in the new\n> > definition?\n> >\n>\n> Idk, \"invalid\" seems more generic than \"corrupt\", and since we use\n> FrameError\n> to report frames whose post-processing failed, I would still use\n> \"Invalid\"\n>\n> > \"\n> > This status may also indicate corrupt frames produced by\n> > the sensor during its startup or restart phase.\n> > \"\n> >\n>\n> I would rather not mention specific cases here, otherwise we should do\n> the same for all sources of invalid/corrupted frames.\n>\n> >\n> > > What matters for applications, is, after all, knowing if frames can be\n> > > consumed or not, so I think we can drop mentioning \"errors\" or more\n> > > precise failure conditions.\n> > >\n> >\n> > Having 2 flags does allow the application to know what's going on under\n> the\n> > hood, i.e. bad frames from the device, or bad IQ due to software. But I\n> > could be convinced that these could be folded into one error flag if\n> that's\n> > a better way.\n> >\n>\n> No no, I'm not against 2 flags, I would simply avoid mentioning sensor\n> corrupted frames in the FrameError definition.\n>\n\nAh, I think I understand - so you suggest that we make no change to the\ndefinition of FrameError, and just include the \"invalid\" frames to the\ncategory? I'm ok with this.\n\n\n>\n> >\n> > >\n> > >\n> > > > * \\var FrameMetadata::FrameCancelled\n> > > > * Capture stopped before the frame completed. The frame data is not\n> > > valid. All\n> > > > * fields of the FrameMetadata structure but the status field are\n> > > invalid.\n> > > > + * \\var FrameMetadata::FrameStartup The frame has been successfully\n> > > captured.\n> > >\n> > > Following the above I would say that\n> > >\n> > > \"The frame data is valid, however the...\n> > >\n> > > > + * However, the IPA is in a cold-start or reset phase and will\n> result\n> > > in image\n> > >\n> > > Do we need to mention IPA ?\n> > >\n> >\n> > Maybe s/IPA/imaging algorithms (3A)/ ?\n> >\n> >\n> > >\n> > > \".. however the capture pipeline is in a cold-start or reset\n> > > phase\"\n> > >\n> > >\n> > > > + * quality parameters producing unusable images. Applications are\n> > > recommended to\n> > > > + * not consume these frames. All fields of the FrameMetadata\n> structure\n> > > are\n> > >\n> > > All other fields ? Or are you ok with how it is ?\n> > >\n> >\n> > Yes, I think all other metadata fields ought to report valid data in\n> these\n> > cases.\n>\n> Yeah, my question was only about:\n>\n> \"All other fields\" vs\n> \"All fields\"\n>\n\n\"All other fields\" sounds correct, I'll update this.\n\nRegards,\nNaush\n\n\n> Up to you\n>\n> >\n> > Regards,\n> > Naush\n> >\n> >\n> > >\n> > > Thanks\n> > > j\n> > >\n> > > > + * valid.\n> > > > */\n> > > >\n> > > > /**\n> > > > --\n> > > > 2.43.0\n> > > >\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 B2EC8BD78E\n\tfor ;\n\tTue, 20 May 2025 11:58:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C3D8468D85;\n\tTue, 20 May 2025 13:57:59 +0200 (CEST)","from mail-ua1-x92e.google.com (mail-ua1-x92e.google.com\n\t[IPv6:2607:f8b0:4864:20::92e])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 57D26614DE\n\tfor ;\n\tTue, 20 May 2025 13:57:57 +0200 (CEST)","by mail-ua1-x92e.google.com with SMTP id\n\ta1e0cc1a2514c-87dbfb3b077so12074241.0\n\tfor ;\n\tTue, 20 May 2025 04:57:57 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"SV9ivYQs\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1747742276; x=1748347076;\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=2s3KPBsvAg27bxrNcp1mnzD4XzgvOUJm6TwNkjmzxFY=;\n\tb=SV9ivYQs3beI0EpZyPfLK4DmhimRUIKP36s/V/PPakn1myR88QrawBEDCWm99zXZNS\n\t1woMbLfDpVHmcuCwEAQwDgKWWuSClb0Myg7qMTjgOC7M1iF2FHHkSIfbS6GKfk/22HTh\n\tAUtDTAoNb/0f8akQssa6oPOu7ir1yLw89VOMojd+9JpZJnZdx3XLMTrus1KShEJcZ/8U\n\tkGHkF3UZ1mGYhaL2JNS+dYkgMoJis+avn2ap3vtvL0lAvKZ33Nj6ro9dts2Q3nEI2eXY\n\tYQbFL7CdVXImc11uMbpPViDrhG2nNOYMSQaM/AdJT/IFHTYSvAFVoT3M8AS+Ns8GRSJF\n\txCTw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1747742276; x=1748347076;\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=2s3KPBsvAg27bxrNcp1mnzD4XzgvOUJm6TwNkjmzxFY=;\n\tb=BUAWSxyJ6R3d3cwBCTIAMN96vWdhRPVwnd1c0sirwUzz2aUnT5rylxfh9BNWEc49hH\n\t+27hpKb7g60LDV9HdtYEtTd+g5TaCuy5Imh8IRXpgDdonZQa9m/9/BxXLaSkJXiq07zU\n\tcD5sjLm6yFZV9zBL800d9wIqT2sM+V2ciFdOqWP9lpKqGjS3BATJ98oFl02aHALIA/DL\n\tfIilg1w8WinYtJUlSsCnHdyfn5qVpjKZ47DshGCMVAE+b6SDHE7IAsbHjNCiDrTSQGrH\n\t1KUh4BvkQpsFzzNh5qDrGxXF7r5lFekuMvj0RHaCHl5tJG7q18VKkbvHGz8OQiB5CZkI\n\t735Q==","X-Gm-Message-State":"AOJu0YzsnDSzJDce7VjEndCqCUjpzkKOGt77icOwtfQhcApFbdh4SJY4\n\te+sNB/9RSjeNTJC3imnqreeC/x3UooTL/XBR6pp69kBv5YDSUHuSvTqJHK1+GwQBiUfn93CxPGN\n\t+Q4amnTCj60/gjCNBRwnc6crdApXJCTWJs/Y6ugUCyQ==","X-Gm-Gg":"ASbGncsIDWfK/QHq8rC4mFczfF6PsbS+mPiCx5P9c9CZmU9JZ39II/MxjIaIgOStDu4\n\tQGCpbhU1PQ7hTYscarO3LM06jHKRCcQ4ovI5EH9skAkuBfH7Cpx27Kxu/InZQsTsxna0dQSUgzp\n\ted2wL//ioUt35Je9NumByblCWVRRVktx89","X-Google-Smtp-Source":"AGHT+IFEFUKT6QCSJOzoQfRGOKGBpXxMP3AkH49HktLd9O3aaKnx5rxoYBvy9CBrz8EOhoAqb1Hr3cLreapRkFZjbKI=","X-Received":"by 2002:a05:6102:50a4:b0:4b2:5c0b:779e with SMTP id\n\tada2fe7eead31-4dfa6c4ab7bmr4720072137.5.1747742275997;\n\tTue, 20 May 2025 04:57:55 -0700 (PDT)","MIME-Version":"1.0","References":"<20250519092245.269048-1-naush@raspberrypi.com>\n\t<20250519092245.269048-2-naush@raspberrypi.com>\n\t\n\t\n\t","In-Reply-To":"","From":"Naushir Patuck ","Date":"Tue, 20 May 2025 12:57:19 +0100","X-Gm-Features":"AX0GCFtalSThwAXhcASS7Pgsr5twcDkGIQ_n_CI5lLBWZuW4xoJ5wbuRNOhhRb4","Message-ID":"","Subject":"Re: [PATCH v1 1/6] libcamera: framebuffer: Add\n\tFrameMetadata::Status::FrameStartup","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"multipart/alternative; boundary=\"00000000000085277606358ff5f4\"","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":34296,"web_url":"https://patchwork.libcamera.org/comment/34296/","msgid":"","date":"2025-05-20T12:41:20","subject":"Re: [PATCH v1 1/6] libcamera: framebuffer: Add\n\tFrameMetadata::Status::FrameStartup","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Naush\n\nOn Tue, May 20, 2025 at 12:57:19PM +0100, Naushir Patuck wrote:\n> Hi Jacopo,\n>\n>\n> On Tue, 20 May 2025 at 11:17, Jacopo Mondi \n> wrote:\n>\n> > Hi Naush\n> >\n> > On Tue, May 20, 2025 at 09:05:33AM +0100, Naushir Patuck wrote:\n> > > Hi Jacopo,\n> > >\n> > > On Tue, 20 May 2025 at 07:52, Jacopo Mondi <\n> > jacopo.mondi@ideasonboard.com>\n> > > wrote:\n> > >\n> > > > Hi Naush\n> > > >\n> > > > On Mon, May 19, 2025 at 10:20:49AM +0100, Naushir Patuck wrote:\n> > > > > Add a new status enum, FrameStartup, used to denote that even though\n> > > > > the frame has been successfully captured, the IQ parameters set by\n> > the\n> > > > > IPA will cause the frame to be unusable and applications are advised\n> > to\n> > > > > not consume this frame. An example of this would be on a cold-start\n> > of\n> > > > > the 3A algorithms, and there will be large oscillations to converge\n> > to\n> > > > > a stable state quickly.\n> > > > >\n> > > > > Additional, update the definition of the FrameError state to include\n> > its\n> > > > > usage when the sensor is known to produce a number of invalid/error\n> > > > > frames after stream-on.\n> > > > >\n> > > > > Signed-off-by: Naushir Patuck \n> > > > > ---\n> > > > > include/libcamera/framebuffer.h | 1 +\n> > > > > src/libcamera/framebuffer.cpp | 11 +++++++++--\n> > > > > 2 files changed, 10 insertions(+), 2 deletions(-)\n> > > > >\n> > > > > diff --git a/include/libcamera/framebuffer.h\n> > > > b/include/libcamera/framebuffer.h\n> > > > > index ff83924300ac..e83825b466aa 100644\n> > > > > --- a/include/libcamera/framebuffer.h\n> > > > > +++ b/include/libcamera/framebuffer.h\n> > > > > @@ -26,6 +26,7 @@ struct FrameMetadata {\n> > > > > FrameSuccess,\n> > > > > FrameError,\n> > > > > FrameCancelled,\n> > > > > + FrameStartup,\n> > > > > };\n> > > > >\n> > > > > struct Plane {\n> > > > > diff --git a/src/libcamera/framebuffer.cpp\n> > > > b/src/libcamera/framebuffer.cpp\n> > > > > index 826848f75a56..36e56d593bc3 100644\n> > > > > --- a/src/libcamera/framebuffer.cpp\n> > > > > +++ b/src/libcamera/framebuffer.cpp\n> > > > > @@ -44,11 +44,18 @@ LOG_DEFINE_CATEGORY(Buffer)\n> > > > > * of the FrameMetadata structure are valid.\n> > > > > * \\var FrameMetadata::FrameError\n> > > > > * An error occurred during capture of the frame. The frame data\n> > may be\n> > > > partly\n> > > > > - * or fully invalid. The sequence and timestamp fields of the\n> > > > FrameMetadata\n> > > > > - * structure is valid, the other fields may be invalid.\n> > > > > + * or fully invalid. This status may also indicate an invalid frame\n> > > > produced by\n> > > > > + * the sensor during its startup or restart phase. The sequence and\n> > > > timestamp\n> > > > > + * fields of the FrameMetadata structure is valid, the other fields\n> > may\n> > > > be\n> > > > > + * invalid.\n> > > >\n> > > > This suggests to me that either you expect pipelines to set\n> > > >\n> > > > FrameError | FrameStartup\n> > > >\n> > > > to indicate a 'sensor startup' frame, which I don't think it's the\n> > > > intention here.\n> > > >\n> > > > I wonder if we actually add anything here.\n> > > >\n> > >\n> > > The intention is for the enum flags to be exclusive, i.e. either\n> > FrameError\n> > > or FrameStartup.\n> >\n> > Ack, this was my understanding as well\n> >\n> > >\n> > > Here, I've overloaded the FrameError flag to capture the existing usage\n> > > like random transmission errors plus the bad frames produced by the\n> > sensor\n> > > device on startup.\n> > >\n> > >\n> > > > Or maybe just change the definition to:\n> > > >\n> > > > * The frame data may be partly or fully invalid. The sequence and\n> > > > * timestamp fields of the FrameMetadata structure is valid, the other\n> > > > * fields may be invalid.\n> > > >\n> > > > to remove the \"An error occurred\" part, as \"sensor startup\" frames are\n> > > > not errors, but contain known-to-be-invalid data.\n> > > >\n> > >\n> > > Maybe I should replace the word \"invalid\" with \"corrupt\" in the new\n> > > definition?\n> > >\n> >\n> > Idk, \"invalid\" seems more generic than \"corrupt\", and since we use\n> > FrameError\n> > to report frames whose post-processing failed, I would still use\n> > \"Invalid\"\n> >\n> > > \"\n> > > This status may also indicate corrupt frames produced by\n> > > the sensor during its startup or restart phase.\n> > > \"\n> > >\n> >\n> > I would rather not mention specific cases here, otherwise we should do\n> > the same for all sources of invalid/corrupted frames.\n> >\n> > >\n> > > > What matters for applications, is, after all, knowing if frames can be\n> > > > consumed or not, so I think we can drop mentioning \"errors\" or more\n> > > > precise failure conditions.\n> > > >\n> > >\n> > > Having 2 flags does allow the application to know what's going on under\n> > the\n> > > hood, i.e. bad frames from the device, or bad IQ due to software. But I\n> > > could be convinced that these could be folded into one error flag if\n> > that's\n> > > a better way.\n> > >\n> >\n> > No no, I'm not against 2 flags, I would simply avoid mentioning sensor\n> > corrupted frames in the FrameError definition.\n> >\n>\n> Ah, I think I understand - so you suggest that we make no change to the\n> definition of FrameError, and just include the \"invalid\" frames to the\n> category? I'm ok with this.\n\nMy suggestion is to drop the first sentence that mentions \"errors\nduring the capture\"\n\nFrom\n\n * \\var FrameMetadata::FrameError\n * An error occurred during capture of the frame. The frame data may be partly\n * or fully invalid. The sequence and timestamp fields of the FrameMetadata\n * structure is valid, the other fields may be invalid.\n\nto\n\n * \\var FrameMetadata::FrameError\n * The frame data may be partly or fully invalid. The sequence and\n * timestamp fields of the FrameMetadata structure is valid, the other\n * fields may be invalid.\n\n\n>\n>\n> >\n> > >\n> > > >\n> > > >\n> > > > > * \\var FrameMetadata::FrameCancelled\n> > > > > * Capture stopped before the frame completed. The frame data is not\n> > > > valid. All\n> > > > > * fields of the FrameMetadata structure but the status field are\n> > > > invalid.\n> > > > > + * \\var FrameMetadata::FrameStartup The frame has been successfully\n> > > > captured.\n> > > >\n> > > > Following the above I would say that\n> > > >\n> > > > \"The frame data is valid, however the...\n> > > >\n> > > > > + * However, the IPA is in a cold-start or reset phase and will\n> > result\n> > > > in image\n> > > >\n> > > > Do we need to mention IPA ?\n> > > >\n> > >\n> > > Maybe s/IPA/imaging algorithms (3A)/ ?\n> > >\n> > >\n> > > >\n> > > > \".. however the capture pipeline is in a cold-start or reset\n> > > > phase\"\n> > > >\n> > > >\n> > > > > + * quality parameters producing unusable images. Applications are\n> > > > recommended to\n> > > > > + * not consume these frames. All fields of the FrameMetadata\n> > structure\n> > > > are\n> > > >\n> > > > All other fields ? Or are you ok with how it is ?\n> > > >\n> > >\n> > > Yes, I think all other metadata fields ought to report valid data in\n> > these\n> > > cases.\n> >\n> > Yeah, my question was only about:\n> >\n> > \"All other fields\" vs\n> > \"All fields\"\n> >\n>\n> \"All other fields\" sounds correct, I'll update this.\n>\n\nThanks\n j\n\n> Regards,\n> Naush\n>\n>\n> > Up to you\n> >\n> > >\n> > > Regards,\n> > > Naush\n> > >\n> > >\n> > > >\n> > > > Thanks\n> > > > j\n> > > >\n> > > > > + * valid.\n> > > > > */\n> > > > >\n> > > > > /**\n> > > > > --\n> > > > > 2.43.0\n> > > > >\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 55A98BD78E\n\tfor ;\n\tTue, 20 May 2025 12:41:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2374068D8F;\n\tTue, 20 May 2025 14:41:26 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8D3EC68B67\n\tfor ;\n\tTue, 20 May 2025 14:41:24 +0200 (CEST)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7A0F374C;\n\tTue, 20 May 2025 14:41:03 +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=\"L9zfnEA7\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1747744863;\n\tbh=M0bGj2g4HAnPd9MsGUN8t7I6F/b5fwkq4J92+oYstaY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=L9zfnEA7jkjnFgaGOP6OcHS5HxHNbs1X1r0FREjRg7951H0D5jinQvFqnOoOTESGK\n\tuJG4ql9l7vH1RYLuYTtlCWt1o+TvxALLhUF/wKeTo+PAIOLl2WnK9kx6MGENDOTiiD\n\t9YvjQYAMA5Gyt5Vutbw8rmdidn6HbkGTRLuQ1uDc=","Date":"Tue, 20 May 2025 14:41:20 +0200","From":"Jacopo Mondi ","To":"Naushir Patuck ","Cc":"Jacopo Mondi , \n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v1 1/6] libcamera: framebuffer: Add\n\tFrameMetadata::Status::FrameStartup","Message-ID":"","References":"<20250519092245.269048-1-naush@raspberrypi.com>\n\t<20250519092245.269048-2-naush@raspberrypi.com>\n\t\n\t\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","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":34301,"web_url":"https://patchwork.libcamera.org/comment/34301/","msgid":"","date":"2025-05-21T10:35:22","subject":"Re: [PATCH v1 1/6] libcamera: framebuffer: Add\n\tFrameMetadata::Status::FrameStartup","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Naush\n one more question on this patch\n\nOn Mon, May 19, 2025 at 10:20:49AM +0100, Naushir Patuck wrote:\n> Add a new status enum, FrameStartup, used to denote that even though\n> the frame has been successfully captured, the IQ parameters set by the\n> IPA will cause the frame to be unusable and applications are advised to\n> not consume this frame. An example of this would be on a cold-start of\n> the 3A algorithms, and there will be large oscillations to converge to\n> a stable state quickly.\n>\n> Additional, update the definition of the FrameError state to include its\n> usage when the sensor is known to produce a number of invalid/error\n> frames after stream-on.\n>\n> Signed-off-by: Naushir Patuck \n> ---\n> include/libcamera/framebuffer.h | 1 +\n> src/libcamera/framebuffer.cpp | 11 +++++++++--\n> 2 files changed, 10 insertions(+), 2 deletions(-)\n>\n> diff --git a/include/libcamera/framebuffer.h b/include/libcamera/framebuffer.h\n> index ff83924300ac..e83825b466aa 100644\n> --- a/include/libcamera/framebuffer.h\n> +++ b/include/libcamera/framebuffer.h\n> @@ -26,6 +26,7 @@ struct FrameMetadata {\n> \t\tFrameSuccess,\n> \t\tFrameError,\n> \t\tFrameCancelled,\n> +\t\tFrameStartup,\n> \t};\n>\n> \tstruct Plane {\n> diff --git a/src/libcamera/framebuffer.cpp b/src/libcamera/framebuffer.cpp\n> index 826848f75a56..36e56d593bc3 100644\n> --- a/src/libcamera/framebuffer.cpp\n> +++ b/src/libcamera/framebuffer.cpp\n> @@ -44,11 +44,18 @@ LOG_DEFINE_CATEGORY(Buffer)\n> * of the FrameMetadata structure are valid.\n> * \\var FrameMetadata::FrameError\n> * An error occurred during capture of the frame. The frame data may be partly\n> - * or fully invalid. The sequence and timestamp fields of the FrameMetadata\n> - * structure is valid, the other fields may be invalid.\n> + * or fully invalid. This status may also indicate an invalid frame produced by\n> + * the sensor during its startup or restart phase. The sequence and timestamp\n> + * fields of the FrameMetadata structure is valid, the other fields may be\n> + * invalid.\n> * \\var FrameMetadata::FrameCancelled\n> * Capture stopped before the frame completed. The frame data is not valid. All\n> * fields of the FrameMetadata structure but the status field are invalid.\n> + * \\var FrameMetadata::FrameStartup The frame has been successfully captured.\n> + * However, the IPA is in a cold-start or reset phase and will result in image\n> + * quality parameters producing unusable images. Applications are recommended to\n> + * not consume these frames. All fields of the FrameMetadata structure are\n> + * valid.\n\nWill the metadata associated with a FrameStartup frame be considered\nvalid ?\n\n> */\n>\n> /**\n> --\n> 2.43.0\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 6B946C31E9\n\tfor ;\n\tWed, 21 May 2025 10:35:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9854568D7E;\n\tWed, 21 May 2025 12:35:27 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 04D5E68C91\n\tfor ;\n\tWed, 21 May 2025 12:35:25 +0200 (CEST)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 93CFC6B5;\n\tWed, 21 May 2025 12:35:04 +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=\"hk1vTX5g\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1747823704;\n\tbh=kcFm4vD+UcVuUZuO5kQpufdnxaAmKzFTJVJcUJAIgFs=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=hk1vTX5gHLCE4YpVLVa+GRQr5yeYpr7kklCMi95ZjciwJRan9jd1wMxhdqDzaq7p1\n\tMaOZdK/RaHRc0iT9OYzb6ah8/fSn1OPrCHZN+Li3hVFYldawTvH7lLo46Gc8H7t8V5\n\t0cSi/CnQTs2AKixJVLipVFIZOJ6UgnobTOwyNZ5U=","Date":"Wed, 21 May 2025 12:35:22 +0200","From":"Jacopo Mondi ","To":"Naushir Patuck ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v1 1/6] libcamera: framebuffer: Add\n\tFrameMetadata::Status::FrameStartup","Message-ID":"","References":"<20250519092245.269048-1-naush@raspberrypi.com>\n\t<20250519092245.269048-2-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20250519092245.269048-2-naush@raspberrypi.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":34302,"web_url":"https://patchwork.libcamera.org/comment/34302/","msgid":"","date":"2025-05-21T10:36:30","subject":"Re: [PATCH v1 1/6] libcamera: framebuffer: Add\n\tFrameMetadata::Status::FrameStartup","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Jacopo,\n\nOn Wed, 21 May 2025 at 11:35, Jacopo Mondi \nwrote:\n\n> Hi Naush\n> one more question on this patch\n>\n> On Mon, May 19, 2025 at 10:20:49AM +0100, Naushir Patuck wrote:\n> > Add a new status enum, FrameStartup, used to denote that even though\n> > the frame has been successfully captured, the IQ parameters set by the\n> > IPA will cause the frame to be unusable and applications are advised to\n> > not consume this frame. An example of this would be on a cold-start of\n> > the 3A algorithms, and there will be large oscillations to converge to\n> > a stable state quickly.\n> >\n> > Additional, update the definition of the FrameError state to include its\n> > usage when the sensor is known to produce a number of invalid/error\n> > frames after stream-on.\n> >\n> > Signed-off-by: Naushir Patuck \n> > ---\n> > include/libcamera/framebuffer.h | 1 +\n> > src/libcamera/framebuffer.cpp | 11 +++++++++--\n> > 2 files changed, 10 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/include/libcamera/framebuffer.h\n> b/include/libcamera/framebuffer.h\n> > index ff83924300ac..e83825b466aa 100644\n> > --- a/include/libcamera/framebuffer.h\n> > +++ b/include/libcamera/framebuffer.h\n> > @@ -26,6 +26,7 @@ struct FrameMetadata {\n> > FrameSuccess,\n> > FrameError,\n> > FrameCancelled,\n> > + FrameStartup,\n> > };\n> >\n> > struct Plane {\n> > diff --git a/src/libcamera/framebuffer.cpp\n> b/src/libcamera/framebuffer.cpp\n> > index 826848f75a56..36e56d593bc3 100644\n> > --- a/src/libcamera/framebuffer.cpp\n> > +++ b/src/libcamera/framebuffer.cpp\n> > @@ -44,11 +44,18 @@ LOG_DEFINE_CATEGORY(Buffer)\n> > * of the FrameMetadata structure are valid.\n> > * \\var FrameMetadata::FrameError\n> > * An error occurred during capture of the frame. The frame data may be\n> partly\n> > - * or fully invalid. The sequence and timestamp fields of the\n> FrameMetadata\n> > - * structure is valid, the other fields may be invalid.\n> > + * or fully invalid. This status may also indicate an invalid frame\n> produced by\n> > + * the sensor during its startup or restart phase. The sequence and\n> timestamp\n> > + * fields of the FrameMetadata structure is valid, the other fields may\n> be\n> > + * invalid.\n> > * \\var FrameMetadata::FrameCancelled\n> > * Capture stopped before the frame completed. The frame data is not\n> valid. All\n> > * fields of the FrameMetadata structure but the status field are\n> invalid.\n> > + * \\var FrameMetadata::FrameStartup The frame has been successfully\n> captured.\n> > + * However, the IPA is in a cold-start or reset phase and will result\n> in image\n> > + * quality parameters producing unusable images. Applications are\n> recommended to\n> > + * not consume these frames. All fields of the FrameMetadata structure\n> are\n> > + * valid.\n>\n> Will the metadata associated with a FrameStartup frame be considered\n> valid ?\n>\n\nYes, all metadata associated with FrameStartup ought to be accurate/valid.\n\nRegards,\nNaush\n\n\n> > */\n> >\n> > /**\n> > --\n> > 2.43.0\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 A8FB4BD78E\n\tfor ;\n\tWed, 21 May 2025 10:37:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 51D1D68D8C;\n\tWed, 21 May 2025 12:37:08 +0200 (CEST)","from mail-ua1-x933.google.com (mail-ua1-x933.google.com\n\t[IPv6:2607:f8b0:4864:20::933])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 283EB68C91\n\tfor ;\n\tWed, 21 May 2025 12:37:07 +0200 (CEST)","by mail-ua1-x933.google.com with SMTP id\n\ta1e0cc1a2514c-87dbfb3b077so96132241.0\n\tfor ;\n\tWed, 21 May 2025 03:37:07 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"R4mudKhT\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1747823826; x=1748428626;\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=g8pznFXOa2cMdhqp+J2EUv7Esl34/zMofEtdO7AKVTg=;\n\tb=R4mudKhTyvU4venHxkj1hekr9rGlXlkQvalSyM7Bb/xiD/aaLzhEx0cVHLK07NrQI5\n\t810FukClkxqMo7dPcFOJsZCM9kvqhGGi9P4m/LXGWfY2K1UsMDVk3LVk/UUsJLrHUFOr\n\tS72ZolmmZ7pPQywfIG2q4hiQqZkZFzPYcAYOvbMtRtD74/tNpyl08klNX64Z7WU9t97B\n\tscAWewOwUxZxk/R9RorZHuiXjPgHv4/NYUl70Rs62XxJLdsCNPvPM9ncfCL5U+FwSD5t\n\tfewcVHDihqvC6IQDwqIKpwb9BFdrT/zzYAA7ljRf/j4Kuak3lyYK/LY6f0X10Dm86IyA\n\tdNDw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1747823826; x=1748428626;\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=g8pznFXOa2cMdhqp+J2EUv7Esl34/zMofEtdO7AKVTg=;\n\tb=rXYYuzuX79eUOISpmfApiCbhrDntTkTDxIiuZ2Wb5XVORSkH12pyjRp/GfOTdW2cTj\n\toQyS9Xjg93T+udjTEulAWznEfbFJRMjzwK+sHBwUcRRG6FDn802Qk1fIUIVqwYGU+vUb\n\t4UDaWNu/ytn/3GCONCtTmM3Y2u51swISj7GQzOHVpLg+/Eh6/q9vqZsUxBusf3EA0Kqk\n\tD+CYXNcZnkCO0xx9yDFaMZmx45ukzXxJRSM+EFVFuALARKfyztKqW3oX0mbze1lCyJUu\n\tIXElsb0v2Sp1ozyR83CCJBNmB+l8ggit4xOMcmLvkJbznsRo8q/spR0CXma+44bRfdY6\n\t7dEg==","X-Gm-Message-State":"AOJu0Yyr9DpZwOqJEcZcRjW0Ubcqw94FGwF4ujnOeEZMVe1DmrvExivI\n\t+qs8LKVnQq9z3c35QWdNQNU4Ex/ECGwLPTbBaijVQz4x5abk0Zlw8rYiZEbslxd8PsoB0xUiCvP\n\tocV46ac5bVcANueaGVSYHTnj0yjojkJjFS9VgD2jv5zGPRxaAFJNa3iY=","X-Gm-Gg":"ASbGncsSiMtztfl+iwRKz0CQdMFMqpfV93k66qIQCO1cag6XncHdS48S4ijZepxIlI6\n\tbQDnFS/i987RkMOWe02csC/YBPBIYxzmZ93q10lQnQcVmQY8LOgP/WY6hDdGi3QrQcAg84SRJ9F\n\tfuGdbbrYyxrgt53xqaZUU9DFQpcNLKrbIZMtZxuWHTUiXF/RkkivOsh4uE4SkOGoUJ","X-Google-Smtp-Source":"AGHT+IEAqWRYhdjMdK3LYl5SC8hTOy2b/gWtpfO2PsuyHzmQPQPTlu6UGbqK9iXuyfBQsJeDIPLAVtYAsnKkH7MZlKY=","X-Received":"by 2002:a05:6102:5707:b0:4de:e4b3:7d with SMTP id\n\tada2fe7eead31-4dfa6cb511bmr6033424137.9.1747823825900;\n\tWed, 21 May 2025 03:37:05 -0700 (PDT)","MIME-Version":"1.0","References":"<20250519092245.269048-1-naush@raspberrypi.com>\n\t<20250519092245.269048-2-naush@raspberrypi.com>\n\t","In-Reply-To":"","From":"Naushir Patuck ","Date":"Wed, 21 May 2025 11:36:30 +0100","X-Gm-Features":"AX0GCFviwZpe-m0FDKWLU3VQp5fHZEUO7yiP706uJncBRX7UwpY4XDxhpmvSThc","Message-ID":"","Subject":"Re: [PATCH v1 1/6] libcamera: framebuffer: Add\n\tFrameMetadata::Status::FrameStartup","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"multipart/alternative; boundary=\"00000000000045edb40635a2f2cf\"","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":34303,"web_url":"https://patchwork.libcamera.org/comment/34303/","msgid":"<20250521113817.GH12514@pendragon.ideasonboard.com>","date":"2025-05-21T11:38:17","subject":"Re: [PATCH v1 1/6] libcamera: framebuffer: Add\n\tFrameMetadata::Status::FrameStartup","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, May 20, 2025 at 02:41:20PM +0200, Jacopo Mondi wrote:\n> On Tue, May 20, 2025 at 12:57:19PM +0100, Naushir Patuck wrote:\n> > On Tue, 20 May 2025 at 11:17, Jacopo Mondi wrote:\n> > > On Tue, May 20, 2025 at 09:05:33AM +0100, Naushir Patuck wrote:\n> > > > On Tue, 20 May 2025 at 07:52, Jacopo Mondi wrote:\n> > > > > On Mon, May 19, 2025 at 10:20:49AM +0100, Naushir Patuck wrote:\n> > > > > > Add a new status enum, FrameStartup, used to denote that even though\n> > > > > > the frame has been successfully captured, the IQ parameters set by the\n> > > > > > IPA will cause the frame to be unusable and applications are advised to\n> > > > > > not consume this frame. An example of this would be on a cold-start of\n> > > > > > the 3A algorithms, and there will be large oscillations to converge to\n> > > > > > a stable state quickly.\n> > > > > >\n> > > > > > Additional, update the definition of the FrameError state to include its\n> > > > > > usage when the sensor is known to produce a number of invalid/error\n> > > > > > frames after stream-on.\n> > > > > >\n> > > > > > Signed-off-by: Naushir Patuck \n> > > > > > ---\n> > > > > > include/libcamera/framebuffer.h | 1 +\n> > > > > > src/libcamera/framebuffer.cpp | 11 +++++++++--\n> > > > > > 2 files changed, 10 insertions(+), 2 deletions(-)\n> > > > > >\n> > > > > > diff --git a/include/libcamera/framebuffer.h\n> > > > > b/include/libcamera/framebuffer.h\n> > > > > > index ff83924300ac..e83825b466aa 100644\n> > > > > > --- a/include/libcamera/framebuffer.h\n> > > > > > +++ b/include/libcamera/framebuffer.h\n> > > > > > @@ -26,6 +26,7 @@ struct FrameMetadata {\n> > > > > > FrameSuccess,\n> > > > > > FrameError,\n> > > > > > FrameCancelled,\n> > > > > > + FrameStartup,\n> > > > > > };\n> > > > > >\n> > > > > > struct Plane {\n> > > > > > diff --git a/src/libcamera/framebuffer.cpp\n> > > > > b/src/libcamera/framebuffer.cpp\n> > > > > > index 826848f75a56..36e56d593bc3 100644\n> > > > > > --- a/src/libcamera/framebuffer.cpp\n> > > > > > +++ b/src/libcamera/framebuffer.cpp\n> > > > > > @@ -44,11 +44,18 @@ LOG_DEFINE_CATEGORY(Buffer)\n> > > > > > * of the FrameMetadata structure are valid.\n> > > > > > * \\var FrameMetadata::FrameError\n> > > > > > * An error occurred during capture of the frame. The frame data may be partly\n> > > > > > - * or fully invalid. The sequence and timestamp fields of the FrameMetadata\n> > > > > > - * structure is valid, the other fields may be invalid.\n> > > > > > + * or fully invalid. This status may also indicate an invalid frame produced by\n> > > > > > + * the sensor during its startup or restart phase. The sequence and timestamp\n> > > > > > + * fields of the FrameMetadata structure is valid, the other fields may be\n> > > > > > + * invalid.\n> > > > >\n> > > > > This suggests to me that either you expect pipelines to set\n> > > > >\n> > > > > FrameError | FrameStartup\n> > > > >\n> > > > > to indicate a 'sensor startup' frame, which I don't think it's the\n> > > > > intention here.\n> > > > >\n> > > > > I wonder if we actually add anything here.\n> > > >\n> > > > The intention is for the enum flags to be exclusive, i.e. either FrameError\n> > > > or FrameStartup.\n> > >\n> > > Ack, this was my understanding as well\n> > >\n> > > > Here, I've overloaded the FrameError flag to capture the existing usage\n> > > > like random transmission errors plus the bad frames produced by the sensor\n> > > > device on startup.\n> > > >\n> > > > > Or maybe just change the definition to:\n> > > > >\n> > > > > * The frame data may be partly or fully invalid. The sequence and\n> > > > > * timestamp fields of the FrameMetadata structure is valid, the other\n> > > > > * fields may be invalid.\n> > > > >\n> > > > > to remove the \"An error occurred\" part, as \"sensor startup\" frames are\n> > > > > not errors, but contain known-to-be-invalid data.\n> > > >\n> > > > Maybe I should replace the word \"invalid\" with \"corrupt\" in the new\n> > > > definition?\n> > >\n> > > Idk, \"invalid\" seems more generic than \"corrupt\", and since we use FrameError\n> > > to report frames whose post-processing failed, I would still use \"Invalid\"\n> > >\n> > > > \"\n> > > > This status may also indicate corrupt frames produced by\n> > > > the sensor during its startup or restart phase.\n> > > > \"\n> > >\n> > > I would rather not mention specific cases here, otherwise we should do\n> > > the same for all sources of invalid/corrupted frames.\n> > >\n> > > > > What matters for applications, is, after all, knowing if frames can be\n> > > > > consumed or not, so I think we can drop mentioning \"errors\" or more\n> > > > > precise failure conditions.\n> > > >\n> > > > Having 2 flags does allow the application to know what's going on under the\n> > > > hood, i.e. bad frames from the device, or bad IQ due to software. But I\n> > > > could be convinced that these could be folded into one error flag if that's\n> > > > a better way.\n> > >\n> > > No no, I'm not against 2 flags, I would simply avoid mentioning sensor\n> > > corrupted frames in the FrameError definition.\n> >\n> > Ah, I think I understand - so you suggest that we make no change to the\n> > definition of FrameError, and just include the \"invalid\" frames to the\n> > category? I'm ok with this.\n> \n> My suggestion is to drop the first sentence that mentions \"errors\n> during the capture\"\n> \n> From\n> \n> * \\var FrameMetadata::FrameError\n> * An error occurred during capture of the frame. The frame data may be partly\n> * or fully invalid. The sequence and timestamp fields of the FrameMetadata\n> * structure is valid, the other fields may be invalid.\n> \n> to\n> \n> * \\var FrameMetadata::FrameError\n> * The frame data may be partly or fully invalid. The sequence and\n> * timestamp fields of the FrameMetadata structure is valid, the other\n> * fields may be invalid.\n\nI find example useful to better understand the API. How about the\nfollowing ?\n\n * The frame data is partly or fully corrupted, missing or otherwise invalid.\n * This can for instance indicate a hardware transmission error, or invalid data\n * produced by the sensor during its startup phase. The sequence and timestamp\n * fields of the FrameMetadata structure is valid, all the other fields may be\n * invalid.\n\n> > > > > > * \\var FrameMetadata::FrameCancelled\n> > > > > > * Capture stopped before the frame completed. The frame data is not valid. All\n> > > > > > * fields of the FrameMetadata structure but the status field are invalid.\n> > > > > > + * \\var FrameMetadata::FrameStartup The frame has been successfully captured.\n> > > > >\n> > > > > Following the above I would say that\n> > > > >\n> > > > > \"The frame data is valid, however the...\n> > > > >\n> > > > > > + * However, the IPA is in a cold-start or reset phase and will result in image\n> > > > >\n> > > > > Do we need to mention IPA ?\n> > > >\n> > > > Maybe s/IPA/imaging algorithms (3A)/ ?\n> > > >\n> > > > > \".. however the capture pipeline is in a cold-start or reset\n> > > > > phase\"\n> > > > >\n> > > > > > + * quality parameters producing unusable images. Applications are recommended to\n> > > > > > + * not consume these frames. All fields of the FrameMetadata structure are\n> > > > >\n> > > > > All other fields ? Or are you ok with how it is ?\n> > > >\n> > > > Yes, I think all other metadata fields ought to report valid data in these\n> > > > cases.\n> > >\n> > > Yeah, my question was only about:\n> > >\n> > > \"All other fields\" vs\n> > > \"All fields\"\n> >\n> > \"All other fields\" sounds correct, I'll update this.\n> >\n> > > > > > + * valid.\n> > > > > > */\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 0C313C31E9\n\tfor ;\n\tWed, 21 May 2025 11:38:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A4D1868D8C;\n\tWed, 21 May 2025 13:38:24 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2E1DE68C91\n\tfor ;\n\tWed, 21 May 2025 13:38:23 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(179.218-130-109.adsl-dyn.isp.belgacom.be [109.130.218.179])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B49876AF;\n\tWed, 21 May 2025 13:38:01 +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=\"V9JDhJi7\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1747827481;\n\tbh=SiuiviX9hqKPjyCLCg0V7aMuKefpVHvFIiXpgrZK690=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=V9JDhJi7WovVDBoZfQhJ0ObVCfhNR929/BOeG2Z9J5SbYmQ/TC9iAud1KlrccVbTO\n\taAi4H/GBZNlA8ylwFq+zodg43a5fmyraS2cE/Ynl0jvpMTCvCNRcnmrCj+pPn15w4a\n\t8CLvykUUAf9yHoScTLMVxwNwb8kJsqYDBVPKz+sw=","Date":"Wed, 21 May 2025 13:38:17 +0200","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Cc":"Naushir Patuck ,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v1 1/6] libcamera: framebuffer: Add\n\tFrameMetadata::Status::FrameStartup","Message-ID":"<20250521113817.GH12514@pendragon.ideasonboard.com>","References":"<20250519092245.269048-1-naush@raspberrypi.com>\n\t<20250519092245.269048-2-naush@raspberrypi.com>\n\t\n\t\n\t\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","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":34304,"web_url":"https://patchwork.libcamera.org/comment/34304/","msgid":"","date":"2025-05-21T11:52:11","subject":"Re: [PATCH v1 1/6] libcamera: framebuffer: Add\n\tFrameMetadata::Status::FrameStartup","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Laurent,\n\n\nOn Wed, 21 May 2025 at 12:38, Laurent Pinchart <\nlaurent.pinchart@ideasonboard.com> wrote:\n\n> On Tue, May 20, 2025 at 02:41:20PM +0200, Jacopo Mondi wrote:\n> > On Tue, May 20, 2025 at 12:57:19PM +0100, Naushir Patuck wrote:\n> > > On Tue, 20 May 2025 at 11:17, Jacopo Mondi wrote:\n> > > > On Tue, May 20, 2025 at 09:05:33AM +0100, Naushir Patuck wrote:\n> > > > > On Tue, 20 May 2025 at 07:52, Jacopo Mondi wrote:\n> > > > > > On Mon, May 19, 2025 at 10:20:49AM +0100, Naushir Patuck wrote:\n> > > > > > > Add a new status enum, FrameStartup, used to denote that even\n> though\n> > > > > > > the frame has been successfully captured, the IQ parameters\n> set by the\n> > > > > > > IPA will cause the frame to be unusable and applications are\n> advised to\n> > > > > > > not consume this frame. An example of this would be on a\n> cold-start of\n> > > > > > > the 3A algorithms, and there will be large oscillations to\n> converge to\n> > > > > > > a stable state quickly.\n> > > > > > >\n> > > > > > > Additional, update the definition of the FrameError state to\n> include its\n> > > > > > > usage when the sensor is known to produce a number of\n> invalid/error\n> > > > > > > frames after stream-on.\n> > > > > > >\n> > > > > > > Signed-off-by: Naushir Patuck \n> > > > > > > ---\n> > > > > > > include/libcamera/framebuffer.h | 1 +\n> > > > > > > src/libcamera/framebuffer.cpp | 11 +++++++++--\n> > > > > > > 2 files changed, 10 insertions(+), 2 deletions(-)\n> > > > > > >\n> > > > > > > diff --git a/include/libcamera/framebuffer.h\n> > > > > > b/include/libcamera/framebuffer.h\n> > > > > > > index ff83924300ac..e83825b466aa 100644\n> > > > > > > --- a/include/libcamera/framebuffer.h\n> > > > > > > +++ b/include/libcamera/framebuffer.h\n> > > > > > > @@ -26,6 +26,7 @@ struct FrameMetadata {\n> > > > > > > FrameSuccess,\n> > > > > > > FrameError,\n> > > > > > > FrameCancelled,\n> > > > > > > + FrameStartup,\n> > > > > > > };\n> > > > > > >\n> > > > > > > struct Plane {\n> > > > > > > diff --git a/src/libcamera/framebuffer.cpp\n> > > > > > b/src/libcamera/framebuffer.cpp\n> > > > > > > index 826848f75a56..36e56d593bc3 100644\n> > > > > > > --- a/src/libcamera/framebuffer.cpp\n> > > > > > > +++ b/src/libcamera/framebuffer.cpp\n> > > > > > > @@ -44,11 +44,18 @@ LOG_DEFINE_CATEGORY(Buffer)\n> > > > > > > * of the FrameMetadata structure are valid.\n> > > > > > > * \\var FrameMetadata::FrameError\n> > > > > > > * An error occurred during capture of the frame. The frame\n> data may be partly\n> > > > > > > - * or fully invalid. The sequence and timestamp fields of the\n> FrameMetadata\n> > > > > > > - * structure is valid, the other fields may be invalid.\n> > > > > > > + * or fully invalid. This status may also indicate an invalid\n> frame produced by\n> > > > > > > + * the sensor during its startup or restart phase. The\n> sequence and timestamp\n> > > > > > > + * fields of the FrameMetadata structure is valid, the other\n> fields may be\n> > > > > > > + * invalid.\n> > > > > >\n> > > > > > This suggests to me that either you expect pipelines to set\n> > > > > >\n> > > > > > FrameError | FrameStartup\n> > > > > >\n> > > > > > to indicate a 'sensor startup' frame, which I don't think it's\n> the\n> > > > > > intention here.\n> > > > > >\n> > > > > > I wonder if we actually add anything here.\n> > > > >\n> > > > > The intention is for the enum flags to be exclusive, i.e. either\n> FrameError\n> > > > > or FrameStartup.\n> > > >\n> > > > Ack, this was my understanding as well\n> > > >\n> > > > > Here, I've overloaded the FrameError flag to capture the existing\n> usage\n> > > > > like random transmission errors plus the bad frames produced by\n> the sensor\n> > > > > device on startup.\n> > > > >\n> > > > > > Or maybe just change the definition to:\n> > > > > >\n> > > > > > * The frame data may be partly or fully invalid. The sequence\n> and\n> > > > > > * timestamp fields of the FrameMetadata structure is valid, the\n> other\n> > > > > > * fields may be invalid.\n> > > > > >\n> > > > > > to remove the \"An error occurred\" part, as \"sensor startup\"\n> frames are\n> > > > > > not errors, but contain known-to-be-invalid data.\n> > > > >\n> > > > > Maybe I should replace the word \"invalid\" with \"corrupt\" in the new\n> > > > > definition?\n> > > >\n> > > > Idk, \"invalid\" seems more generic than \"corrupt\", and since we use\n> FrameError\n> > > > to report frames whose post-processing failed, I would still use\n> \"Invalid\"\n> > > >\n> > > > > \"\n> > > > > This status may also indicate corrupt frames produced by\n> > > > > the sensor during its startup or restart phase.\n> > > > > \"\n> > > >\n> > > > I would rather not mention specific cases here, otherwise we should\n> do\n> > > > the same for all sources of invalid/corrupted frames.\n> > > >\n> > > > > > What matters for applications, is, after all, knowing if frames\n> can be\n> > > > > > consumed or not, so I think we can drop mentioning \"errors\" or\n> more\n> > > > > > precise failure conditions.\n> > > > >\n> > > > > Having 2 flags does allow the application to know what's going on\n> under the\n> > > > > hood, i.e. bad frames from the device, or bad IQ due to software.\n> But I\n> > > > > could be convinced that these could be folded into one error flag\n> if that's\n> > > > > a better way.\n> > > >\n> > > > No no, I'm not against 2 flags, I would simply avoid mentioning\n> sensor\n> > > > corrupted frames in the FrameError definition.\n> > >\n> > > Ah, I think I understand - so you suggest that we make no change to the\n> > > definition of FrameError, and just include the \"invalid\" frames to the\n> > > category? I'm ok with this.\n> >\n> > My suggestion is to drop the first sentence that mentions \"errors\n> > during the capture\"\n> >\n> > From\n> >\n> > * \\var FrameMetadata::FrameError\n> > * An error occurred during capture of the frame. The frame data may be\n> partly\n> > * or fully invalid. The sequence and timestamp fields of the\n> FrameMetadata\n> > * structure is valid, the other fields may be invalid.\n> >\n> > to\n> >\n> > * \\var FrameMetadata::FrameError\n> > * The frame data may be partly or fully invalid. The sequence and\n> > * timestamp fields of the FrameMetadata structure is valid, the other\n> > * fields may be invalid.\n>\n> I find example useful to better understand the API. How about the\n> following ?\n>\n> * The frame data is partly or fully corrupted, missing or otherwise\n> invalid.\n> * This can for instance indicate a hardware transmission error, or\n> invalid data\n> * produced by the sensor during its startup phase. The sequence and\n> timestamp\n> * fields of the FrameMetadata structure is valid, all the other fields\n> may be\n> * invalid.\n>\n\nThat works for me, I'll update the text as above.\n\nNaush\n\n\n>\n> > > > > > > * \\var FrameMetadata::FrameCancelled\n> > > > > > > * Capture stopped before the frame completed. The frame data\n> is not valid. All\n> > > > > > > * fields of the FrameMetadata structure but the status field\n> are invalid.\n> > > > > > > + * \\var FrameMetadata::FrameStartup The frame has been\n> successfully captured.\n> > > > > >\n> > > > > > Following the above I would say that\n> > > > > >\n> > > > > > \"The frame data is valid, however the...\n> > > > > >\n> > > > > > > + * However, the IPA is in a cold-start or reset phase and\n> will result in image\n> > > > > >\n> > > > > > Do we need to mention IPA ?\n> > > > >\n> > > > > Maybe s/IPA/imaging algorithms (3A)/ ?\n> > > > >\n> > > > > > \".. however the capture pipeline is in a cold-start or\n> reset\n> > > > > > phase\"\n> > > > > >\n> > > > > > > + * quality parameters producing unusable images. Applications\n> are recommended to\n> > > > > > > + * not consume these frames. All fields of the FrameMetadata\n> structure are\n> > > > > >\n> > > > > > All other fields ? Or are you ok with how it is ?\n> > > > >\n> > > > > Yes, I think all other metadata fields ought to report valid data\n> in these\n> > > > > cases.\n> > > >\n> > > > Yeah, my question was only about:\n> > > >\n> > > > \"All other fields\" vs\n> > > > \"All fields\"\n> > >\n> > > \"All other fields\" sounds correct, I'll update this.\n> > >\n> > > > > > > + * valid.\n> > > > > > > */\n> > > > > > >\n> > > > > > > /**\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\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 251B7BD78E\n\tfor ;\n\tWed, 21 May 2025 11:52:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1CE8368D92;\n\tWed, 21 May 2025 13:52:50 +0200 (CEST)","from mail-ua1-x92a.google.com (mail-ua1-x92a.google.com\n\t[IPv6:2607:f8b0:4864:20::92a])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 545D268C91\n\tfor ;\n\tWed, 21 May 2025 13:52:47 +0200 (CEST)","by mail-ua1-x92a.google.com with SMTP id\n\ta1e0cc1a2514c-87dbfb3b077so101143241.0\n\tfor ;\n\tWed, 21 May 2025 04:52:47 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"DbObW2TG\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1747828366; x=1748433166;\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=ChCv8H/pak55OWPyU+wJ7G87YcVLQnqo6sqqVaTfWCQ=;\n\tb=DbObW2TG5xtDWX2GDCUEyPuqL3dk3/D9WyIiaLLSdIW3i+vlzjiIf+z4blnmjh7SZP\n\tWnWUwFFp0YMapWNW/y4VBHxo7VQxx6OkkSndNT61lYjVJt1YM+m8V9UgcrImk4Gav90W\n\tXxwrduWW54tB7gVWK0Gjj+LXeSEsjzvAcUBojGKAIshZYr5J6gmMp0MQGSAOo9BASKeU\n\tR6fyHrXHt5wK3S6TXJsr+Qez3dSWJbLb11zccconJNTmTZAEwyIi+EawVve7QmQQlkh4\n\t4B+FoPBRSajJ3je648tJkm4gAPC7qM0EZV9urdsxPMA1IRfTLcZ1VeBwegZgGjE/4tQP\n\tXzwA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1747828366; x=1748433166;\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=ChCv8H/pak55OWPyU+wJ7G87YcVLQnqo6sqqVaTfWCQ=;\n\tb=cSAetOxFTgq6Hu3bXtCF1yZoi1RmBt8l826G52zav0IU4vaXZvUvoU/QpPCqfUQgCU\n\tu+/PwtVwWuDc3SIJSBZAIThr73I/oU9q4/9vT5YSR5UzapREueIbVCg/tjJghxjK0D8y\n\tjKJdauVWuFCda7wvGEd6CTmxxQHNLuyr3BhCs9tAEOwNlJynhqmmQ1IaySPZY8mzM5TB\n\tN6PbH4aU30iIX8uPTJDg3e4SGyWB6fxYPPR5yi9Ej3XCb1zswXrikXcwwG6sE42qsBnV\n\t8coHcnojeropF2qk1Mz5erWhvXRO6671GxjiSdlaj/HlB2IVLxYaIJ2xSkfOo9IyStII\n\tMejg==","X-Forwarded-Encrypted":"i=1;\n\tAJvYcCVnf1fz/fqs5EjpMZ3RAGTA2OM+t9FLi8KYOuO55N+/hUbKpL4iJ03yqkdhAbe3aKNyQZws8nTX/rx2daJYqR4=@lists.libcamera.org","X-Gm-Message-State":"AOJu0Yy57cHbaOnir/ZP+TAQoD8nBlD6t5vXAKrVJV3a4tfsq5556nLo\n\tHFLs4L97tqWISpqo4bNKr3M9Zt3TyzM2wDE/dz8NefgWd77+zlXKFUgMwGvvlkTvbKUq4qkZpvF\n\t2OIS4BytF/fC5hejWJ+pGuN5+srsLZKc0u6oXGFDN9iX6qfvdw4Z+","X-Gm-Gg":"ASbGncsWu/kDbbRjPNXT+p/ZsocSt1ij2IUUr3XDHrAx0lrhvriX7xYtLgus85e3naH\n\t05bHQQPNCx+OPsQWyvEVCsUjYpxvI4qFA/NBC32gpJSzrjmU0M6M4mbkYg5+pxm3NMizKqY0k9+\n\tcQAAjeVomuMF/vzh7tG8rsN+8vg3UDJWE39Ura80AZW4ULRJGjBUW+9Wbd22sXcxvw","X-Google-Smtp-Source":"AGHT+IGqz2qgOhC+wyPVSvv25gEk+7X/MeksArAMSn2E92DhT9jcn16flMJJsQJIsP+byFQDRa3oAy6xpUwxEinvFzA=","X-Received":"by 2002:a05:6102:26c8:b0:4e1:52fa:73e0 with SMTP id\n\tada2fe7eead31-4e152fa74b2mr3821521137.0.1747828366048;\n\tWed, 21 May 2025 04:52:46 -0700 (PDT)","MIME-Version":"1.0","References":"<20250519092245.269048-1-naush@raspberrypi.com>\n\t<20250519092245.269048-2-naush@raspberrypi.com>\n\t\n\t\n\t\n\t\n\t\n\t<20250521113817.GH12514@pendragon.ideasonboard.com>","In-Reply-To":"<20250521113817.GH12514@pendragon.ideasonboard.com>","From":"Naushir Patuck ","Date":"Wed, 21 May 2025 12:52:11 +0100","X-Gm-Features":"AX0GCFvZMeEdhI-IbkUd5w5R-Ed7pdwy5wwP71K1YantgvyWxjbNDUSFDUW39MM","Message-ID":"","Subject":"Re: [PATCH v1 1/6] libcamera: framebuffer: Add\n\tFrameMetadata::Status::FrameStartup","To":"Laurent Pinchart ","Cc":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org","Content-Type":"multipart/alternative; boundary=\"000000000000e317a20635a400cc\"","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\" "}}]