4 files changed, 203 insertions, 0 deletions
@@ -36,6 +36,9 @@ which matches the kernel driver, where `OV02C10_REG_ANALOG_GAIN` runs from | Patch | Fixes | | --- | --- | | `patches/0001-libipa-camera_sensor-add-ov02c10.patch` | Adds the `ov02c10` `CameraSensorHelper` (gain `code/16`, black level `0x40` at 10 bits) and its `camera_sensor_properties` entry. | +| `patches/0002-pipeline-simple-capture-bayer-order-from-mbus-code.patch` | Takes the capture device's Bayer order from the media bus code the pipeline reports instead of deriving it from the transform, so the rear camera streams. | +| `patches/0003-ipa-simple-data-add-ov02c10-tuning-file.patch` | Adds `ov02c10.yaml`, enabling a CCM for the front camera. Hand-tuned, not measured against a colour target. | +| `patches/0004-ipa-simple-awb-keep-gains-across-reconfiguration.patch` | Keeps the AWB gains over a reconfiguration, so a still captured right after one is not rendered with the sensor's raw white balance. | ## Versioning diff --git a/patches/0002-pipeline-simple-capture-bayer-order-from-mbus-code.patch b/patches/0002-pipeline-simple-capture-bayer-order-from-mbus-code.patch new file mode 100644 index 0000000..ac0298a --- /dev/null +++ b/patches/0002-pipeline-simple-capture-bayer-order-from-mbus-code.patch @@ -0,0 +1,59 @@ +pipeline: simple: Take the capture Bayer order from the reported media bus code + +When the media bus code that reaches the video node differs from the one the +pipeline was enumerated with, the simple pipeline handler recomputes the Bayer +order for the capture device by applying the configured transform to the +sensor's native order. That assumes the sensor's flip controls mirror the pixel +array and nothing else, so that the order after flipping is the mirror of the +order before it. + +The OV13858 on the Microsoft Surface Pro 12in breaks that assumption. It +reports V4L2_CTRL_FLAG_MODIFY_LAYOUT on V4L2_CID_HFLIP, so libcamera treats +flips as altering the Bayer order, and the sensor is mounted upside down +(camera_sensor_rotation is 180), so libcamera flips it to compensate. The +driver, however, keeps reporting SRGGB10_1X10 in every flip state, and +measurements off the CAMSS RDI video node with the sensor's own colour bar +pattern show that the flipped sensor really does emit RGGB, while the derived +order is GBRG: + + flips 0/0: greens on (0,0)/(1,1), i.e. GBRG emitted, SRGGB10 reported + flips 1/1: greens on (0,1)/(1,0), i.e. RGGB emitted, SRGGB10 reported + +The capture device is therefore configured as GBRG while every subdev along the +pipeline reports RGGB. camss validates exactly that pair in its +video_check_format(), so VIDIOC_STREAMON fails and the camera never produces a +frame: + + ERROR V4L2 v4l2_videodevice.cpp:1998 /dev/video2[57:cap]: Failed to start streaming: Broken pipe + +Derive the order from the code the pipeline actually reports instead. For a +sensor whose driver does change its code on flip the two are identical, so +nothing changes there, and for a driver that compensates the flip internally +the capture device now agrees with the subdev it is fed from. + +--- a/src/libcamera/pipeline/simple/simple.cpp ++++ b/src/libcamera/pipeline/simple/simple.cpp +@@ -1529,12 +1529,19 @@ + videoFormat = video->toV4L2PixelFormat(pipeConfig->captureFormat); + } else { + /* +- * Bayer pattern has changed because of the transform that was applied on +- * the sensor. Get the V4L2PixelFormat corresponding to the configured Bayer +- * pattern. ++ * The media bus code reaching the video node is not the one the ++ * pipeline was enumerated with, which for a Bayer sensor means ++ * the pattern changed, typically because a transform was ++ * applied to the sensor. ++ * ++ * Take the order from the code the pipeline actually reports ++ * rather than deriving it from the transform. A driver is free ++ * to compensate a flip internally, in which case the derived ++ * order and the reported one disagree, and it is the reported ++ * one that the capture device is validated against. + */ + BayerFormat cfgBayer = BayerFormat::fromPixelFormat(pipeConfig->captureFormat); +- cfgBayer.order = data->sensor_->bayerOrder(config->combinedTransform()); ++ cfgBayer.order = BayerFormat::fromMbusCode(format.code).order; + videoFormat = cfgBayer.toV4L2PixelFormat(); + } + diff --git a/patches/0003-ipa-simple-data-add-ov02c10-tuning-file.patch b/patches/0003-ipa-simple-data-add-ov02c10-tuning-file.patch new file mode 100644 index 0000000..24f2143 --- /dev/null +++ b/patches/0003-ipa-simple-data-add-ov02c10-tuning-file.patch @@ -0,0 +1,78 @@ +ipa: simple: data: Add an OV02C10 tuning file + +The simple IPA applies a colour correction matrix only when the tuning file +asks for it, and there is no tuning file for the OV02C10, so the front camera +of the Surface Pro 12in falls back to uncalibrated.yaml: + + WARN IPAProxy ipa_proxy.cpp:196 Configuration file 'ov02c10.yaml' not found for IPA module 'simple', falling back to '/usr/share/libcamera/ipa/simple/uncalibrated.yaml' + +That leaves the CCM at identity, i.e. the raw sensor channels are displayed as +if they were sRGB. The grey-world AWB does balance the frame, so the preview is +not tinted on average -- captured through PipeWire on this board it measures +R/G 1.03 and B/G 0.98 -- but it is flat: mean HSV saturation 0.07, with 94% of +the pixels below 0.15. Skin therefore lands close to grey and reads cyan. + +Add a tuning file that enables Ccm with a hand-tuned matrix whose blue row sums +to 0.85, both to restore saturation and to take the cool edge off what the +grey-world AWB leaves behind. It is not lab-calibrated, and the file says so. + +--- /dev/null ++++ b/src/ipa/simple/data/ov02c10.yaml +@@ -0,0 +1,47 @@ ++# SPDX-License-Identifier: CC0-1.0 ++# ++# Software ISP tuning for the OV02C10 front camera of the Microsoft Surface ++# Pro 12in. ++# ++# With no tuning file the simple IPA falls back to uncalibrated.yaml, which ++# runs no CCM: the raw sensor channels are handed to the display as though they ++# were already sRGB. The grey-world AWB still balances the frame on average, so ++# the preview is not tinted overall -- measured on this board its mean channel ++# ratios are R/G 1.03 and B/G 0.98 -- it is flat, with a mean HSV saturation of ++# 0.07 and 94% of the pixels below 0.15. What little colour survives reads ++# cool, and that is what makes skin look cyan. ++# ++# The matrix below is hand-tuned, not measured against a colour target. Its ++# off-diagonal terms undo part of the CFA crosstalk and so restore saturation, ++# and its blue row sums to 0.85 rather than 1.0, taking 15% out of blue to ++# counter the cool cast the grey-world AWB leaves behind. It comes from the ++# Samsung Galaxy Book OV02C10 work, which met the same sensor on the same ++# software ISP: ++# ++# https://github.com/hfb0/galaxy-book-ipu6-ov02c10-linux ++# ++# Both entries carry the same matrix, so nothing is interpolated over colour ++# temperature. There is no measured data to justify two different ones. ++# ++# The black level is deliberately not repeated here. The CameraSensorHelper ++# added by 0001-libipa-camera_sensor-add-ov02c10.patch already reports 0x40 at ++# 10 bits, which is what a dark frame off the CAMSS RDI node measures, and one ++# source for it is enough. ++# ++# Enabling Ccm also exposes the Saturation control, and costs nothing ++# noticeable here because the software ISP debayers on the GPU. ++%YAML 1.1 ++--- ++version: 1 ++algorithms: ++ - BlackLevel: ++ - Awb: ++ - Ccm: ++ ccms: ++ - ct: 2860 ++ ccm: [ 1.35, -0.20, -0.15, -0.15, 1.30, -0.15, -0.05, -0.20, 1.10 ] ++ - ct: 6500 ++ ccm: [ 1.35, -0.20, -0.15, -0.15, 1.30, -0.15, -0.05, -0.20, 1.10 ] ++ - Adjust: ++ - Agc: ++... +--- a/src/ipa/simple/data/meson.build ++++ b/src/ipa/simple/data/meson.build +@@ -1,6 +1,7 @@ + # SPDX-License-Identifier: CC0-1.0 + + conf_files = files([ ++ 'ov02c10.yaml', + 'uncalibrated.yaml', + ]) + diff --git a/patches/0004-ipa-simple-awb-keep-gains-across-reconfiguration.patch b/patches/0004-ipa-simple-awb-keep-gains-across-reconfiguration.patch new file mode 100644 index 0000000..8bc5628 --- /dev/null +++ b/patches/0004-ipa-simple-awb-keep-gains-across-reconfiguration.patch @@ -0,0 +1,63 @@ +ipa: simple: awb: Keep the gains across a reconfiguration + +Awb::configure() resets the colour gains to 1.0, so every reconfiguration +throws away the white balance the previous one had converged on and the first +frames of the new configuration are rendered with the sensor's raw white +balance. + +An application that reconfigures the camera and captures immediately therefore +gets a tinted image, which is what taking a still picture with GNOME Snapshot +does on the Surface Pro 12in: its preview is correctly balanced, and the +picture it writes is heavily green, because the still is captured at a +different resolution than the preview and the frame it keeps is the first one +after the reconfiguration. Captured through PipeWire, the front camera needs +two frames to converge: + + frame R/G B/G + 0 0.709 0.805 + 1 0.516 0.734 + 2 1.045 0.920 + 44 1.044 0.905 + +The gains describe the scene in front of the sensor, not the stream +configuration, so keep them and only initialise them once. + +--- a/src/ipa/simple/algorithms/awb.h ++++ b/src/ipa/simple/algorithms/awb.h +@@ -29,6 +29,9 @@ + IPAFrameContext &frameContext, + const SwIspStats *stats, + ControlList &metadata) override; ++ ++private: ++ bool initialised_ = false; + }; + + } /* namespace ipa::soft::algorithms */ +--- a/src/ipa/simple/algorithms/awb.cpp ++++ b/src/ipa/simple/algorithms/awb.cpp +@@ -26,8 +26,24 @@ + int Awb::configure(IPAContext &context, + [[maybe_unused]] const IPAConfigInfo &configInfo) + { ++ /* ++ * Initialise the gains on the first configuration only. ++ * ++ * A camera can be reconfigured while it keeps looking at the same ++ * scene, which is what an application does when it captures a still ++ * picture at a different resolution than the one its preview runs at. ++ * Discarding the gains that the previous configuration converged on ++ * then paints the first frames of the new one with the sensor's raw ++ * white balance, and an application that captures straight away gets a ++ * tinted picture. The gains are a property of the scene rather than of ++ * the stream configuration, so carry them over. ++ */ ++ if (initialised_) ++ return 0; ++ + auto &gains = context.activeState.awb.gains; + gains = { { 1.0, 1.0, 1.0 } }; ++ initialised_ = true; + + return 0; + } |