[{"id":32034,"web_url":"https://patchwork.libcamera.org/comment/32034/","msgid":"<20241105162002.GD6317@pendragon.ideasonboard.com>","date":"2024-11-05T16:20:02","subject":"Re: [PATCH 0/1] Add v4l2_subdev_format in V4L2SubdeviceFormat","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Harvey,\n\nOn Tue, Nov 05, 2024 at 10:49:20AM +0000, Harvey Yang wrote:\n> Hi folks,\n> \n> This patch adds extra information in V4L2SubdeviceFormat, which will\n> be needed in mtkisp7:\n> https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/third_party/libcamera/mtkisp7/src/libcamera/pipeline/mtkisp7/camsys/camsys.cpp;l=371\n> \n> I think we can also choose to just add the format (type:\n> v4l2_mbus_framefmt) into the struct though. However, in either approach,\n> we are duplicating the information with other existing member variables.\n> \n> Please check if this patch makes sense, or if there are other\n> approaches.\n\nI'll return the question: could you explain why this solution makes the\nmost sense, and what other approaches you've considered and decided to\nreject, if any ? Giving us rationales for the design decisions behind\nyour patch series is crucial for us to understand what you're doing, and\nwill help speeding up the discussions and merging of the code.\n\n> Han-Lin Chen (1):\n> libcamera: Add v4l2_subdev_format in V4L2SubdeviceFormat\n> \n> include/libcamera/internal/v4l2_subdevice.h | 2 ++\n> src/libcamera/pipeline/simple/simple.cpp | 2 +-\n> src/libcamera/sensor/camera_sensor.cpp | 1 +\n> src/libcamera/v4l2_subdevice.cpp | 10 ++++++++++\n> 4 files changed, 14 insertions(+), 1 deletion(-)\n> \n> -- \n> 2.47.0.199.ga7371fff76-goog","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 0F3AEBDB1C\n\tfor ;\n\tTue, 5 Nov 2024 16:20:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 38679653FA;\n\tTue, 5 Nov 2024 17:20:15 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 072186035B\n\tfor ;\n\tTue, 5 Nov 2024 17:20:12 +0100 (CET)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 216DA874;\n\tTue, 5 Nov 2024 17:20:02 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"QGL9Upoo\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1730823603;\n\tbh=rbIXfUj96PHXwXYVoPaZtJK5LeIK8Afmr1fJVxAOTWQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=QGL9Upoow4ue2mrO3eauKct1vaQCqDA9RJZmbmisBhDMJc4ARx7aIx7OUVhxgUucv\n\tuiNm3EPG80pju5HXlU+izjOUcBRMxy91yV/P0nzA/qORWbuM8GXq1C5MPrK8UEZgei\n\t1gyen1AmGzIzNWKOLQzWa2/jnuDlKqfQLo1zyF5Q=","Date":"Tue, 5 Nov 2024 18:20:02 +0200","From":"Laurent Pinchart ","To":"Harvey Yang ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 0/1] Add v4l2_subdev_format in V4L2SubdeviceFormat","Message-ID":"<20241105162002.GD6317@pendragon.ideasonboard.com>","References":"<20241105105445.1468954-1-chenghaoyang@chromium.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20241105105445.1468954-1-chenghaoyang@chromium.org>","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\" "}}]