From patchwork Thu Dec 9 10:12:37 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Plowman X-Patchwork-Id: 15090 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 5393EBF415 for ; Thu, 9 Dec 2021 10:13:33 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id A5A9160876; Thu, 9 Dec 2021 11:13:32 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=raspberrypi.com header.i=@raspberrypi.com header.b="Wop+9SrC"; dkim-atps=neutral Received: from mail-wm1-x32e.google.com (mail-wm1-x32e.google.com [IPv6:2a00:1450:4864:20::32e]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 01F6E60224 for ; Thu, 9 Dec 2021 11:13:30 +0100 (CET) Received: by mail-wm1-x32e.google.com with SMTP id k37-20020a05600c1ca500b00330cb84834fso6147895wms.2 for ; Thu, 09 Dec 2021 02:13:30 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=raspberrypi.com; s=google; h=from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=DJhqL6zxRGmwohsCMajmPh6+F/ia3WjN1h8z2LN4OQw=; b=Wop+9SrCvHBjHBwDRSMLsZbseKnkcCPCyB44oLhMb3kdCR//CAx7Mg0pwCXgtllsrS ZqLxCSSjNYlECZnZRjdracoVZZ1Bh78jbdAMFAkHrmpm/lwFY59q0X6YcZOiYIv1dkB2 /j5uaVa2Pka9cjbYlxhS74l5NWZigwh1jjpJBXvFXZVxZ7Z/VnN6pLyJnqidsVg2RUR0 ZjfNMEH63pZbdX7LTywQkgSsli+DKNXNOjR0iSMsEtJh94SQ6o0/2vLrc5CORPKGLFIJ eZQ6h5AMKSwMTrNygG7sSDD1rTiheODhuGPrCnVCGfehTgW5wKP7z+aOcMObDolKlPT9 rGug== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=DJhqL6zxRGmwohsCMajmPh6+F/ia3WjN1h8z2LN4OQw=; b=MPvVMHUEMPij7AaYsQWcydeGsCULGPrcYdRfYO2bZ2WXTTrEchQaRXkOuR4lrlPB+j zjzfgDtFkn1+gXB46q4rsFDQIZxVpgrfNsCkkLKricUul4NcRt7cy6V3kl/g8KG0/JrE pXhSU4s7ldsBy9bpScCVFyziAlZEJ2m7forDn8lMgIDp7fRi8gIbdpcN68Ucdgdi3aIw 8xSpxtzock91GjArF+8HEKUpOJmSZ1HLROZcjdYOR6pe2JgOcLz4WBW/FSngQkvjM3Sx 2WO8ZK8G7ZNsKnzIomElONI3LXPfSClx5uW2O80RKwn3xXMBIa1JwCOD+0V5fc7iIgDw tNeA== X-Gm-Message-State: AOAM531k5WRcwIg7LzFK5UDsVfQ4GurD4f3wWL2nvcj3VX0+cNhLHckS 62we3licL4Pd2IHAeKGHZ5N6TEGlb8c+pbW6 X-Google-Smtp-Source: ABdhPJxMo2M9iYhuoJGpddugCgyRiiJwcewfEyxZLQGLohBpWg1PC2I54H1qmEJW+YmNOBQ/qxxAow== X-Received: by 2002:a05:600c:2297:: with SMTP id 23mr5738684wmf.73.1639044810261; Thu, 09 Dec 2021 02:13:30 -0800 (PST) Received: from pi4-davidp.pitowers.org ([2a00:1098:3142:14:e4a2:3070:eea4:e434]) by smtp.gmail.com with ESMTPSA id j8sm5079449wrh.16.2021.12.09.02.13.29 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 09 Dec 2021 02:13:29 -0800 (PST) From: David Plowman To: libcamera-devel@lists.libcamera.org, Laurent Pinchart , Kieran Bingham , Hans Verkuil , Tomasz Figa , Jacopo Mondi , Naushir Patuck Date: Thu, 9 Dec 2021 10:12:37 +0000 Message-Id: <20211209101245.6187-1-david.plowman@raspberrypi.com> X-Mailer: git-send-email 2.30.2 MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v10 0/8] Colour spaces X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Hi everyone We're on to version 10 now! Thanks for all the latest round of reviews, that's much appreciated. I've adopted pretty much all of those suggestions, but I'll try and round up the handful of discussion points into this cover note. 1. Should we have a "None" Y'CbCr encoding? I have added one as it bothered me to have "Rec601" in ColorSpace::Raw otherwise. I wonder slightly what you should do if a device returns V4L2_COLORSPACE_RAW and a non-default YCbCr encoding. What would that mean? Should we turn it into ColorSpace::Raw regardless, or take it at face value and let a pipeline handler complain (if it cares)? For now I've taken the second approach, I suspect that in reality no one is really going to care, but we could change that. 2. std::array in place of std::vector in color_space.cpp Unfortunately I still can't make the std::array constexpr, because the named ColorSpaces within them aren't. And they can't be because they're static class members which would have to be initialised within the class, only you can't initialise the objects of the class from within the declaration... But I have changed it to std::array anyway (just a const one), aren't they in theory marginally cheaper than std::vectors? 3. Error messages in v4l2_videodevice.cpp etc. Should these be in to/fromColorSpace instead? I've left them where they are because I can still imagine a pipeline handler calling these functions to check the results for themselves, at which point functions that spit out messages are just annoying. But I don't feel very strongly about this given that currently no one else is calling them, so would be happy enough to change it! 4. Why should the largest output stream determine the color space? Because it's the biggest, therefore it's the most important. :) Though to be fair sometimes we use our low resolution output stream for image analysis that users never see, so the colour space there probably is less important. But in the end we have to pick one of them and it doesn't really matter that much which. 5. Warnings in the RPi pipeline handler... I've removed one bunch of warnings on the assurance that validate() has been called... Some others I've left in because they really shouldn't happen and I am completely paranoid about colour spaces. If I could be less anxious about them that would be nice... 6. Where to call validateColorSpaces? I've left this where it is. The one thing we're never going to do is change raw pixel formats to non-raw ones, and vice versa. And mostly I think pixel formats have little to do with colour spaces. There are some exceptions (e.g. raw formats imply "raw" colour spaces), and YUV type outputs imply using the Y'CbCr encoding matrix which is sort of related, but apart from that I think they're mostly distinct. (Everyone will now list more exceptions...) Anyway, thanks again for all the effort that has gone into the many reviews! Best regards David David Plowman (8): libcamera: Add ColorSpace class libcamera: stream: Add ColorSpace fields to StreamConfiguration libcamera: video_device: Convert between ColorSpace class and V4L2 formats libcamera: video_device: Support passing ColorSpaces to V4L2 video devices libcamera: v4l2_subdevice: Add colorSpace field to V4L2SubdeviceFormat libcamera: v4l2_subdevice: Support passing ColorSpaces to V4L2 subdevices libcamera: Add validateColorSpaces to CameraConfiguration class libcamera: pipeline: raspberrypi: Support color spaces include/libcamera/camera.h | 10 + include/libcamera/color_space.h | 70 ++++ include/libcamera/internal/v4l2_device.h | 8 + include/libcamera/internal/v4l2_subdevice.h | 3 + include/libcamera/internal/v4l2_videodevice.h | 3 + include/libcamera/meson.build | 1 + include/libcamera/stream.h | 3 + src/libcamera/camera.cpp | 84 +++++ src/libcamera/camera_sensor.cpp | 1 + src/libcamera/color_space.cpp | 318 ++++++++++++++++++ src/libcamera/meson.build | 1 + src/libcamera/pipeline/ipu3/cio2.cpp | 7 +- .../pipeline/raspberrypi/raspberrypi.cpp | 40 +++ src/libcamera/pipeline/simple/simple.cpp | 8 +- src/libcamera/stream.cpp | 20 ++ src/libcamera/v4l2_device.cpp | 194 +++++++++++ src/libcamera/v4l2_subdevice.cpp | 25 +- src/libcamera/v4l2_videodevice.cpp | 32 ++ 18 files changed, 821 insertions(+), 7 deletions(-) create mode 100644 include/libcamera/color_space.h create mode 100644 src/libcamera/color_space.cpp