[PATCH 2/3] gstreamer: Generate controls from control_ids_*.yaml files
Kieran Bingham
kieran.bingham at ideasonboard.com
Thu Aug 8 11:50:10 CEST 2024
Hi Nicolas, Jaslo,
Adding comments where I think I can add helpful (I hope) feedback.
Jaslo - Thanks again for starting this work - It's certainly worthwhile
- even if there might be a few challenges below!
And I've already seen one person on IRC who was able to solve their
issues by using this series! \o/
Quoting Nicolas Dufresne (2024-08-07 20:13:08)
> Hi,
>
> I started inspecting the generated documentation. It is generally hard to get a
> perfectly clean results, but let me comment few things. We can at least try some
> improvement.
>
<snip>
> > ae-flicker-period : Manual flicker period in microseconds. This value sets the current flicker period to avoid. It is used when AeFlickerMode is set to FlickerManual. To cancel 50Hz mains flicker, this should be set to 10000 (corresponding to 100Hz), or 8333 (120Hz) for 60Hz mains. Setting the mode to FlickerManual when no AeFlickerPeriod has ever been set means that no flicker cancellation occurs (until the value of this control is updated). Switching to modes other than FlickerManual has no effect on the value of the AeFlickerPeriod control. See also: ae-flicker-mode.
> > flags: readable, writable, controllable
> > Integer. Range: -2147483648 - 2147483647 Default: 0
> >
> > ae-locked : Report the lock status of a running AE algorithm. If the AE algorithm is locked the value shall be set to true, if it's converging it shall be set to false. If the AE algorithm is not running the control shall not be present in the metadata control list. See also: ae-enable.
> > flags: readable, writable, controllable
> > Boolean. Default: false
>
> As this one is a status, it should only be readable, without the writable/controllable. If I'm not mistaken, you get this value withing the finalized request, would be nice enhancement to notify our users of the change using g_notify() and/or bus messages.
This comes down to the fact that readable controls / properties are only
reported in metadata and not exposed as a control by the camera...
> >
> > ae-metering-mode : Specify a metering mode for the AE algorithm to use. The metering modes determine which parts of the image are used to determine the scene brightness. Metering modes may be platform specific and not all metering modes may be supported.
> > flags: readable, writable, controllable
> > Enum "AeMeteringMode" Default: 0, "centre-weighted"
> > (0): centre-weighted - Centre-weighted metering mode.
> > (1): spot - Spot metering mode.
> > (2): matrix - Matrix metering mode.
> > (3): custom - Custom metering mode.
>
> From the doc, this seems pretty obscure. Shouldn't "custom" comes with another control ? Perhaps we introduce a mask to hide the controls that are not yet usable or that the usage isn't obvious and may need special case or rework ?
>
> >
> > af-metering : Instruct the AF algorithm how it should decide which parts of the image should be used to measure focus.
> > flags: readable, writable, controllable
> > Enum "AfMetering" Default: 0, "auto"
> > (0): auto - The AF algorithm should decide for itself where it will measure focus.
> > (1): windows - The AF algorithm should use the rectangles defined by the AfWindows control to measure focus. If no windows are specified the behaviour is platform dependent.
> >
> > af-mode : Control to set the mode of the AF (autofocus) algorithm. An implementation may choose not to implement all the modes.
> > flags: readable, writable, controllable
> > Enum "AfMode" Default: 0, "manual"
>
> Is that appropriate default ? Of course does not matter if there is no focus on the target. I wonder how you get to know if focus is there or not ...
Cameras without autofocus should not expose the controls, as they do not
'exist' and they will not be handled...
However - that doesn't fit in this gstreamer model where all controls
are going to be 'always' exposed. (Even if they are actually 'metadata
only' as well). It's up to the Camera at runtime to report what the
pipeline handler will accept based on the underlying hardware
capabilities or configuration.
In fact setting them would likely / potentially cause an assertion? (or
at least a failed request - potentially causing a frame drop... ?).
<snip>
> > Enum "AfState" Default: 0, "idle"
> > (0): idle - The AF algorithm is in manual mode (AfModeManual) or in auto mode (AfModeAuto) and a scan has not yet been triggered, or an in-progress scan was cancelled.
> > (1): scanning - The AF algorithm is in auto mode (AfModeAuto), and a scan has been started using the AfTrigger control. The scan can be cancelled by sending AfTriggerCancel at which point the algorithm will either move back to AfStateIdle or, if the scan actually completes before the cancel request is processed, to one of AfStateFocused or AfStateFailed. Alternatively the AF algorithm could be in continuous mode (AfModeContinuous) at which point it may enter this state spontaneously whenever it determines that a rescan is needed.
> > (2): focused - The AF algorithm is in auto (AfModeAuto) or continuous (AfModeContinuous) mode and a scan has completed with the result that the algorithm believes the image is now in focus.
> > (3): failed - The AF algorithm is in auto (AfModeAuto) or continuous (AfModeContinuous) mode and a scan has completed with the result that the algorithm did not find a good focus position.
> >
> > af-trigger : This control starts an autofocus scan when AfMode is set to AfModeAuto, and can also be used to terminate a scan early. It is ignored if AfMode is set to AfModeManual or AfModeContinuous.
> > flags: readable, writable, controllable
> > Enum "AfTrigger" Default: 0, "start"
> > (0): start - Start an AF scan. Ignored if a scan is in progress.
> > (1): cancel - Cancel an AF scan. This does not cause the lens to move anywhere else. Ignored if no scan is in progress.
>
> This one is missing something to make sense, at least from GStreamer point of view. When you are not starting or cancelling, what value this control should change to ? That question might be relevant for libcamera globally.
>
> Without this third state, I would say this one should not be there, and replace with action signals.
In libcamera - controls only 'exist' in a single request. What is an
'action signal' in Gstreamer?
So - AfTrigger for start and cancel are, in this case 'events'
- "Start an AFscan at this request"
- "Cancel any active scan"
There are no 'events' in between. Controls are only 'set' in Requests.
Any 'response' is only available in the metadata from a completed
Request.
You can not 'read' the state of a control in the same sense of a V4L2
control.
(Maybe this should change for some use cases, or for an abilty to set
controls directly on a camera, which I believe Stefan has explored, and
not through a request, but it's the current model).
> > analogue-gain : Analogue gain value applied in the sensor device. The value of the control specifies the gain multiplier applied to all colour channels. This value cannot be lower than 1.0. Setting this value means that it is now fixed and the AE algorithm may not change it. Setting it back to zero returns it to the control of the AE algorithm. See also: exposure-time, ae-enable. Todo: Document the interactions between AeEnable and setting a fixed value for this control. Consider interactions with other AE features, such as aperture and aperture/shutter priority mode, and decide if control of which features should be automatically adjusted shouldn't better be handled through a separate AE mode control.
> > flags: readable, writable, controllable
> > Float. Range: -3.402823e+38 - 3.402823e+38 Default: 0
>
> I'm not confident there is no range, would be nice if we can align to something sensible (might need yaml changes of course).
Different cameras will have different ranges ... only determinable at
runtime. We can't specify this in YAML. Though - it should always be
positive at least! (and as you've mentioned elsewhere, if it was
machine readable to say always above 1.0 would be beneficial too).
> > awb-enable : Enable or disable the AWB. See also: colour-gains.
> > flags: readable, writable, controllable
> > Boolean. Default: false
>
> Should be on by default (if not, we should do that in gst ;-P).
Maybe that's a GST decision. It is likely use case dependent.
But the same could be said for either default ... (and 'on' seems more
likely to be useful than 'off' in the majority case?)
> > awb-locked : Report the lock status of a running AWB algorithm. If the AWB algorithm is locked the value shall be set to true, if it's converging it shall be set to false. If the AWB algorithm is not running the control shall not be present in the metadata control list. See also: awb-enable.
> > flags: readable, writable, controllable
> > Boolean. Default: false
>
> Read-only.
Reported through metadata() same for all the other Read-only 'control
ids' I suspect.
>
> >
> > awb-mode : Specify the range of illuminants to use for the AWB algorithm. The modes supported are platform specific, and not all modes may be supported.
> > flags: readable, writable, controllable
> > Enum "AwbMode" Default: 0, "auto"
> > (0): auto - Search over the whole colour temperature range.
> > (1): incandescent - Incandescent AWB lamp mode.
> > (2): tungsten - Tungsten AWB lamp mode.
> > (3): fluorescent - Fluorescent AWB lamp mode.
> > (4): indoor - Indoor AWB lighting mode.
> > (5): daylight - Daylight AWB lighting mode.
> > (6): cloudy - Cloudy AWB lighting mode.
> > (7): custom - Custom AWB mode.
>
> Again, unclear how custom can be used.
I think/suspect this was added by Raspberry Pi. We should certainly
revisit this I think.
> > brightness : Specify a fixed brightness parameter. Positive values (up to 1.0) produce brighter images; negative values (up to -1.0) produce darker images and 0.0 leaves pixels unchanged.
> > flags: readable, writable, controllable
> > Float. Range: -3.402823e+38 - 3.402823e+38 Default: 0
>
> So this time the range is documented, we need a machine readable form.
Indeed.
> >
> > camera-name : Select by name which camera to use.
> > flags: readable, writable, changeable only in NULL or READY state
> > String. Default: null
> >
> > colour-correction-matrix: The 3x3 matrix that converts camera RGB to sRGB within the imaging pipeline. This should describe the matrix that is used after pixels have been white-balanced, but before any gamma transformation. The 3x3 matrix is stored in conventional reading order in an array of 9 floating point values.
> > flags: readable, writable, controllable
> > Default: "< >"
> > GstValueArray of GValues of type "gfloat"
>
> The type only allow flat list, but as its a matrix I was expecting
>
> "< < 1, 2, 3 >,
> < 4, 5, 6 >,
> < 7, 8, 9 > >"
>
> That being said I would be fine with flat raster data:
>
> "< 1, 2, 3,
> 4, 5, 6,
> 7, 8, 9 >"
>
> The down side is for bindings. Consider Python, the first would create a collection that you can access with matrix[Y][X] and more importantly can store into numpy.matrix.
>
> A comment for all list "< >" syntax. The usage and serialization syntax is not easy to guess. For matrix, if we had the identity matrix as default instead of an empty list, that would help. Otherwise we need a way to extend the doc for GStreamer usage.
>
I recall various bits of work on that recently...
But I think this is already handled through python. Stefan/Tomi ?
<snip>
> > draft-ae-precapture-trigger: Control for AE metering trigger. Currently identical to ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER. Whether the camera device will trigger a precapture metering sequence when it processes this request.
> > flags: readable, writable, controllable
> > Enum "AePrecaptureTrigger" Default: 0, "idle"
> > (0): idle - The trigger is idle.
> > (1): start - The pre-capture AE metering is started by the camera.
> > (2): cancel - The camera will cancel any active or completed metering sequence. The AE algorithm is reset to its initial state.
>
> Perhaps we can skip over draft- as this could later break our interface. We can also add an env to include them ?
Skipping them is an option. I think most of the 'draft' were only added
to match the Android interface. But I wonder how we'll handle other
namespaced controls like 'RPi' which I guess will be below...
<snip>
> I think my comments begins to be repetitive, and if we solve the earlier issues
> we will solve them all. The main thing is that the yaml should explicitly define
> more stuff rather then relying to text. And using that, we can easily adjust
Agreed, I think at least the yaml files could specify if a control can
only be accessed as metadata which would simplify all of the 'read-only'
issues.
> GStreamer properties. A final note, not all of these will be supported. I was
> wondering if we should add an extra property that tells use which one are
> actually supported ? We don't need anything fency, a "/" seperate list in a
> string or similar could do. We can also go for GstStructure.
I don't understand this part fully. Do you mean a new control/property
that gstreamer libcamerasrc will read at runtime from libcamera? It
already has a Camera->controls() that it can read that from.
But you mention GstStructure - do you mean that the libcamerasrc will
generate this ? (Perhaps from the Camera->controls()?)
> Nicolas
Kieran
More information about the libcamera-devel
mailing list