| Message ID | 20220701154701.354052-4-jacopo@jmondi.org |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series |
|
| Related | show |
Hi Jacopo, On Fri, Jul 01, 2022 at 05:46:56PM +0200, Jacopo Mondi wrote: > The AeState control is tricky, it describes the state of the whole AEGC > block which is independently controlled by ExposureTimeMode and > AnalogueGainMode. > > Try to expand the documentation. > > Use "AEGC" when referring to the algorithm. > > Maybe the control should be named AEGCState ? It's uuuugly. But also maybe more correct. idk I'm neutral maybe people that have a bigger preference can do the bikeshedding :) > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > --- > src/libcamera/control_ids.yaml | 64 ++++++++++++++++++++++++---------- > 1 file changed, 46 insertions(+), 18 deletions(-) > > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml > index bb5eeb1507a9..d50df8bcad28 100644 > --- a/src/libcamera/control_ids.yaml > +++ b/src/libcamera/control_ids.yaml > @@ -10,43 +10,71 @@ controls: > - AeState: > type: int32_t > description: | > - Control to report the AE algorithm state associated with the capture > - result. > + Control to report the AEGC algorithm state. > > - The state is still reported even if ExposureTimeMode or > - AnalogueGainMode is set to Manual. > + The AEGC algorithm computes the exposure time and the analogue gain > + values to be applied to the image sensor. > + > + The AEGC algorithm behaviour is controlled by the ExposureTimeMode and > + AnalogueGainMode controls, which allow applications to decide how > + the exposure time and gain are computed, in Auto or Manual mode, > + independently one from the other. s/one from the other/from one another/ > + > + The AeState control reports the AEGC algorithm state through a single > + value and describes it a single computation block which computes s/it // > + both the exposure time and the analogue gain values. > + > + When both the exposure time and analogue gain values are configured to > + be in Manual mode, the AEGC algorithm is quiescent and does not actively > + compute any value and the AeState control will report AeStateIdle. > + > + When at least the exposure time or analogue gain are configured to be > + computed by the AEGC algorithm, the AeState control will report if the > + algorithm has converged to stable values for any of the controls set I would've s/any/all/, but now I'm wondering if it's even possible for AEGC with both E and G set to auto to have only one converged but not the other. Although maybe s/any/all/ is proper, because this is *one* state to describe *all* of AEGC, so I think if at least one is still converging it should be converging, and only if both are converged then it should be converged. Even if it's not really possible, that's probably the more correct description. > + to be computed in Auto mode. > > - \sa AnalogueGain > \sa AnalogueGainMode > - \sa ExposureTime > \sa ExposureTimeMode > > enum: > - name: AeStateIdle > value: 0 > description: | > - The AE algorithm is inactive. > + The AEGC algorithm is inactive. > + > + This state is returned when both AnalogueGainMode and > + ExposureTimeMode are set to Manual and the algorithm is not > + actively computing any value. Do we need to explicitly state an exception for if only one is manual-able? Although due to the constraints that we've defined, if only one is manual-able then both are auto-able. So maybe it's implicit? > > - This state should be returned if both AnalogueGainMode and > - ExposureTimeMode are set to manual (or one, if the camera only > - supports one of the two controls). > + When ExposureTimeMode or AnalogueGainMode are set to Auto mode, the > + AEGC algorithm might spontaneously initiate a new scan, in which > + case the AeState control is moved to AeStateSearching. > - name: AeStateSearching > value: 1 > description: | > - The AE algorithm has not converged yet. > + The AEGC algorithm is actively computing new values, for either the > + exposure time or the analogue gain, but has not converged to a > + stable result yet. > + > + This state is returned if at least one of AnalogueGainMode > + or ExposureTimeMode is set to auto and the algorithm hasn't > + converged yet. > > - This state should be returned if at least one of AnalogueGainMode > - or ExposureTimeMode is set to auto, and the AE algorithm hasn't > - converged yet. If the AE algorithm converges, the state shall go to > - AeStateConverged. > + The AEGC algorithm converges once stable values are computed for > + any of the controls set to be computed in Auto mode. > + > + Once the algorithm converges the state is moved to AeStateConverged. > - name: AeStateConverged > value: 2 > description: | > The AE algorithm has converged. s/AE/AEGC/? Otherwise looks good. Thanks, Paul > > - This state should be returned if at least one of AnalogueGainMode > - or ExposureTimeMode is set to auto, and the AE algorithm has > - converged. > + This state is returned if at least one of AnalogueGainMode > + or ExposureTimeMode is set to Auto, and the AEGC algorithm has > + converged to stable value. > + > + The AEGC algorithm might spontaneously re-initiate an AE scan, in > + which case the state is moved to AeStateSearching. > > # AeMeteringMode needs further attention: > # - Auto-generate max enum value. > -- > 2.36.1 >
Hi Paul On Thu, Aug 04, 2022 at 12:04:10AM +0900, paul.elder@ideasonboard.com wrote: > Hi Jacopo, > > On Fri, Jul 01, 2022 at 05:46:56PM +0200, Jacopo Mondi wrote: > > The AeState control is tricky, it describes the state of the whole AEGC > > block which is independently controlled by ExposureTimeMode and > > AnalogueGainMode. > > > > Try to expand the documentation. > > > > Use "AEGC" when referring to the algorithm. > > > > Maybe the control should be named AEGCState ? > > It's uuuugly. > It is ! > But also maybe more correct. idk I'm neutral maybe people that have a > bigger preference can do the bikeshedding :) > Let's see if we get more comments, I'll keep it as it is for now > > > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > > --- > > src/libcamera/control_ids.yaml | 64 ++++++++++++++++++++++++---------- > > 1 file changed, 46 insertions(+), 18 deletions(-) > > > > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml > > index bb5eeb1507a9..d50df8bcad28 100644 > > --- a/src/libcamera/control_ids.yaml > > +++ b/src/libcamera/control_ids.yaml > > @@ -10,43 +10,71 @@ controls: > > - AeState: > > type: int32_t > > description: | > > - Control to report the AE algorithm state associated with the capture > > - result. > > + Control to report the AEGC algorithm state. > > > > - The state is still reported even if ExposureTimeMode or > > - AnalogueGainMode is set to Manual. > > + The AEGC algorithm computes the exposure time and the analogue gain > > + values to be applied to the image sensor. > > + > > + The AEGC algorithm behaviour is controlled by the ExposureTimeMode and > > + AnalogueGainMode controls, which allow applications to decide how > > + the exposure time and gain are computed, in Auto or Manual mode, > > + independently one from the other. > > s/one from the other/from one another/ > Thanks :) > > + > > + The AeState control reports the AEGC algorithm state through a single > > + value and describes it a single computation block which computes > > s/it // > I actually meant "describes it as a single computation block" Do you think it's correct ? > > + both the exposure time and the analogue gain values. > > + > > + When both the exposure time and analogue gain values are configured to > > + be in Manual mode, the AEGC algorithm is quiescent and does not actively > > + compute any value and the AeState control will report AeStateIdle. > > + > > + When at least the exposure time or analogue gain are configured to be > > + computed by the AEGC algorithm, the AeState control will report if the > > + algorithm has converged to stable values for any of the controls set > > I would've s/any/all/, but now I'm wondering if it's even possible for > AEGC with both E and G set to auto to have only one converged but not > the other. Good question > > Although maybe s/any/all/ is proper, because this is *one* state to > describe *all* of AEGC, so I think if at least one is still converging > it should be converging, and only if both are converged then it should > be converged. Even if it's not really possible, that's probably the more > correct description. > I think s/any/all as you suggested helps clarify it > > + to be computed in Auto mode. > > > > - \sa AnalogueGain > > \sa AnalogueGainMode > > - \sa ExposureTime > > \sa ExposureTimeMode > > > > enum: > > - name: AeStateIdle > > value: 0 > > description: | > > - The AE algorithm is inactive. > > + The AEGC algorithm is inactive. > > + > > + This state is returned when both AnalogueGainMode and > > + ExposureTimeMode are set to Manual and the algorithm is not > > + actively computing any value. > > Do we need to explicitly state an exception for if only one is > manual-able? Although due to the constraints that we've defined, if > only one is manual-able then both are auto-able. So maybe it's implicit? > If I got your question right you suggest that only one might be manual-able and the other is auto-only, hence the state will never be reported as idle ? Isn't this desired ? The AeState control will report the state of the auto control, either if it is searching or it has converged, while the other will be operated in manual mode. I might have missed what constraints you are referring to here. > > > > - This state should be returned if both AnalogueGainMode and > > - ExposureTimeMode are set to manual (or one, if the camera only > > - supports one of the two controls). > > + When ExposureTimeMode or AnalogueGainMode are set to Auto mode, the > > + AEGC algorithm might spontaneously initiate a new scan, in which > > + case the AeState control is moved to AeStateSearching. > > - name: AeStateSearching > > value: 1 > > description: | > > - The AE algorithm has not converged yet. > > + The AEGC algorithm is actively computing new values, for either the > > + exposure time or the analogue gain, but has not converged to a > > + stable result yet. > > + > > + This state is returned if at least one of AnalogueGainMode > > + or ExposureTimeMode is set to auto and the algorithm hasn't > > + converged yet. > > > > - This state should be returned if at least one of AnalogueGainMode > > - or ExposureTimeMode is set to auto, and the AE algorithm hasn't > > - converged yet. If the AE algorithm converges, the state shall go to > > - AeStateConverged. > > + The AEGC algorithm converges once stable values are computed for > > + any of the controls set to be computed in Auto mode. > > + > > + Once the algorithm converges the state is moved to AeStateConverged. > > - name: AeStateConverged > > value: 2 > > description: | > > The AE algorithm has converged. > > s/AE/AEGC/? > Thanks! > > Otherwise looks good. > > > Thanks, > > Paul > > > > > > - This state should be returned if at least one of AnalogueGainMode > > - or ExposureTimeMode is set to auto, and the AE algorithm has > > - converged. > > + This state is returned if at least one of AnalogueGainMode > > + or ExposureTimeMode is set to Auto, and the AEGC algorithm has > > + converged to stable value. > > + > > + The AEGC algorithm might spontaneously re-initiate an AE scan, in > > + which case the state is moved to AeStateSearching. > > > > # AeMeteringMode needs further attention: > > # - Auto-generate max enum value. > > -- > > 2.36.1 > >
diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml index bb5eeb1507a9..d50df8bcad28 100644 --- a/src/libcamera/control_ids.yaml +++ b/src/libcamera/control_ids.yaml @@ -10,43 +10,71 @@ controls: - AeState: type: int32_t description: | - Control to report the AE algorithm state associated with the capture - result. + Control to report the AEGC algorithm state. - The state is still reported even if ExposureTimeMode or - AnalogueGainMode is set to Manual. + The AEGC algorithm computes the exposure time and the analogue gain + values to be applied to the image sensor. + + The AEGC algorithm behaviour is controlled by the ExposureTimeMode and + AnalogueGainMode controls, which allow applications to decide how + the exposure time and gain are computed, in Auto or Manual mode, + independently one from the other. + + The AeState control reports the AEGC algorithm state through a single + value and describes it a single computation block which computes + both the exposure time and the analogue gain values. + + When both the exposure time and analogue gain values are configured to + be in Manual mode, the AEGC algorithm is quiescent and does not actively + compute any value and the AeState control will report AeStateIdle. + + When at least the exposure time or analogue gain are configured to be + computed by the AEGC algorithm, the AeState control will report if the + algorithm has converged to stable values for any of the controls set + to be computed in Auto mode. - \sa AnalogueGain \sa AnalogueGainMode - \sa ExposureTime \sa ExposureTimeMode enum: - name: AeStateIdle value: 0 description: | - The AE algorithm is inactive. + The AEGC algorithm is inactive. + + This state is returned when both AnalogueGainMode and + ExposureTimeMode are set to Manual and the algorithm is not + actively computing any value. - This state should be returned if both AnalogueGainMode and - ExposureTimeMode are set to manual (or one, if the camera only - supports one of the two controls). + When ExposureTimeMode or AnalogueGainMode are set to Auto mode, the + AEGC algorithm might spontaneously initiate a new scan, in which + case the AeState control is moved to AeStateSearching. - name: AeStateSearching value: 1 description: | - The AE algorithm has not converged yet. + The AEGC algorithm is actively computing new values, for either the + exposure time or the analogue gain, but has not converged to a + stable result yet. + + This state is returned if at least one of AnalogueGainMode + or ExposureTimeMode is set to auto and the algorithm hasn't + converged yet. - This state should be returned if at least one of AnalogueGainMode - or ExposureTimeMode is set to auto, and the AE algorithm hasn't - converged yet. If the AE algorithm converges, the state shall go to - AeStateConverged. + The AEGC algorithm converges once stable values are computed for + any of the controls set to be computed in Auto mode. + + Once the algorithm converges the state is moved to AeStateConverged. - name: AeStateConverged value: 2 description: | The AE algorithm has converged. - This state should be returned if at least one of AnalogueGainMode - or ExposureTimeMode is set to auto, and the AE algorithm has - converged. + This state is returned if at least one of AnalogueGainMode + or ExposureTimeMode is set to Auto, and the AEGC algorithm has + converged to stable value. + + The AEGC algorithm might spontaneously re-initiate an AE scan, in + which case the state is moved to AeStateSearching. # AeMeteringMode needs further attention: # - Auto-generate max enum value.
The AeState control is tricky, it describes the state of the whole AEGC block which is independently controlled by ExposureTimeMode and AnalogueGainMode. Try to expand the documentation. Use "AEGC" when referring to the algorithm. Maybe the control should be named AEGCState ? Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> --- src/libcamera/control_ids.yaml | 64 ++++++++++++++++++++++++---------- 1 file changed, 46 insertions(+), 18 deletions(-)