| Message ID | 20211001103325.1077590-2-paul.elder@ideasonboard.com |
|---|---|
| State | Superseded |
| Delegated to: | Paul Elder |
| Headers | show |
| Series |
|
| Related | show |
Hi Paul Thanks for all this work! I was fine with all the functionality being described, and mostly everything made sense to me, but there were just a couple of places where I thought we might be able to explain things more clearly. But this is just my opinion, others may disagree, so please take with the necessary quantities of salt! On Fri, 1 Oct 2021 at 11:33, Paul Elder <paul.elder@ideasonboard.com> wrote: > > We have multiple goals: > - we need a lock of some sort, to instruct the AEGC to not update output > results > - we need manual modes, to override the values computed by the AEGC > - we need to support seamless transitions from auto -> manual, and do so > without flickering > - we need custom minimum values for the manual controls, that is no > magic values for enabling/disabling auto > - all of these need to be done with AE sub-controls (exposure time, > analogue gain) > > To achieve these goals, we introduce mode controls for the AE > sub-controls: ExposureTimeMode and AnalogueGainMode. These have an auto > state, and a disabled state. The disabled state has an internal one-way > state change from locked to manual, triggered by the presence of the > value-controls (ExposureTime and AnalogueGain). > > We then remove the AeEnable control, as it is a redundant control in the > face of these two mode controls. > > We also remove AeLocked, as it is insufficient for reporting the AE > state, and we promote AeState to non-draft to fill its role. Notably, > the locked state is removed, since this information can be obtained from > the aforementioned mode controls. > > Bug: https://bugs.libcamera.org/show_bug.cgi?id=42 > Bug: https://bugs.libcamera.org/show_bug.cgi?id=43 > Bug: https://bugs.libcamera.org/show_bug.cgi?id=47 > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > > --- > Changes in v2: > - No changes, just resubmitting at the head of this series so that it's > together and so that /people will actually see it/ > > Initial version: > Still RFC as I haven't updated the users of the control yet, and I want > to check that these are the controls and docs that we want. > > We've decided that the "master AE control" will be implemented by a > helper... but looking at uvcvideo and the V4L2 controls I'm wondering if > such helper should come earlier than later? > --- > src/libcamera/control_ids.yaml | 215 +++++++++++++++++++++++---------- > 1 file changed, 150 insertions(+), 65 deletions(-) > > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml > index 9d4638ae..18b186e8 100644 > --- a/src/libcamera/control_ids.yaml > +++ b/src/libcamera/control_ids.yaml > @@ -7,23 +7,61 @@ > # Unless otherwise stated, all controls are bi-directional, i.e. they can be > # set through Request::controls() and returned out through Request::metadata(). > controls: > - - AeEnable: > - type: bool > - description: | > - Enable or disable the AE. > - > - \sa ExposureTime AnalogueGain > - > - - AeLocked: > - type: bool > + - AeState: > + type: int32_t > description: | > - 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. > + Control to report the current AE algorithm state. Enabling or disabling > + any of the AE-related mode controls (eg. AnalogueGainMode, > + ExposureTimeMode) is not required to reset the state to > + AeStateInactive. The camera device can do several state transitions > + between two results, if it is allowed by the state transition table. > + For example, AeStateInactive may never actually be seen in a result. I was fine until we got to that bit "Enabling or disabling". The rest of this paragraph struck me as a real burst of detail and corner cases (?) while I was still struggling with what AeStateInactive meant. Perhaps some of this specific stuff can be moved until after all the enum values have been explained? I found the enum descriptions quite easy to follow and having read through those felt in a better place to tackle the details. Having said that, I'm still a bit vague as to why "resetting the state to AeStateInactive" is a particular issue of concern? > + > + The state in the result is the state for this image (in sync with this > + image). If AE state becomes AeStateConverged, then the image data > + associated with the result should be good to use. > + > + As some AE algorithms may still be running when any of the AE-related > + controls are in manual mode, the states are valid even when the mode > + controls are set to Disabled. To know if this image image used AE or "image" is repeated. Do we need to say anything about what happens when all controls are disabled? (Does anything different happen??) > + manual (or a mixture), check the relevant modes (eg. AnalogueGainMode, > + ExposureTimeMode). > + > + \sa AnalogueGain > + \sa AnalogueGainMode > + \sa ExposureTime > + \sa ExposureTimeMode > > - \sa AeEnable > + enum: > + - name: AeStateInactive > + value: 0 > + description: | > + The AE algorithm is inactive. > + If the camera initiates an AE scan, the state shall go to Searching. > + - name: AeStateSearching > + value: 1 > + description: | > + The AE algorithm has not converged yet. > + If the camera finishes an AE scan, the state shall go to Converged. > + If the camera finishes an AE scan, but flash is required, the state > + shall go to FlashRequired. > + - name: AeStateConverged > + value: 2 > + description: | > + The AE algorithm has converged. > + If the camera initiates an AE scan, the state shall go to Searching. > + - name: AeStateFlashRequired > + value: 3 > + description: | > + The AE algorithm would need a flash for good results > + If the camera initiates an AE scan, the state shall go to Searching. > + If AeLock is on, the state shall go to Locked. > + - name: AeStatePrecapture > + value: 4 > + description: | > + The AE algorithm has started a pre-capture metering session. > + After the sequence is finished, the state shall go to Converged if This was all good, my only minor comment might be that using AeStateConverged and AeStateSearching in place of just Converged/Searching might be a little more precise? > + \sa AePrecaptureTrigger > > # AeMeteringMode needs further attention: > # - Auto-generate max enum value. > @@ -93,6 +131,17 @@ controls: > how the desired total exposure is divided between the shutter time > and the sensor's analogue gain. The exposure modes are platform > specific, and not all exposure modes may be supported. > + > + When one of AnalogueGainMode or ExposureTimeMode is set to Disabled and > + a corresponding manual control is provided, AeExposureMode may be Actually I think the same is true even if you don't give a manual value. As soon as you fix any of them, the other(s) will vary in such a way as to ignore the exposure profile. > + ignored. This is because, for example, if AeExposureMode is set to > + ExposureLong but ExposureTimeMode is Disabled and a short ExposureTime > + is provided (AnalogueGainMode set to Auto), then the AeExposureMode > + doesn't make sense. The example is good, but seemed a bit wordy to me? Maybe the whole thing might be: "When any of AnalogueGainMode or ExposureTimeMode is set to Disabled. the fixed values will override any choices made by the AeExposureMode which may consequently be ignored." (Or perhaps I'm assuming a deeper understanding on the part of the reader than most will have?) > + > + \sa AnalogueGainMode > + \sa ExposureTimeMode > + > enum: > - name: ExposureNormal > value: 0 > @@ -111,13 +160,15 @@ controls: > type: float > description: | > Specify an Exposure Value (EV) parameter. The EV parameter will only be > - applied if the AE algorithm is currently enabled. > + applied if the AE algorithm is currently enabled, that is, at least one > + of AnalogueGainMode and ExposureTimeMode are auto. > > By convention EV adjusts the exposure as log2. For example > EV = [-2, -1, 0.5, 0, 0.5, 1, 2] results in an exposure adjustment > of [1/4x, 1/2x, 1/sqrt(2)x, 1x, sqrt(2)x, 2x, 4x]. > > - \sa AeEnable > + \sa AnalogueGainMode > + \sa ExposureTimeMode > > - ExposureTime: > type: int32_t > @@ -125,17 +176,49 @@ controls: > Exposure time (shutter speed) for the frame applied in the sensor > device. This value is specified in micro-seconds. > > - 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. > + This control will only take effect if ExposureTimeMode is Disabled. Its > + presence in a request acts as a trigger to switch to the internal > + manual mode within ExposureTimeModeDisabled. > + > + When reported in metadata, this control indicates what exposure time > + was used for the current request, regardless of ExposureTimeMode. > + ExposureTimeMode will indicate the source of the exposure time value, > + whether it came from the AE algorithm or not. > > - \sa AnalogueGain AeEnable > + \sa AnalogueGain > + \sa ExposureTimeMode > > - \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. > + - ExposureTimeMode: > + type: int32_t > + description: | > + Control to set and report the source of the exposure time that will be > + applied. > + > + \sa ExposureTime > + enum: > + - name: ExposureTimeModeAuto > + value: 0 > + description: | > + The exposure time will be calculated automatically and set by the > + AE algorithm. > + - name: ExposureTimeModeDisabled > + value: 1 > + description: | > + The exposure time will not be updated by the AE algorithm. It will > + come from the last calculated value when the mode was Auto, or from > + the value specified in ExposureTime. > + > + This Disabled state has two internal states; locked and manual. TBH I still struggle a bit with the word "locked". I think I prefer "fixed", it seems more obvious. But not to worry... if the Android world prefers it then I will get used to it! Thanks again for doing all this! Best regards David > + When the Disabled state is first entered, the internal state will > + be locked, and the latest exposure time value set by the AE > + algorithm will be used (or the default ExposureTime value, if the > + mode started out as Disabled). When an ExposureTime is provided, > + then the internal state will go to manual, and the provided value > + will be used for the exposure time. Going from manual to locked is > + not possible. If an ExposureTime value is provided in the same > + request as going into the Disabled state, then the internal locked > + state will be skipped, and the internal state will start out as > + manual. > > - AnalogueGain: > type: float > @@ -144,17 +227,49 @@ controls: > 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. > + This control will only take effect if ExposureTimeMode is Disabled. Its > + presence in a request acts as a trigger to switch to the internal > + manual mode within ExposureTimeModeDisabled. > > - \sa ExposureTime AeEnable > + When reported in metadata, this control indicates what exposure time > + was used for the current request, regardless of ExposureTimeMode. > + ExposureTimeMode will indicate the source of the exposure time value, > + whether it came from the AE algorithm or not. > + > + \sa ExposureTime > + \sa AnalogueGainMode > + > + - AnalogueGainMode: > + type: int32_t > + description: | > + Control to set and report the source of the analogue gain that will be > + applied. > > - \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. > + \sa AnalogueGain > + enum: > + - name: AnalogueGainModeAuto > + value: 0 > + description: | > + The analogue gain will be calculated automatically and set by the > + AE algorithm. > + - name: AnalogueGainModeDisabled > + value: 1 > + description: | > + The analogue gain will not be updated by the AE algorithm. It will > + come from the last calculated value when the mode was Auto, or from > + the value specified in AnalogueGain. > + > + This Disabled state has two internal states; locked and manual. > + When the Disabled state is first entered, the internal state will > + be locked, and the latest analogue gain value set by the AE > + algorithm will be used (or the default AnalogueGain value, if the > + mode started out as Disabled). When an AnalogueGain is provided, > + then the internal state will go to manual, and the provided value > + will be used for the analogue gain. Going from manual to locked is > + not possible. If an AnalogueGain value is provided in the same > + request as going into the Disabled state, then the internal locked > + state will be skipped, and the internal state will start out as > + manual. > > - Brightness: > type: float > @@ -477,36 +592,6 @@ controls: > High quality aberration correction which might reduce the frame > rate. > > - - AeState: > - type: int32_t > - draft: true > - description: | > - Control to report the current AE algorithm state. Currently identical to > - ANDROID_CONTROL_AE_STATE. > - > - Current state of the AE algorithm. > - enum: > - - name: AeStateInactive > - value: 0 > - description: The AE algorithm is inactive. > - - name: AeStateSearching > - value: 1 > - description: The AE algorithm has not converged yet. > - - name: AeStateConverged > - value: 2 > - description: The AE algorithm has converged. > - - name: AeStateLocked > - value: 3 > - description: The AE algorithm is locked. > - - name: AeStateFlashRequired > - value: 4 > - description: The AE algorithm would need a flash for good results > - - name: AeStatePrecapture > - value: 5 > - description: | > - The AE algorithm has started a pre-capture metering session. > - \sa AePrecaptureTrigger > - > - AfState: > type: int32_t > draft: true > -- > 2.27.0 >
Hi Paul, On Fri, Oct 01, 2021 at 07:33:19PM +0900, Paul Elder wrote: > We have multiple goals: > - we need a lock of some sort, to instruct the AEGC to not update output > results > - we need manual modes, to override the values computed by the AEGC > - we need to support seamless transitions from auto -> manual, and do so > without flickering > - we need custom minimum values for the manual controls, that is no > magic values for enabling/disabling auto > - all of these need to be done with AE sub-controls (exposure time, > analogue gain) > > To achieve these goals, we introduce mode controls for the AE > sub-controls: ExposureTimeMode and AnalogueGainMode. These have an auto > state, and a disabled state. The disabled state has an internal one-way > state change from locked to manual, triggered by the presence of the > value-controls (ExposureTime and AnalogueGain). > > We then remove the AeEnable control, as it is a redundant control in the > face of these two mode controls. > > We also remove AeLocked, as it is insufficient for reporting the AE > state, and we promote AeState to non-draft to fill its role. Notably, > the locked state is removed, since this information can be obtained from > the aforementioned mode controls. > > Bug: https://bugs.libcamera.org/show_bug.cgi?id=42 > Bug: https://bugs.libcamera.org/show_bug.cgi?id=43 > Bug: https://bugs.libcamera.org/show_bug.cgi?id=47 > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > > --- > Changes in v2: > - No changes, just resubmitting at the head of this series so that it's > together and so that /people will actually see it/ > > Initial version: > Still RFC as I haven't updated the users of the control yet, and I want > to check that these are the controls and docs that we want. > > We've decided that the "master AE control" will be implemented by a > helper... but looking at uvcvideo and the V4L2 controls I'm wondering if > such helper should come earlier than later? > --- > src/libcamera/control_ids.yaml | 215 +++++++++++++++++++++++---------- > 1 file changed, 150 insertions(+), 65 deletions(-) > > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml > index 9d4638ae..18b186e8 100644 > --- a/src/libcamera/control_ids.yaml > +++ b/src/libcamera/control_ids.yaml > @@ -7,23 +7,61 @@ > # Unless otherwise stated, all controls are bi-directional, i.e. they can be > # set through Request::controls() and returned out through Request::metadata(). > controls: > - - AeEnable: > - type: bool > - description: | > - Enable or disable the AE. > - > - \sa ExposureTime AnalogueGain > - > - - AeLocked: > - type: bool > + - AeState: Now that we have two separate controls for the exposure time and analogue gain, do we want a single state for the whole AEGC block ? > + type: int32_t > description: | > - 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. > + Control to report the current AE algorithm state. Enabling or disabling > + any of the AE-related mode controls (eg. AnalogueGainMode, > + ExposureTimeMode) is not required to reset the state to > + AeStateInactive. The camera device can do several state transitions I understand that, from where we come from the "Enabling or disabling any of the AE-related mode controls (eg. AnalogueGainMode, ExposureTimeMode) is not required to reset the state to AeStateInactive." makes sense, but you're reading this for the first time you would be wondering why is this information here, even before what the inactive state is has been defined. If you consider this relevant, I would place this in the Inactive state description itself > + between two results, if it is allowed by the state transition table. which table ? :) > + For example, AeStateInactive may never actually be seen in a result. I would place this information in the transition table if we have one > + > + The state in the result is the state for this image (in sync with this The () part is a repetition, and doesn't this apply to all controls part of a result ? Don't they all apply to the frames part of the request they are part of ? (Also I would refer to requests, not images) > + image). If AE state becomes AeStateConverged, then the image data > + associated with the result should be good to use. > + > + As some AE algorithms may still be running when any of the AE-related > + controls are in manual mode, the states are valid even when the mode What is manual mode ? Don't we have an "enabled"/"disabled" state only ? I would just say that the state is reported even if the exposure time and the analogue gain algorithms are disabled > + controls are set to Disabled. To know if this image image used AE or > + manual (or a mixture), check the relevant modes (eg. AnalogueGainMode, > + ExposureTimeMode). > + > + \sa AnalogueGain > + \sa AnalogueGainMode > + \sa ExposureTime > + \sa ExposureTimeMode > > - \sa AeEnable > + enum: > + - name: AeStateInactive > + value: 0 > + description: | > + The AE algorithm is inactive. > + If the camera initiates an AE scan, the state shall go to Searching. > + - name: AeStateSearching > + value: 1 > + description: | > + The AE algorithm has not converged yet. > + If the camera finishes an AE scan, the state shall go to Converged. > + If the camera finishes an AE scan, but flash is required, the state > + shall go to FlashRequired. > + - name: AeStateConverged > + value: 2 > + description: | > + The AE algorithm has converged. > + If the camera initiates an AE scan, the state shall go to Searching. > + - name: AeStateFlashRequired > + value: 3 > + description: | > + The AE algorithm would need a flash for good results > + If the camera initiates an AE scan, the state shall go to Searching. > + If AeLock is on, the state shall go to Locked. > + - name: AeStatePrecapture > + value: 4 > + description: | > + The AE algorithm has started a pre-capture metering session. > + After the sequence is finished, the state shall go to Converged if > + \sa AePrecaptureTrigger We don't have AE capture triggers, nor a transition table defined at all. And as said above, now we have two controls for exposure and analog gain, and a single state for the whole AE. Do we want AeState at all ? > > # AeMeteringMode needs further attention: > # - Auto-generate max enum value. > @@ -93,6 +131,17 @@ controls: > how the desired total exposure is divided between the shutter time > and the sensor's analogue gain. The exposure modes are platform > specific, and not all exposure modes may be supported. > + > + When one of AnalogueGainMode or ExposureTimeMode is set to Disabled and > + a corresponding manual control is provided, AeExposureMode may be > + ignored. This is because, for example, if AeExposureMode is set to > + ExposureLong but ExposureTimeMode is Disabled and a short ExposureTime > + is provided (AnalogueGainMode set to Auto), then the AeExposureMode > + doesn't make sense. Why not just: This controls takes effect only if ExposureTimeMode is set to ExposureTimeModeAuto and it's ignored otherwise > + > + \sa AnalogueGainMode > + \sa ExposureTimeMode > + > enum: > - name: ExposureNormal > value: 0 > @@ -111,13 +160,15 @@ controls: > type: float > description: | > Specify an Exposure Value (EV) parameter. The EV parameter will only be > - applied if the AE algorithm is currently enabled. > + applied if the AE algorithm is currently enabled, that is, at least one > + of AnalogueGainMode and ExposureTimeMode are auto. Mostly a question for RPi who upstreamed this, but I don't fully get how ExposureValue works. Seems like it is used as a global multiplier for the analogue gain in AGC::computeGain. Is it worth reworking it or is it just me that I'm missing some part ? > > By convention EV adjusts the exposure as log2. For example > EV = [-2, -1, 0.5, 0, 0.5, 1, 2] results in an exposure adjustment > of [1/4x, 1/2x, 1/sqrt(2)x, 1x, sqrt(2)x, 2x, 4x]. > > - \sa AeEnable > + \sa AnalogueGainMode > + \sa ExposureTimeMode > > - ExposureTime: > type: int32_t > @@ -125,17 +176,49 @@ controls: > Exposure time (shutter speed) for the frame applied in the sensor > device. This value is specified in micro-seconds. > > - 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. > + This control will only take effect if ExposureTimeMode is Disabled. Its > + presence in a request acts as a trigger to switch to the internal > + manual mode within ExposureTimeModeDisabled. I find the concept of "internal manual mode" confusing, and I think we should not insert it in the documentation. This control will only take effect if ExposureTimeMode is Disabled. ^ take or takes ? and is ignored when ExposureTimeMode is set to Auto. > + When reported in metadata, this control indicates what exposure time > + was used for the current request, regardless of ExposureTimeMode. > + ExposureTimeMode will indicate the source of the exposure time value, > + whether it came from the AE algorithm or not. > > - \sa AnalogueGain AeEnable > + \sa AnalogueGain > + \sa ExposureTimeMode > > - \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. > + - ExposureTimeMode: So, I don't want to be too picky, but does it make sense to say a "Mode" is either "Auto" or "Disabled" ? Aren't "AutoExposure = {Enable, Disabled}" or "ExposureMode = {Auto, Manual}" better ? I'm asking without having a real answer to the question. > + type: int32_t > + description: | > + Control to set and report the source of the exposure time that will be > + applied. Controls how the frame exposure time is computed. When set to "Auto" the auto-exposure algorithm computes the exposure time of the next frame and configures the image sensor accordingly. When set to "Disabled" the auto-exposure algorithm stops computing the exposure time. > + \sa ExposureTime > + enum: > + - name: ExposureTimeModeAuto > + value: 0 > + description: | > + The exposure time will be calculated automatically and set by the > + AE algorithm. > + - name: ExposureTimeModeDisabled > + value: 1 > + description: | > + The exposure time will not be updated by the AE algorithm. It will > + come from the last calculated value when the mode was Auto, or from > + the value specified in ExposureTime. > + > + This Disabled state has two internal states; locked and manual. > + When the Disabled state is first entered, the internal state will > + be locked, and the latest exposure time value set by the AE > + algorithm will be used (or the default ExposureTime value, if the > + mode started out as Disabled). When an ExposureTime is provided, > + then the internal state will go to manual, and the provided value > + will be used for the exposure time. Going from manual to locked is > + not possible. If an ExposureTime value is provided in the same > + request as going into the Disabled state, then the internal locked > + state will be skipped, and the internal state will start out as > + manual. Is it useful to define such internal states ? Isn't the same expressed with ? When transitioning from Auto to Disabled mode the last computed exposure value is used until a new value is specified through the ExposureTime control. If an ExposureTime value is specified in the same request where the ExposureTimeMode is set to Disabled, the provided ExposureTime is applied immediately. I also think we need to describe how to perform a 'flickerless' transition, but that depends on what we decide to do with AeState. If I'm not mistaken we can deduce that the exposure value is 'locked' by inspecting the ExposureTimeMode value in the result. We can have a dedicated section in the control description. I'll make an attempt but I'm sure this can be vastly improved. - Flicker-less manual exposure time transition: Applications that transition from ExposureTimeMode Auto mode to the direct control of the exposure time through the ExposureTime control should aim to do so by selecting an ExposureTime value as close as possible to the last value computed by the auto exposure algorithm to avoid any visible flickering. In order to select the correct value to use in the first Request with the ExposureTime control specified, applications should accommodate the natural delay in applying controls caused by the capture pipeline frame depth. When switching to manual exposure time control, applications should not immediately specify an ExposureTime value in the same request where ExposureTimeMode is set to Disabled. They should instead wait for the first Request where ExposureTimeMode is returned as Disabled in the Request metadata and use the there returned exposure time to populate the ExposureTime value in the next queued Request. The implementation of the auto-exposure time algorithm should equally try to minimize flickering and when transitioning from manual exposure time control to auto exposure, the last used value should be used as starting point. I'm not sure about the last paragraph though, does it match the outcome of our last discussion ? > > - AnalogueGain: > type: float > @@ -144,17 +227,49 @@ controls: > 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. > + This control will only take effect if ExposureTimeMode is Disabled. Its > + presence in a request acts as a trigger to switch to the internal > + manual mode within ExposureTimeModeDisabled. s/ExposureTimeMode/AnalogueGainMode ? > > - \sa ExposureTime AeEnable > + When reported in metadata, this control indicates what exposure time > + was used for the current request, regardless of ExposureTimeMode. > + ExposureTimeMode will indicate the source of the exposure time value, > + whether it came from the AE algorithm or not. Again, all the 'exposure' should be 'analogue gain' instead ? > + > + \sa ExposureTime > + \sa AnalogueGainMode > + > + - AnalogueGainMode: > + type: int32_t > + description: | > + Control to set and report the source of the analogue gain that will be > + applied. > > - \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. > + \sa AnalogueGain > + enum: > + - name: AnalogueGainModeAuto > + value: 0 > + description: | > + The analogue gain will be calculated automatically and set by the > + AE algorithm. > + - name: AnalogueGainModeDisabled > + value: 1 > + description: | > + The analogue gain will not be updated by the AE algorithm. It will > + come from the last calculated value when the mode was Auto, or from > + the value specified in AnalogueGain. > + > + This Disabled state has two internal states; locked and manual. > + When the Disabled state is first entered, the internal state will > + be locked, and the latest analogue gain value set by the AE > + algorithm will be used (or the default AnalogueGain value, if the > + mode started out as Disabled). When an AnalogueGain is provided, > + then the internal state will go to manual, and the provided value > + will be used for the analogue gain. Going from manual to locked is > + not possible. If an AnalogueGain value is provided in the same > + request as going into the Disabled state, then the internal locked > + state will be skipped, and the internal state will start out as > + manual. Same comments as per the ExposureTimeMode control. > > - Brightness: > type: float > @@ -477,36 +592,6 @@ controls: > High quality aberration correction which might reduce the frame > rate. > > - - AeState: > - type: int32_t > - draft: true > - description: | > - Control to report the current AE algorithm state. Currently identical to > - ANDROID_CONTROL_AE_STATE. > - > - Current state of the AE algorithm. > - enum: > - - name: AeStateInactive > - value: 0 > - description: The AE algorithm is inactive. > - - name: AeStateSearching > - value: 1 > - description: The AE algorithm has not converged yet. > - - name: AeStateConverged > - value: 2 > - description: The AE algorithm has converged. > - - name: AeStateLocked > - value: 3 > - description: The AE algorithm is locked. > - - name: AeStateFlashRequired > - value: 4 > - description: The AE algorithm would need a flash for good results > - - name: AeStatePrecapture > - value: 5 > - description: | > - The AE algorithm has started a pre-capture metering session. > - \sa AePrecaptureTrigger > - > - AfState: > type: int32_t > draft: true > -- > 2.27.0 >
Hi everyone Just to answer Jacopo's question that he asked us... On Fri, 1 Oct 2021 at 17:07, Jacopo Mondi <jacopo@jmondi.org> wrote: > > Hi Paul, > > On Fri, Oct 01, 2021 at 07:33:19PM +0900, Paul Elder wrote: > > We have multiple goals: > > - we need a lock of some sort, to instruct the AEGC to not update output > > results > > - we need manual modes, to override the values computed by the AEGC > > - we need to support seamless transitions from auto -> manual, and do so > > without flickering > > - we need custom minimum values for the manual controls, that is no > > magic values for enabling/disabling auto > > - all of these need to be done with AE sub-controls (exposure time, > > analogue gain) > > > > To achieve these goals, we introduce mode controls for the AE > > sub-controls: ExposureTimeMode and AnalogueGainMode. These have an auto > > state, and a disabled state. The disabled state has an internal one-way > > state change from locked to manual, triggered by the presence of the > > value-controls (ExposureTime and AnalogueGain). > > > > We then remove the AeEnable control, as it is a redundant control in the > > face of these two mode controls. > > > > We also remove AeLocked, as it is insufficient for reporting the AE > > state, and we promote AeState to non-draft to fill its role. Notably, > > the locked state is removed, since this information can be obtained from > > the aforementioned mode controls. > > > > Bug: https://bugs.libcamera.org/show_bug.cgi?id=42 > > Bug: https://bugs.libcamera.org/show_bug.cgi?id=43 > > Bug: https://bugs.libcamera.org/show_bug.cgi?id=47 > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > > > > --- > > Changes in v2: > > - No changes, just resubmitting at the head of this series so that it's > > together and so that /people will actually see it/ > > > > Initial version: > > Still RFC as I haven't updated the users of the control yet, and I want > > to check that these are the controls and docs that we want. > > > > We've decided that the "master AE control" will be implemented by a > > helper... but looking at uvcvideo and the V4L2 controls I'm wondering if > > such helper should come earlier than later? > > --- > > src/libcamera/control_ids.yaml | 215 +++++++++++++++++++++++---------- > > 1 file changed, 150 insertions(+), 65 deletions(-) > > > > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml > > index 9d4638ae..18b186e8 100644 > > --- a/src/libcamera/control_ids.yaml > > +++ b/src/libcamera/control_ids.yaml > > @@ -7,23 +7,61 @@ > > # Unless otherwise stated, all controls are bi-directional, i.e. they can be > > # set through Request::controls() and returned out through Request::metadata(). > > controls: > > - - AeEnable: > > - type: bool > > - description: | > > - Enable or disable the AE. > > - > > - \sa ExposureTime AnalogueGain > > - > > - - AeLocked: > > - type: bool > > + - AeState: > > Now that we have two separate controls for the exposure time and > analogue gain, do we want a single state for the whole AEGC block ? > > > + type: int32_t > > description: | > > - 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. > > + Control to report the current AE algorithm state. Enabling or disabling > > + any of the AE-related mode controls (eg. AnalogueGainMode, > > + ExposureTimeMode) is not required to reset the state to > > + AeStateInactive. The camera device can do several state transitions > > I understand that, from where we come from the > > "Enabling or disabling any of the AE-related mode controls (eg. AnalogueGainMode, > ExposureTimeMode) is not required to reset the state to > AeStateInactive." > > makes sense, but you're reading this for the first time you would be > wondering why is this information here, even before what the inactive > state is has been defined. If you consider this relevant, I would > place this in the Inactive state description itself > > > + between two results, if it is allowed by the state transition table. > > which table ? :) > > > + For example, AeStateInactive may never actually be seen in a result. > > I would place this information in the transition table if we have one > > > + > > + The state in the result is the state for this image (in sync with this > > The () part is a repetition, and doesn't this apply to all controls > part of a result ? Don't they all apply to the frames part of the > request they are part of ? (Also I would refer to requests, not > images) > > > + image). If AE state becomes AeStateConverged, then the image data > > + associated with the result should be good to use. > > + > > + As some AE algorithms may still be running when any of the AE-related > > + controls are in manual mode, the states are valid even when the mode > > What is manual mode ? Don't we have an "enabled"/"disabled" state only > ? > > I would just say that the state is reported even if the exposure time > and the analogue gain algorithms are disabled > > > + controls are set to Disabled. To know if this image image used AE or > > + manual (or a mixture), check the relevant modes (eg. AnalogueGainMode, > > + ExposureTimeMode). > > + > > + \sa AnalogueGain > > + \sa AnalogueGainMode > > + \sa ExposureTime > > + \sa ExposureTimeMode > > > > - \sa AeEnable > > + enum: > > + - name: AeStateInactive > > + value: 0 > > + description: | > > + The AE algorithm is inactive. > > + If the camera initiates an AE scan, the state shall go to Searching. > > + - name: AeStateSearching > > + value: 1 > > + description: | > > + The AE algorithm has not converged yet. > > + If the camera finishes an AE scan, the state shall go to Converged. > > + If the camera finishes an AE scan, but flash is required, the state > > + shall go to FlashRequired. > > + - name: AeStateConverged > > + value: 2 > > + description: | > > + The AE algorithm has converged. > > + If the camera initiates an AE scan, the state shall go to Searching. > > + - name: AeStateFlashRequired > > + value: 3 > > + description: | > > + The AE algorithm would need a flash for good results > > + If the camera initiates an AE scan, the state shall go to Searching. > > + If AeLock is on, the state shall go to Locked. > > + - name: AeStatePrecapture > > + value: 4 > > + description: | > > + The AE algorithm has started a pre-capture metering session. > > + After the sequence is finished, the state shall go to Converged if > > + \sa AePrecaptureTrigger > > We don't have AE capture triggers, nor a transition table defined at > all. And as said above, now we have two controls for exposure and > analog gain, and a single state for the whole AE. > > Do we want AeState at all ? > > > > > # AeMeteringMode needs further attention: > > # - Auto-generate max enum value. > > @@ -93,6 +131,17 @@ controls: > > how the desired total exposure is divided between the shutter time > > and the sensor's analogue gain. The exposure modes are platform > > specific, and not all exposure modes may be supported. > > + > > + When one of AnalogueGainMode or ExposureTimeMode is set to Disabled and > > + a corresponding manual control is provided, AeExposureMode may be > > + ignored. This is because, for example, if AeExposureMode is set to > > + ExposureLong but ExposureTimeMode is Disabled and a short ExposureTime > > + is provided (AnalogueGainMode set to Auto), then the AeExposureMode > > + doesn't make sense. > > Why not just: > > This controls takes effect only if ExposureTimeMode is set > to ExposureTimeModeAuto and it's ignored otherwise > > > + > > + \sa AnalogueGainMode > > + \sa ExposureTimeMode > > + > > enum: > > - name: ExposureNormal > > value: 0 > > @@ -111,13 +160,15 @@ controls: > > type: float > > description: | > > Specify an Exposure Value (EV) parameter. The EV parameter will only be > > - applied if the AE algorithm is currently enabled. > > + applied if the AE algorithm is currently enabled, that is, at least one > > + of AnalogueGainMode and ExposureTimeMode are auto. > > Mostly a question for RPi who upstreamed this, but > I don't fully get how ExposureValue works. Seems like it is used as a > global multiplier for the analogue gain in AGC::computeGain. Is it > worth reworking it or is it just me that I'm missing some part ? It's used as a global multiplier for the AGC's _target_ values, at least in our implementation. For example, generally a 2x increase in target values will give a 2x increase in combined exposure and analogue gain, though exactly how much each increases by is determined by the exposure profile. If any levers are fixed, then any such increase has to go into the ones that aren't fixed. Clearly if all are fixed, then changing the ev can have no effect at all. Does that all make sense? I think Paul's text seems accurate to me. Thanks! David > > > > > By convention EV adjusts the exposure as log2. For example > > EV = [-2, -1, 0.5, 0, 0.5, 1, 2] results in an exposure adjustment > > of [1/4x, 1/2x, 1/sqrt(2)x, 1x, sqrt(2)x, 2x, 4x]. > > > > - \sa AeEnable > > + \sa AnalogueGainMode > > + \sa ExposureTimeMode > > > > - ExposureTime: > > type: int32_t > > @@ -125,17 +176,49 @@ controls: > > Exposure time (shutter speed) for the frame applied in the sensor > > device. This value is specified in micro-seconds. > > > > - 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. > > + This control will only take effect if ExposureTimeMode is Disabled. Its > > + presence in a request acts as a trigger to switch to the internal > > + manual mode within ExposureTimeModeDisabled. > > I find the concept of "internal manual mode" confusing, and I think we > should not insert it in the documentation. > > This control will only take effect if ExposureTimeMode is Disabled. > > ^ take or takes ? > > and is ignored when ExposureTimeMode is set to Auto. > > > > + When reported in metadata, this control indicates what exposure time > > + was used for the current request, regardless of ExposureTimeMode. > > + ExposureTimeMode will indicate the source of the exposure time value, > > + whether it came from the AE algorithm or not. > > > > - \sa AnalogueGain AeEnable > > + \sa AnalogueGain > > + \sa ExposureTimeMode > > > > - \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. > > + - ExposureTimeMode: > > So, I don't want to be too picky, but does it make sense to say a > "Mode" is either "Auto" or "Disabled" ? Aren't "AutoExposure = {Enable, > Disabled}" or "ExposureMode = {Auto, Manual}" better ? I'm asking > without having a real answer to the question. > > > + type: int32_t > > + description: | > > + Control to set and report the source of the exposure time that will be > > + applied. > > Controls how the frame exposure time is computed. When set > to "Auto" the auto-exposure algorithm computes the exposure > time of the next frame and configures the image sensor > accordingly. When set to "Disabled" the auto-exposure > algorithm stops computing the exposure time. > > > + \sa ExposureTime > > + enum: > > + - name: ExposureTimeModeAuto > > + value: 0 > > + description: | > > + The exposure time will be calculated automatically and set by the > > + AE algorithm. > > + - name: ExposureTimeModeDisabled > > + value: 1 > > + description: | > > + The exposure time will not be updated by the AE algorithm. It will > > + come from the last calculated value when the mode was Auto, or from > > + the value specified in ExposureTime. > > + > > + This Disabled state has two internal states; locked and manual. > > + When the Disabled state is first entered, the internal state will > > + be locked, and the latest exposure time value set by the AE > > + algorithm will be used (or the default ExposureTime value, if the > > + mode started out as Disabled). When an ExposureTime is provided, > > + then the internal state will go to manual, and the provided value > > + will be used for the exposure time. Going from manual to locked is > > + not possible. If an ExposureTime value is provided in the same > > + request as going into the Disabled state, then the internal locked > > + state will be skipped, and the internal state will start out as > > + manual. > > Is it useful to define such internal states ? Isn't the same expressed > with ? > > When transitioning from Auto to Disabled mode the last > computed exposure value is used until a new value is > specified through the ExposureTime control. If an > ExposureTime value is specified in the same request where > the ExposureTimeMode is set to Disabled, the provided > ExposureTime is applied immediately. > > I also think we need to describe how to perform a 'flickerless' > transition, but that depends on what we decide to do with AeState. If > I'm not mistaken we can deduce that the exposure value is 'locked' by > inspecting the ExposureTimeMode value in the result. > > We can have a dedicated section in the control description. I'll make > an attempt but I'm sure this can be vastly improved. > > - Flicker-less manual exposure time transition: > > Applications that transition from ExposureTimeMode > Auto mode to the direct control of the exposure time through > the ExposureTime control should aim to do so by selecting > an ExposureTime value as close as possible to the last > value computed by the auto exposure algorithm to avoid > any visible flickering. > > In order to select the correct value to use in the first > Request with the ExposureTime control specified, > applications should accommodate the natural delay in > applying controls caused by the capture pipeline frame > depth. > > When switching to manual exposure time control, > applications should not immediately specify an > ExposureTime value in the same request where > ExposureTimeMode is set to Disabled. They should instead > wait for the first Request where ExposureTimeMode is > returned as Disabled in the Request metadata and use the > there returned exposure time to populate the ExposureTime > value in the next queued Request. > > The implementation of the auto-exposure time algorithm > should equally try to minimize flickering and when > transitioning from manual exposure time control to auto > exposure, the last used value should be used as starting > point. > > I'm not sure about the last paragraph though, does it match the > outcome of our last discussion ? > > > > > - AnalogueGain: > > type: float > > @@ -144,17 +227,49 @@ controls: > > 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. > > + This control will only take effect if ExposureTimeMode is Disabled. Its > > + presence in a request acts as a trigger to switch to the internal > > + manual mode within ExposureTimeModeDisabled. > > s/ExposureTimeMode/AnalogueGainMode ? > > > > - \sa ExposureTime AeEnable > > + When reported in metadata, this control indicates what exposure time > > + was used for the current request, regardless of ExposureTimeMode. > > + ExposureTimeMode will indicate the source of the exposure time value, > > + whether it came from the AE algorithm or not. > > Again, all the 'exposure' should be 'analogue gain' instead ? > > > + > > + \sa ExposureTime > > + \sa AnalogueGainMode > > + > > + - AnalogueGainMode: > > + type: int32_t > > + description: | > > + Control to set and report the source of the analogue gain that will be > > + applied. > > > > - \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. > > + \sa AnalogueGain > > + enum: > > + - name: AnalogueGainModeAuto > > + value: 0 > > + description: | > > + The analogue gain will be calculated automatically and set by the > > + AE algorithm. > > + - name: AnalogueGainModeDisabled > > + value: 1 > > + description: | > > + The analogue gain will not be updated by the AE algorithm. It will > > + come from the last calculated value when the mode was Auto, or from > > + the value specified in AnalogueGain. > > + > > + This Disabled state has two internal states; locked and manual. > > + When the Disabled state is first entered, the internal state will > > + be locked, and the latest analogue gain value set by the AE > > + algorithm will be used (or the default AnalogueGain value, if the > > + mode started out as Disabled). When an AnalogueGain is provided, > > + then the internal state will go to manual, and the provided value > > + will be used for the analogue gain. Going from manual to locked is > > + not possible. If an AnalogueGain value is provided in the same > > + request as going into the Disabled state, then the internal locked > > + state will be skipped, and the internal state will start out as > > + manual. > > Same comments as per the ExposureTimeMode control. > > > > > - Brightness: > > type: float > > @@ -477,36 +592,6 @@ controls: > > High quality aberration correction which might reduce the frame > > rate. > > > > - - AeState: > > - type: int32_t > > - draft: true > > - description: | > > - Control to report the current AE algorithm state. Currently identical to > > - ANDROID_CONTROL_AE_STATE. > > - > > - Current state of the AE algorithm. > > - enum: > > - - name: AeStateInactive > > - value: 0 > > - description: The AE algorithm is inactive. > > - - name: AeStateSearching > > - value: 1 > > - description: The AE algorithm has not converged yet. > > - - name: AeStateConverged > > - value: 2 > > - description: The AE algorithm has converged. > > - - name: AeStateLocked > > - value: 3 > > - description: The AE algorithm is locked. > > - - name: AeStateFlashRequired > > - value: 4 > > - description: The AE algorithm would need a flash for good results > > - - name: AeStatePrecapture > > - value: 5 > > - description: | > > - The AE algorithm has started a pre-capture metering session. > > - \sa AePrecaptureTrigger > > - > > - AfState: > > type: int32_t > > draft: true > > -- > > 2.27.0 > >
Hi David, Thanks for the feedback. On Fri, Oct 01, 2021 at 01:53:18PM +0100, David Plowman wrote: > Hi Paul > > Thanks for all this work! I was fine with all the functionality being > described, and mostly everything made sense to me, but there were just > a couple of places where I thought we might be able to explain things > more clearly. But this is just my opinion, others may disagree, so > please take with the necessary quantities of salt! > > On Fri, 1 Oct 2021 at 11:33, Paul Elder <paul.elder@ideasonboard.com> wrote: > > > > We have multiple goals: > > - we need a lock of some sort, to instruct the AEGC to not update output > > results > > - we need manual modes, to override the values computed by the AEGC > > - we need to support seamless transitions from auto -> manual, and do so > > without flickering > > - we need custom minimum values for the manual controls, that is no > > magic values for enabling/disabling auto > > - all of these need to be done with AE sub-controls (exposure time, > > analogue gain) > > > > To achieve these goals, we introduce mode controls for the AE > > sub-controls: ExposureTimeMode and AnalogueGainMode. These have an auto > > state, and a disabled state. The disabled state has an internal one-way > > state change from locked to manual, triggered by the presence of the > > value-controls (ExposureTime and AnalogueGain). > > > > We then remove the AeEnable control, as it is a redundant control in the > > face of these two mode controls. > > > > We also remove AeLocked, as it is insufficient for reporting the AE > > state, and we promote AeState to non-draft to fill its role. Notably, > > the locked state is removed, since this information can be obtained from > > the aforementioned mode controls. > > > > Bug: https://bugs.libcamera.org/show_bug.cgi?id=42 > > Bug: https://bugs.libcamera.org/show_bug.cgi?id=43 > > Bug: https://bugs.libcamera.org/show_bug.cgi?id=47 > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > > > > --- > > Changes in v2: > > - No changes, just resubmitting at the head of this series so that it's > > together and so that /people will actually see it/ > > > > Initial version: > > Still RFC as I haven't updated the users of the control yet, and I want > > to check that these are the controls and docs that we want. > > > > We've decided that the "master AE control" will be implemented by a > > helper... but looking at uvcvideo and the V4L2 controls I'm wondering if > > such helper should come earlier than later? > > --- > > src/libcamera/control_ids.yaml | 215 +++++++++++++++++++++++---------- > > 1 file changed, 150 insertions(+), 65 deletions(-) > > > > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml > > index 9d4638ae..18b186e8 100644 > > --- a/src/libcamera/control_ids.yaml > > +++ b/src/libcamera/control_ids.yaml > > @@ -7,23 +7,61 @@ > > # Unless otherwise stated, all controls are bi-directional, i.e. they can be > > # set through Request::controls() and returned out through Request::metadata(). > > controls: > > - - AeEnable: > > - type: bool > > - description: | > > - Enable or disable the AE. > > - > > - \sa ExposureTime AnalogueGain > > - > > - - AeLocked: > > - type: bool > > + - AeState: > > + type: int32_t > > description: | > > - 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. > > + Control to report the current AE algorithm state. Enabling or disabling > > + any of the AE-related mode controls (eg. AnalogueGainMode, > > + ExposureTimeMode) is not required to reset the state to > > + AeStateInactive. The camera device can do several state transitions > > + between two results, if it is allowed by the state transition table. > > + For example, AeStateInactive may never actually be seen in a result. > > I was fine until we got to that bit "Enabling or disabling". The rest > of this paragraph struck me as a real burst of detail and corner cases > (?) while I was still struggling with what AeStateInactive meant. Ah, that was because the android description said "enabling or disabling AE must reset the state to inactive" or something along those lines and I just substituted that whole line with "no". > > Perhaps some of this specific stuff can be moved until after all the > enum values have been explained? I found the enum descriptions quite Not sure I can move it after the enums... but maybe at the end of the overview at lesat? > easy to follow and having read through those felt in a better place to That's good! > tackle the details. Having said that, I'm still a bit vague as to why > "resetting the state to AeStateInactive" is a particular issue of > concern? > > > + > > + The state in the result is the state for this image (in sync with this > > + image). If AE state becomes AeStateConverged, then the image data > > + associated with the result should be good to use. > > + > > + As some AE algorithms may still be running when any of the AE-related > > + controls are in manual mode, the states are valid even when the mode > > + controls are set to Disabled. To know if this image image used AE or > > "image" is repeated. > > Do we need to say anything about what happens when all controls are > disabled? (Does anything different happen??) Maybe I should've said "if at least one is disabled". > > > + manual (or a mixture), check the relevant modes (eg. AnalogueGainMode, > > + ExposureTimeMode). > > + > > + \sa AnalogueGain > > + \sa AnalogueGainMode > > + \sa ExposureTime > > + \sa ExposureTimeMode > > > > - \sa AeEnable > > + enum: > > + - name: AeStateInactive > > + value: 0 > > + description: | > > + The AE algorithm is inactive. > > + If the camera initiates an AE scan, the state shall go to Searching. > > + - name: AeStateSearching > > + value: 1 > > + description: | > > + The AE algorithm has not converged yet. > > + If the camera finishes an AE scan, the state shall go to Converged. > > + If the camera finishes an AE scan, but flash is required, the state > > + shall go to FlashRequired. > > + - name: AeStateConverged > > + value: 2 > > + description: | > > + The AE algorithm has converged. > > + If the camera initiates an AE scan, the state shall go to Searching. > > + - name: AeStateFlashRequired > > + value: 3 > > + description: | > > + The AE algorithm would need a flash for good results > > + If the camera initiates an AE scan, the state shall go to Searching. > > + If AeLock is on, the state shall go to Locked. > > + - name: AeStatePrecapture > > + value: 4 > > + description: | > > + The AE algorithm has started a pre-capture metering session. > > + After the sequence is finished, the state shall go to Converged if > > This was all good, my only minor comment might be that using > AeStateConverged and AeStateSearching in place of just > Converged/Searching might be a little more precise? Ah okay. Probably better to be explicit. > > > + \sa AePrecaptureTrigger > > > > # AeMeteringMode needs further attention: > > # - Auto-generate max enum value. > > @@ -93,6 +131,17 @@ controls: > > how the desired total exposure is divided between the shutter time > > and the sensor's analogue gain. The exposure modes are platform > > specific, and not all exposure modes may be supported. > > + > > + When one of AnalogueGainMode or ExposureTimeMode is set to Disabled and > > + a corresponding manual control is provided, AeExposureMode may be > > Actually I think the same is true even if you don't give a manual > value. As soon as you fix any of them, the other(s) will vary in such > a way as to ignore the exposure profile. Ah yes you're right. > > > + ignored. This is because, for example, if AeExposureMode is set to > > + ExposureLong but ExposureTimeMode is Disabled and a short ExposureTime > > + is provided (AnalogueGainMode set to Auto), then the AeExposureMode > > + doesn't make sense. > > The example is good, but seemed a bit wordy to me? Maybe the whole > thing might be: > > "When any of AnalogueGainMode or ExposureTimeMode is set to Disabled. > the fixed values will override any choices made by the AeExposureMode which may > consequently be ignored." That probably is sufficient. > > (Or perhaps I'm assuming a deeper understanding on the part of the > reader than most will have?) The example was me thinking aloud when coming up with the new rule :p > > > + > > + \sa AnalogueGainMode > > + \sa ExposureTimeMode > > + > > enum: > > - name: ExposureNormal > > value: 0 > > @@ -111,13 +160,15 @@ controls: > > type: float > > description: | > > Specify an Exposure Value (EV) parameter. The EV parameter will only be > > - applied if the AE algorithm is currently enabled. > > + applied if the AE algorithm is currently enabled, that is, at least one > > + of AnalogueGainMode and ExposureTimeMode are auto. > > > > By convention EV adjusts the exposure as log2. For example > > EV = [-2, -1, 0.5, 0, 0.5, 1, 2] results in an exposure adjustment > > of [1/4x, 1/2x, 1/sqrt(2)x, 1x, sqrt(2)x, 2x, 4x]. > > > > - \sa AeEnable > > + \sa AnalogueGainMode > > + \sa ExposureTimeMode > > > > - ExposureTime: > > type: int32_t > > @@ -125,17 +176,49 @@ controls: > > Exposure time (shutter speed) for the frame applied in the sensor > > device. This value is specified in micro-seconds. > > > > - 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. > > + This control will only take effect if ExposureTimeMode is Disabled. Its > > + presence in a request acts as a trigger to switch to the internal > > + manual mode within ExposureTimeModeDisabled. > > + > > + When reported in metadata, this control indicates what exposure time > > + was used for the current request, regardless of ExposureTimeMode. > > + ExposureTimeMode will indicate the source of the exposure time value, > > + whether it came from the AE algorithm or not. > > > > - \sa AnalogueGain AeEnable > > + \sa AnalogueGain > > + \sa ExposureTimeMode > > > > - \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. > > + - ExposureTimeMode: > > + type: int32_t > > + description: | > > + Control to set and report the source of the exposure time that will be > > + applied. > > + > > + \sa ExposureTime > > + enum: > > + - name: ExposureTimeModeAuto > > + value: 0 > > + description: | > > + The exposure time will be calculated automatically and set by the > > + AE algorithm. > > + - name: ExposureTimeModeDisabled > > + value: 1 > > + description: | > > + The exposure time will not be updated by the AE algorithm. It will > > + come from the last calculated value when the mode was Auto, or from > > + the value specified in ExposureTime. > > + > > + This Disabled state has two internal states; locked and manual. > > TBH I still struggle a bit with the word "locked". I think I prefer > "fixed", it seems more obvious. But not to worry... if the Android > world prefers it then I will get used to it! I'm fine with either, I've just gotten used to saying "locked" since that was the informal name of this whole ordeal :/ Thanks, Paul > > Thanks again for doing all this! > > Best regards > David > > > + When the Disabled state is first entered, the internal state will > > + be locked, and the latest exposure time value set by the AE > > + algorithm will be used (or the default ExposureTime value, if the > > + mode started out as Disabled). When an ExposureTime is provided, > > + then the internal state will go to manual, and the provided value > > + will be used for the exposure time. Going from manual to locked is > > + not possible. If an ExposureTime value is provided in the same > > + request as going into the Disabled state, then the internal locked > > + state will be skipped, and the internal state will start out as > > + manual. > > > > - AnalogueGain: > > type: float > > @@ -144,17 +227,49 @@ controls: > > 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. > > + This control will only take effect if ExposureTimeMode is Disabled. Its > > + presence in a request acts as a trigger to switch to the internal > > + manual mode within ExposureTimeModeDisabled. > > > > - \sa ExposureTime AeEnable > > + When reported in metadata, this control indicates what exposure time > > + was used for the current request, regardless of ExposureTimeMode. > > + ExposureTimeMode will indicate the source of the exposure time value, > > + whether it came from the AE algorithm or not. > > + > > + \sa ExposureTime > > + \sa AnalogueGainMode > > + > > + - AnalogueGainMode: > > + type: int32_t > > + description: | > > + Control to set and report the source of the analogue gain that will be > > + applied. > > > > - \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. > > + \sa AnalogueGain > > + enum: > > + - name: AnalogueGainModeAuto > > + value: 0 > > + description: | > > + The analogue gain will be calculated automatically and set by the > > + AE algorithm. > > + - name: AnalogueGainModeDisabled > > + value: 1 > > + description: | > > + The analogue gain will not be updated by the AE algorithm. It will > > + come from the last calculated value when the mode was Auto, or from > > + the value specified in AnalogueGain. > > + > > + This Disabled state has two internal states; locked and manual. > > + When the Disabled state is first entered, the internal state will > > + be locked, and the latest analogue gain value set by the AE > > + algorithm will be used (or the default AnalogueGain value, if the > > + mode started out as Disabled). When an AnalogueGain is provided, > > + then the internal state will go to manual, and the provided value > > + will be used for the analogue gain. Going from manual to locked is > > + not possible. If an AnalogueGain value is provided in the same > > + request as going into the Disabled state, then the internal locked > > + state will be skipped, and the internal state will start out as > > + manual. > > > > - Brightness: > > type: float > > @@ -477,36 +592,6 @@ controls: > > High quality aberration correction which might reduce the frame > > rate. > > > > - - AeState: > > - type: int32_t > > - draft: true > > - description: | > > - Control to report the current AE algorithm state. Currently identical to > > - ANDROID_CONTROL_AE_STATE. > > - > > - Current state of the AE algorithm. > > - enum: > > - - name: AeStateInactive > > - value: 0 > > - description: The AE algorithm is inactive. > > - - name: AeStateSearching > > - value: 1 > > - description: The AE algorithm has not converged yet. > > - - name: AeStateConverged > > - value: 2 > > - description: The AE algorithm has converged. > > - - name: AeStateLocked > > - value: 3 > > - description: The AE algorithm is locked. > > - - name: AeStateFlashRequired > > - value: 4 > > - description: The AE algorithm would need a flash for good results > > - - name: AeStatePrecapture > > - value: 5 > > - description: | > > - The AE algorithm has started a pre-capture metering session. > > - \sa AePrecaptureTrigger > > - > > - AfState: > > type: int32_t > > draft: true > > -- > > 2.27.0 > >
Hi Jacopo, Thanks for the feedback. On Fri, Oct 01, 2021 at 06:08:19PM +0200, Jacopo Mondi wrote: > Hi Paul, > > On Fri, Oct 01, 2021 at 07:33:19PM +0900, Paul Elder wrote: > > We have multiple goals: > > - we need a lock of some sort, to instruct the AEGC to not update output > > results > > - we need manual modes, to override the values computed by the AEGC > > - we need to support seamless transitions from auto -> manual, and do so > > without flickering > > - we need custom minimum values for the manual controls, that is no > > magic values for enabling/disabling auto > > - all of these need to be done with AE sub-controls (exposure time, > > analogue gain) > > > > To achieve these goals, we introduce mode controls for the AE > > sub-controls: ExposureTimeMode and AnalogueGainMode. These have an auto > > state, and a disabled state. The disabled state has an internal one-way > > state change from locked to manual, triggered by the presence of the > > value-controls (ExposureTime and AnalogueGain). > > > > We then remove the AeEnable control, as it is a redundant control in the > > face of these two mode controls. > > > > We also remove AeLocked, as it is insufficient for reporting the AE > > state, and we promote AeState to non-draft to fill its role. Notably, > > the locked state is removed, since this information can be obtained from > > the aforementioned mode controls. > > > > Bug: https://bugs.libcamera.org/show_bug.cgi?id=42 > > Bug: https://bugs.libcamera.org/show_bug.cgi?id=43 > > Bug: https://bugs.libcamera.org/show_bug.cgi?id=47 > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > > > > --- > > Changes in v2: > > - No changes, just resubmitting at the head of this series so that it's > > together and so that /people will actually see it/ > > > > Initial version: > > Still RFC as I haven't updated the users of the control yet, and I want > > to check that these are the controls and docs that we want. > > > > We've decided that the "master AE control" will be implemented by a > > helper... but looking at uvcvideo and the V4L2 controls I'm wondering if > > such helper should come earlier than later? > > --- > > src/libcamera/control_ids.yaml | 215 +++++++++++++++++++++++---------- > > 1 file changed, 150 insertions(+), 65 deletions(-) > > > > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml > > index 9d4638ae..18b186e8 100644 > > --- a/src/libcamera/control_ids.yaml > > +++ b/src/libcamera/control_ids.yaml > > @@ -7,23 +7,61 @@ > > # Unless otherwise stated, all controls are bi-directional, i.e. they can be > > # set through Request::controls() and returned out through Request::metadata(). > > controls: > > - - AeEnable: > > - type: bool > > - description: | > > - Enable or disable the AE. > > - > > - \sa ExposureTime AnalogueGain > > - > > - - AeLocked: > > - type: bool > > + - AeState: > > Now that we have two separate controls for the exposure time and > analogue gain, do we want a single state for the whole AEGC block ? I think so. I learned a while back that the whole AEGC block takes both E and G and outputs both E and G and that AEC and AGC are not separate blocks. Thus we have separate controls for the input and output, but a unified state for the state. > > > + type: int32_t > > description: | > > - 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. > > + Control to report the current AE algorithm state. Enabling or disabling > > + any of the AE-related mode controls (eg. AnalogueGainMode, > > + ExposureTimeMode) is not required to reset the state to > > + AeStateInactive. The camera device can do several state transitions > > I understand that, from where we come from the > > "Enabling or disabling any of the AE-related mode controls (eg. AnalogueGainMode, > ExposureTimeMode) is not required to reset the state to > AeStateInactive." > > makes sense, but you're reading this for the first time you would be > wondering why is this information here, even before what the inactive > state is has been defined. If you consider this relevant, I would > place this in the Inactive state description itself I think it's a rule for the control in general, so I'll move it to the end of the overview. > > > + between two results, if it is allowed by the state transition table. > > which table ? :) It's just a different representation of a table :p > > > + For example, AeStateInactive may never actually be seen in a result. > > I would place this information in the transition table if we have one It's a general rule about the control, so I think it belongs in the overview. It can be any state, it's just that inactive is the most trivial example. > > > + > > + The state in the result is the state for this image (in sync with this > > The () part is a repetition, and doesn't this apply to all controls > part of a result ? Don't they all apply to the frames part of the > request they are part of ? (Also I would refer to requests, not > images) Whoops, yeah it is. I just wanted to be explicit (not sure why). > > > + image). If AE state becomes AeStateConverged, then the image data > > + associated with the result should be good to use. > > + > > + As some AE algorithms may still be running when any of the AE-related > > + controls are in manual mode, the states are valid even when the mode > > What is manual mode ? Don't we have an "enabled"/"disabled" state only > ? Oh yeah s/manual/disabled/ might be better here. > > I would just say that the state is reported even if the exposure time > and the analogue gain algorithms are disabled Yeah. > > > + controls are set to Disabled. To know if this image image used AE or > > + manual (or a mixture), check the relevant modes (eg. AnalogueGainMode, > > + ExposureTimeMode). > > + > > + \sa AnalogueGain > > + \sa AnalogueGainMode > > + \sa ExposureTime > > + \sa ExposureTimeMode > > > > - \sa AeEnable > > + enum: > > + - name: AeStateInactive > > + value: 0 > > + description: | > > + The AE algorithm is inactive. > > + If the camera initiates an AE scan, the state shall go to Searching. > > + - name: AeStateSearching > > + value: 1 > > + description: | > > + The AE algorithm has not converged yet. > > + If the camera finishes an AE scan, the state shall go to Converged. > > + If the camera finishes an AE scan, but flash is required, the state > > + shall go to FlashRequired. > > + - name: AeStateConverged > > + value: 2 > > + description: | > > + The AE algorithm has converged. > > + If the camera initiates an AE scan, the state shall go to Searching. > > + - name: AeStateFlashRequired > > + value: 3 > > + description: | > > + The AE algorithm would need a flash for good results > > + If the camera initiates an AE scan, the state shall go to Searching. > > + If AeLock is on, the state shall go to Locked. > > + - name: AeStatePrecapture > > + value: 4 > > + description: | > > + The AE algorithm has started a pre-capture metering session. > > + After the sequence is finished, the state shall go to Converged if > > + \sa AePrecaptureTrigger > > We don't have AE capture triggers, nor a transition table defined at This is a table in list form :p idk how capture triggers work yet. I pulled the state in from android. > all. And as said above, now we have two controls for exposure and > analog gain, and a single state for the whole AE. > > Do we want AeState at all ? I think we do. We still need a way to report at least AE converged vs not-converged, and the state is shared between AEC and AGC as they're linked together. > > > > > # AeMeteringMode needs further attention: > > # - Auto-generate max enum value. > > @@ -93,6 +131,17 @@ controls: > > how the desired total exposure is divided between the shutter time > > and the sensor's analogue gain. The exposure modes are platform > > specific, and not all exposure modes may be supported. > > + > > + When one of AnalogueGainMode or ExposureTimeMode is set to Disabled and > > + a corresponding manual control is provided, AeExposureMode may be > > + ignored. This is because, for example, if AeExposureMode is set to > > + ExposureLong but ExposureTimeMode is Disabled and a short ExposureTime > > + is provided (AnalogueGainMode set to Auto), then the AeExposureMode > > + doesn't make sense. > > Why not just: > > This controls takes effect only if ExposureTimeMode is set > to ExposureTimeModeAuto and it's ignored otherwise AnalogueGainMode also affects this afaict. In any case, you also think that the example is unnecessary? (I suppose it was just me reasoning aloud...) > > > + > > + \sa AnalogueGainMode > > + \sa ExposureTimeMode > > + > > enum: > > - name: ExposureNormal > > value: 0 > > @@ -111,13 +160,15 @@ controls: > > type: float > > description: | > > Specify an Exposure Value (EV) parameter. The EV parameter will only be > > - applied if the AE algorithm is currently enabled. > > + applied if the AE algorithm is currently enabled, that is, at least one > > + of AnalogueGainMode and ExposureTimeMode are auto. > > Mostly a question for RPi who upstreamed this, but > I don't fully get how ExposureValue works. Seems like it is used as a > global multiplier for the analogue gain in AGC::computeGain. Is it > worth reworking it or is it just me that I'm missing some part ? > > > > > By convention EV adjusts the exposure as log2. For example > > EV = [-2, -1, 0.5, 0, 0.5, 1, 2] results in an exposure adjustment > > of [1/4x, 1/2x, 1/sqrt(2)x, 1x, sqrt(2)x, 2x, 4x]. > > > > - \sa AeEnable > > + \sa AnalogueGainMode > > + \sa ExposureTimeMode > > > > - ExposureTime: > > type: int32_t > > @@ -125,17 +176,49 @@ controls: > > Exposure time (shutter speed) for the frame applied in the sensor > > device. This value is specified in micro-seconds. > > > > - 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. > > + This control will only take effect if ExposureTimeMode is Disabled. Its > > + presence in a request acts as a trigger to switch to the internal > > + manual mode within ExposureTimeModeDisabled. > > I find the concept of "internal manual mode" confusing, and I think we > should not insert it in the documentation. We have a premise that "controls that are submitted in one request but not resubmitted in subsequent requests still have their value maintained", and we have these control values that act as triggers for locked -> manual which violates this premise. Thus we either need to document that exception (which I think mentioning the internal state is useful), or get rid of the premise and require all unchanging control values to still require resubmission. > > This control will only take effect if ExposureTimeMode is Disabled. > > ^ take or takes ? The control will -> take The control (no will) -> takes The controls will -> take The controls (no will) -> take > > and is ignored when ExposureTimeMode is set to Auto. > > > > + When reported in metadata, this control indicates what exposure time > > + was used for the current request, regardless of ExposureTimeMode. > > + ExposureTimeMode will indicate the source of the exposure time value, > > + whether it came from the AE algorithm or not. > > > > - \sa AnalogueGain AeEnable > > + \sa AnalogueGain > > + \sa ExposureTimeMode > > > > - \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. > > + - ExposureTimeMode: > > So, I don't want to be too picky, but does it make sense to say a > "Mode" is either "Auto" or "Disabled" ? Aren't "AutoExposure = {Enable, > Disabled}" or "ExposureMode = {Auto, Manual}" better ? I'm asking > without having a real answer to the question. I don't have a real answer either. I heard objections against "Mode" but nobody proposed a better alternative :D Same thing with "Disabled". It's not "manual" or "locked" or "fixed" alone, it's a superposition of the two, so we need a word that can encompass those :/ > > > + type: int32_t > > + description: | > > + Control to set and report the source of the exposure time that will be > > + applied. > > Controls how the frame exposure time is computed. When set > to "Auto" the auto-exposure algorithm computes the exposure > time of the next frame and configures the image sensor > accordingly. When set to "Disabled" the auto-exposure > algorithm stops computing the exposure time. Oh that's good. > > > + \sa ExposureTime > > + enum: > > + - name: ExposureTimeModeAuto > > + value: 0 > > + description: | > > + The exposure time will be calculated automatically and set by the > > + AE algorithm. > > + - name: ExposureTimeModeDisabled > > + value: 1 > > + description: | > > + The exposure time will not be updated by the AE algorithm. It will > > + come from the last calculated value when the mode was Auto, or from > > + the value specified in ExposureTime. > > + > > + This Disabled state has two internal states; locked and manual. > > + When the Disabled state is first entered, the internal state will > > + be locked, and the latest exposure time value set by the AE > > + algorithm will be used (or the default ExposureTime value, if the > > + mode started out as Disabled). When an ExposureTime is provided, > > + then the internal state will go to manual, and the provided value > > + will be used for the exposure time. Going from manual to locked is > > + not possible. If an ExposureTime value is provided in the same > > + request as going into the Disabled state, then the internal locked > > + state will be skipped, and the internal state will start out as > > + manual. > > Is it useful to define such internal states ? Isn't the same expressed > with ? > > When transitioning from Auto to Disabled mode the last > computed exposure value is used until a new value is > specified through the ExposureTime control. If an > ExposureTime value is specified in the same request where > the ExposureTimeMode is set to Disabled, the provided > ExposureTime is applied immediately. True, that is sufficient. I was thinking that there was some value in being explicit with states... > > I also think we need to describe how to perform a 'flickerless' Ah yeah we should. > transition, but that depends on what we decide to do with AeState. If > I'm not mistaken we can deduce that the exposure value is 'locked' by > inspecting the ExposureTimeMode value in the result. Yeah, AeState doesn't contribute to determining if AEC or AGC are locked. Those are determined by the two Mode controls. AeState is for determining if the combined AEGC has converged or not. However, the two Mode controls are not sufficient for determining if the internal AEC/AGC states are actually in the "locked" state (for the purpose of reporting Lock to android). But that's in a later patch, and we just tag that state onto the request descriptor. > > We can have a dedicated section in the control description. I'll make > an attempt but I'm sure this can be vastly improved. > > - Flicker-less manual exposure time transition: > > Applications that transition from ExposureTimeMode > Auto mode to the direct control of the exposure time through > the ExposureTime control should aim to do so by selecting > an ExposureTime value as close as possible to the last > value computed by the auto exposure algorithm to avoid > any visible flickering. > > In order to select the correct value to use in the first > Request with the ExposureTime control specified, > applications should accommodate the natural delay in > applying controls caused by the capture pipeline frame > depth. > > When switching to manual exposure time control, > applications should not immediately specify an > ExposureTime value in the same request where > ExposureTimeMode is set to Disabled. They should instead > wait for the first Request where ExposureTimeMode is > returned as Disabled in the Request metadata and use the > there returned exposure time to populate the ExposureTime > value in the next queued Request. Ooh that's good. > > The implementation of the auto-exposure time algorithm > should equally try to minimize flickering and when > transitioning from manual exposure time control to auto > exposure, the last used value should be used as starting > point. > > I'm not sure about the last paragraph though, does it match the > outcome of our last discussion ? I'm not sure I understand the purpose of the last paragraph. It seems more directed to IPA developers, that their AEGC should minimize flickering (afaict that's in their interest already), and that they need to use the last used value as a stating point when switching from manual to auto. Maybe the latter point would be good to mention here? > > > > > - AnalogueGain: > > type: float > > @@ -144,17 +227,49 @@ controls: > > 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. > > + This control will only take effect if ExposureTimeMode is Disabled. Its > > + presence in a request acts as a trigger to switch to the internal > > + manual mode within ExposureTimeModeDisabled. > > s/ExposureTimeMode/AnalogueGainMode ? > > > > - \sa ExposureTime AeEnable > > + When reported in metadata, this control indicates what exposure time > > + was used for the current request, regardless of ExposureTimeMode. > > + ExposureTimeMode will indicate the source of the exposure time value, > > + whether it came from the AE algorithm or not. > > Again, all the 'exposure' should be 'analogue gain' instead ? Oops, yeah :p I forgot to fix it in the copy-paste... Thanks, Paul > > > + > > + \sa ExposureTime > > + \sa AnalogueGainMode > > + > > + - AnalogueGainMode: > > + type: int32_t > > + description: | > > + Control to set and report the source of the analogue gain that will be > > + applied. > > > > - \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. > > + \sa AnalogueGain > > + enum: > > + - name: AnalogueGainModeAuto > > + value: 0 > > + description: | > > + The analogue gain will be calculated automatically and set by the > > + AE algorithm. > > + - name: AnalogueGainModeDisabled > > + value: 1 > > + description: | > > + The analogue gain will not be updated by the AE algorithm. It will > > + come from the last calculated value when the mode was Auto, or from > > + the value specified in AnalogueGain. > > + > > + This Disabled state has two internal states; locked and manual. > > + When the Disabled state is first entered, the internal state will > > + be locked, and the latest analogue gain value set by the AE > > + algorithm will be used (or the default AnalogueGain value, if the > > + mode started out as Disabled). When an AnalogueGain is provided, > > + then the internal state will go to manual, and the provided value > > + will be used for the analogue gain. Going from manual to locked is > > + not possible. If an AnalogueGain value is provided in the same > > + request as going into the Disabled state, then the internal locked > > + state will be skipped, and the internal state will start out as > > + manual. > > Same comments as per the ExposureTimeMode control. > > > > > - Brightness: > > type: float > > @@ -477,36 +592,6 @@ controls: > > High quality aberration correction which might reduce the frame > > rate. > > > > - - AeState: > > - type: int32_t > > - draft: true > > - description: | > > - Control to report the current AE algorithm state. Currently identical to > > - ANDROID_CONTROL_AE_STATE. > > - > > - Current state of the AE algorithm. > > - enum: > > - - name: AeStateInactive > > - value: 0 > > - description: The AE algorithm is inactive. > > - - name: AeStateSearching > > - value: 1 > > - description: The AE algorithm has not converged yet. > > - - name: AeStateConverged > > - value: 2 > > - description: The AE algorithm has converged. > > - - name: AeStateLocked > > - value: 3 > > - description: The AE algorithm is locked. > > - - name: AeStateFlashRequired > > - value: 4 > > - description: The AE algorithm would need a flash for good results > > - - name: AeStatePrecapture > > - value: 5 > > - description: | > > - The AE algorithm has started a pre-capture metering session. > > - \sa AePrecaptureTrigger > > - > > - AfState: > > type: int32_t > > draft: true > > -- > > 2.27.0 > >
diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml index 9d4638ae..18b186e8 100644 --- a/src/libcamera/control_ids.yaml +++ b/src/libcamera/control_ids.yaml @@ -7,23 +7,61 @@ # Unless otherwise stated, all controls are bi-directional, i.e. they can be # set through Request::controls() and returned out through Request::metadata(). controls: - - AeEnable: - type: bool - description: | - Enable or disable the AE. - - \sa ExposureTime AnalogueGain - - - AeLocked: - type: bool + - AeState: + type: int32_t description: | - 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. + Control to report the current AE algorithm state. Enabling or disabling + any of the AE-related mode controls (eg. AnalogueGainMode, + ExposureTimeMode) is not required to reset the state to + AeStateInactive. The camera device can do several state transitions + between two results, if it is allowed by the state transition table. + For example, AeStateInactive may never actually be seen in a result. + + The state in the result is the state for this image (in sync with this + image). If AE state becomes AeStateConverged, then the image data + associated with the result should be good to use. + + As some AE algorithms may still be running when any of the AE-related + controls are in manual mode, the states are valid even when the mode + controls are set to Disabled. To know if this image image used AE or + manual (or a mixture), check the relevant modes (eg. AnalogueGainMode, + ExposureTimeMode). + + \sa AnalogueGain + \sa AnalogueGainMode + \sa ExposureTime + \sa ExposureTimeMode - \sa AeEnable + enum: + - name: AeStateInactive + value: 0 + description: | + The AE algorithm is inactive. + If the camera initiates an AE scan, the state shall go to Searching. + - name: AeStateSearching + value: 1 + description: | + The AE algorithm has not converged yet. + If the camera finishes an AE scan, the state shall go to Converged. + If the camera finishes an AE scan, but flash is required, the state + shall go to FlashRequired. + - name: AeStateConverged + value: 2 + description: | + The AE algorithm has converged. + If the camera initiates an AE scan, the state shall go to Searching. + - name: AeStateFlashRequired + value: 3 + description: | + The AE algorithm would need a flash for good results + If the camera initiates an AE scan, the state shall go to Searching. + If AeLock is on, the state shall go to Locked. + - name: AeStatePrecapture + value: 4 + description: | + The AE algorithm has started a pre-capture metering session. + After the sequence is finished, the state shall go to Converged if + \sa AePrecaptureTrigger # AeMeteringMode needs further attention: # - Auto-generate max enum value. @@ -93,6 +131,17 @@ controls: how the desired total exposure is divided between the shutter time and the sensor's analogue gain. The exposure modes are platform specific, and not all exposure modes may be supported. + + When one of AnalogueGainMode or ExposureTimeMode is set to Disabled and + a corresponding manual control is provided, AeExposureMode may be + ignored. This is because, for example, if AeExposureMode is set to + ExposureLong but ExposureTimeMode is Disabled and a short ExposureTime + is provided (AnalogueGainMode set to Auto), then the AeExposureMode + doesn't make sense. + + \sa AnalogueGainMode + \sa ExposureTimeMode + enum: - name: ExposureNormal value: 0 @@ -111,13 +160,15 @@ controls: type: float description: | Specify an Exposure Value (EV) parameter. The EV parameter will only be - applied if the AE algorithm is currently enabled. + applied if the AE algorithm is currently enabled, that is, at least one + of AnalogueGainMode and ExposureTimeMode are auto. By convention EV adjusts the exposure as log2. For example EV = [-2, -1, 0.5, 0, 0.5, 1, 2] results in an exposure adjustment of [1/4x, 1/2x, 1/sqrt(2)x, 1x, sqrt(2)x, 2x, 4x]. - \sa AeEnable + \sa AnalogueGainMode + \sa ExposureTimeMode - ExposureTime: type: int32_t @@ -125,17 +176,49 @@ controls: Exposure time (shutter speed) for the frame applied in the sensor device. This value is specified in micro-seconds. - 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. + This control will only take effect if ExposureTimeMode is Disabled. Its + presence in a request acts as a trigger to switch to the internal + manual mode within ExposureTimeModeDisabled. + + When reported in metadata, this control indicates what exposure time + was used for the current request, regardless of ExposureTimeMode. + ExposureTimeMode will indicate the source of the exposure time value, + whether it came from the AE algorithm or not. - \sa AnalogueGain AeEnable + \sa AnalogueGain + \sa ExposureTimeMode - \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. + - ExposureTimeMode: + type: int32_t + description: | + Control to set and report the source of the exposure time that will be + applied. + + \sa ExposureTime + enum: + - name: ExposureTimeModeAuto + value: 0 + description: | + The exposure time will be calculated automatically and set by the + AE algorithm. + - name: ExposureTimeModeDisabled + value: 1 + description: | + The exposure time will not be updated by the AE algorithm. It will + come from the last calculated value when the mode was Auto, or from + the value specified in ExposureTime. + + This Disabled state has two internal states; locked and manual. + When the Disabled state is first entered, the internal state will + be locked, and the latest exposure time value set by the AE + algorithm will be used (or the default ExposureTime value, if the + mode started out as Disabled). When an ExposureTime is provided, + then the internal state will go to manual, and the provided value + will be used for the exposure time. Going from manual to locked is + not possible. If an ExposureTime value is provided in the same + request as going into the Disabled state, then the internal locked + state will be skipped, and the internal state will start out as + manual. - AnalogueGain: type: float @@ -144,17 +227,49 @@ controls: 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. + This control will only take effect if ExposureTimeMode is Disabled. Its + presence in a request acts as a trigger to switch to the internal + manual mode within ExposureTimeModeDisabled. - \sa ExposureTime AeEnable + When reported in metadata, this control indicates what exposure time + was used for the current request, regardless of ExposureTimeMode. + ExposureTimeMode will indicate the source of the exposure time value, + whether it came from the AE algorithm or not. + + \sa ExposureTime + \sa AnalogueGainMode + + - AnalogueGainMode: + type: int32_t + description: | + Control to set and report the source of the analogue gain that will be + applied. - \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. + \sa AnalogueGain + enum: + - name: AnalogueGainModeAuto + value: 0 + description: | + The analogue gain will be calculated automatically and set by the + AE algorithm. + - name: AnalogueGainModeDisabled + value: 1 + description: | + The analogue gain will not be updated by the AE algorithm. It will + come from the last calculated value when the mode was Auto, or from + the value specified in AnalogueGain. + + This Disabled state has two internal states; locked and manual. + When the Disabled state is first entered, the internal state will + be locked, and the latest analogue gain value set by the AE + algorithm will be used (or the default AnalogueGain value, if the + mode started out as Disabled). When an AnalogueGain is provided, + then the internal state will go to manual, and the provided value + will be used for the analogue gain. Going from manual to locked is + not possible. If an AnalogueGain value is provided in the same + request as going into the Disabled state, then the internal locked + state will be skipped, and the internal state will start out as + manual. - Brightness: type: float @@ -477,36 +592,6 @@ controls: High quality aberration correction which might reduce the frame rate. - - AeState: - type: int32_t - draft: true - description: | - Control to report the current AE algorithm state. Currently identical to - ANDROID_CONTROL_AE_STATE. - - Current state of the AE algorithm. - enum: - - name: AeStateInactive - value: 0 - description: The AE algorithm is inactive. - - name: AeStateSearching - value: 1 - description: The AE algorithm has not converged yet. - - name: AeStateConverged - value: 2 - description: The AE algorithm has converged. - - name: AeStateLocked - value: 3 - description: The AE algorithm is locked. - - name: AeStateFlashRequired - value: 4 - description: The AE algorithm would need a flash for good results - - name: AeStatePrecapture - value: 5 - description: | - The AE algorithm has started a pre-capture metering session. - \sa AePrecaptureTrigger - - AfState: type: int32_t draft: true
We have multiple goals: - we need a lock of some sort, to instruct the AEGC to not update output results - we need manual modes, to override the values computed by the AEGC - we need to support seamless transitions from auto -> manual, and do so without flickering - we need custom minimum values for the manual controls, that is no magic values for enabling/disabling auto - all of these need to be done with AE sub-controls (exposure time, analogue gain) To achieve these goals, we introduce mode controls for the AE sub-controls: ExposureTimeMode and AnalogueGainMode. These have an auto state, and a disabled state. The disabled state has an internal one-way state change from locked to manual, triggered by the presence of the value-controls (ExposureTime and AnalogueGain). We then remove the AeEnable control, as it is a redundant control in the face of these two mode controls. We also remove AeLocked, as it is insufficient for reporting the AE state, and we promote AeState to non-draft to fill its role. Notably, the locked state is removed, since this information can be obtained from the aforementioned mode controls. Bug: https://bugs.libcamera.org/show_bug.cgi?id=42 Bug: https://bugs.libcamera.org/show_bug.cgi?id=43 Bug: https://bugs.libcamera.org/show_bug.cgi?id=47 Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> --- Changes in v2: - No changes, just resubmitting at the head of this series so that it's together and so that /people will actually see it/ Initial version: Still RFC as I haven't updated the users of the control yet, and I want to check that these are the controls and docs that we want. We've decided that the "master AE control" will be implemented by a helper... but looking at uvcvideo and the V4L2 controls I'm wondering if such helper should come earlier than later? --- src/libcamera/control_ids.yaml | 215 +++++++++++++++++++++++---------- 1 file changed, 150 insertions(+), 65 deletions(-)