Message ID | 20210528030531.189492-1-hiroh@chromium.org |
---|---|
State | Changes Requested |
Headers | show |
Series |
|
Related | show |
Hi Hiro, Thank you for the patch. On Fri, May 28, 2021 at 12:05:26PM +0900, Hirokazu Honda wrote: > The control is used to report available sensor test pattern modes > and also specify the mode to sensor. > > Signed-off-by: Hirokazu Honda <hiroh@chromium.org> > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> > --- > src/libcamera/control_ids.yaml | 58 ++++++++++++++++++++++++++++++++++ > 1 file changed, 58 insertions(+) > > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml > index 88d81ac4..b1fe1dc6 100644 > --- a/src/libcamera/control_ids.yaml > +++ b/src/libcamera/control_ids.yaml > @@ -615,4 +615,62 @@ controls: > detection, additional format conversions etc) count as an additional > pipeline stage. > > + - TestPatternMode: > + type: int32_t > + draft: true > + description: | > + Control to select the test pattern mode. Currently identical to > + ANDROID_SENSOR_TEST_PATTERN_MODE. I'd add that the test pattern modes are identical to the ones defined by the CCS specification. Maybe something along the lines of: The modes are described in a pixel-perfect way in the MIPI Camera Command Set (https://www.mipi.org/specifications/camera-command-set) specification. I've wondered for a long time how we could standardize test pattern modes when there's no standard at the hardware level, and I've been pointed out recently to the fact that CCS defines pixel-perfect test patterns (they are parametrized by a set of read-only parameters reported by the sensor though, the specification is worth a read). This comes from the SMIA++ specification that also standardized test pattern modes, but from what I've been told, not in a pixel-perfect way. Apart from that, this looks good to me for now as it's a draft control. We'll have to decide how to handle custom patterns once we destage this control. > + enum: > + - name: TestPatternModeOff > + value: 0 > + description: | > + No test pattern mode is used. The camera device returns frames from > + the image sensor. > + - name: TestPatternModeSolidColor > + value: 1 > + description: | > + Each pixel in [R, G_even, G_odd, B] is replaced by its respective > + color channel provided in test pattern data. > + \todo Add control for test pattern data. > + - name: TestPatternModeColorBars > + value: 2 > + description: | > + All pixel data is replaced with an 8-bar color pattern. The vertical > + bars (left-to-right) are as follows; white, yellow, cyan, green, > + magenta, red, blue and black. Each bar should take up 1/8 of the > + sensor pixel array width. When this is not possible, the bar size > + should be rounded down to the nearest integer and the pattern can > + repeat on the right side. Each bar's height must always take up the > + full sensor pixel array height. > + - name: TestPatternModeColorBarsFadeToGray > + value: 3 > + description: | > + The test pattern is similar to TestPatternModeColorBars, > + except that each bar should start at its specified color at the top > + and fade to gray at the bottom. Furthermore each bar is further > + subdevided into a left and right half. The left half should have a > + smooth gradient, and the right half should have a quantized > + gradient. In particular, the right half's should consist of blocks > + of the same color for 1/16th active sensor pixel array width. The > + least significant bits in the quantized gradient should be copied > + from the most significant bits of the smooth gradient. The height of > + each bar should always be a multiple of 128. When this is not the > + case, the pattern should repeat at the bottom of the image. > + - name: TestPatternModePn9 > + value: 4 > + description: | > + All pixel data is replaced by a pseudo-random sequence generated > + from a PN9 512-bit sequence (typically implemented in hardware with > + a linear feedback shift register). The generator should be reset at > + the beginning of each frame, and thus each subsequent raw frame with > + this test pattern should be exactly the same as the last. > + - name: TestPatternModeCustom1 > + value: 5 This starts at 256 in the Android camera HAL API, I'd do the same as the control is documented as being identical to ANDROID_SENSOR_TEST_PATTERN_MODE. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + description: | > + The first custom test pattern. All custom patterns that are > + available only on this camera device are at least this numeric > + value. All of the custom test patterns will be static (that is the > + raw image must not vary from frame to frame). > + > ...
Hi Laurent, On Mon, Jun 7, 2021 at 9:01 AM Laurent Pinchart < laurent.pinchart@ideasonboard.com> wrote: > Hi Hiro, > > Thank you for the patch. > > On Fri, May 28, 2021 at 12:05:26PM +0900, Hirokazu Honda wrote: > > The control is used to report available sensor test pattern modes > > and also specify the mode to sensor. > > > > Signed-off-by: Hirokazu Honda <hiroh@chromium.org> > > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> > > --- > > src/libcamera/control_ids.yaml | 58 ++++++++++++++++++++++++++++++++++ > > 1 file changed, 58 insertions(+) > > > > diff --git a/src/libcamera/control_ids.yaml > b/src/libcamera/control_ids.yaml > > index 88d81ac4..b1fe1dc6 100644 > > --- a/src/libcamera/control_ids.yaml > > +++ b/src/libcamera/control_ids.yaml > > @@ -615,4 +615,62 @@ controls: > > detection, additional format conversions etc) count as an > additional > > pipeline stage. > > > > + - TestPatternMode: > > + type: int32_t > > + draft: true > > + description: | > > + Control to select the test pattern mode. Currently identical to > > + ANDROID_SENSOR_TEST_PATTERN_MODE. > > I'd add that the test pattern modes are identical to the ones defined by > the CCS specification. Maybe something along the lines of: > > The modes are described in a pixel-perfect way in the MIPI > Camera Command Set ( > https://www.mipi.org/specifications/camera-command-set) > specification. > > I've wondered for a long time how we could standardize test pattern > modes when there's no standard at the hardware level, and I've been > pointed out recently to the fact that CCS defines pixel-perfect test > patterns (they are parametrized by a set of read-only parameters > reported by the sensor though, the specification is worth a read). This > comes from the SMIA++ specification that also standardized test pattern > modes, but from what I've been told, not in a pixel-perfect way. > > Thanks Laurent for the info. The test pattern modes defined in CCS is almost one-to-one correspondence to Android test pattern modes though 100% color tile is not defined in Android. To test the test pattern modes in a pixel-perfect way, we have to tell a client the parameters. I am fine to re-declare the test pattern mode control based on CCS definition, but testing them in a pixel-perfect way seems to require much more work. Regards, -Hiro > Apart from that, this looks good to me for now as it's a draft control. > We'll have to decide how to handle custom patterns once we destage this > control. > > > + enum: > > + - name: TestPatternModeOff > > + value: 0 > > + description: | > > + No test pattern mode is used. The camera device returns > frames from > > + the image sensor. > > + - name: TestPatternModeSolidColor > > + value: 1 > > + description: | > > + Each pixel in [R, G_even, G_odd, B] is replaced by its > respective > > + color channel provided in test pattern data. > > + \todo Add control for test pattern data. > > + - name: TestPatternModeColorBars > > + value: 2 > > + description: | > > + All pixel data is replaced with an 8-bar color pattern. The > vertical > > + bars (left-to-right) are as follows; white, yellow, cyan, > green, > > + magenta, red, blue and black. Each bar should take up 1/8 > of the > > + sensor pixel array width. When this is not possible, the > bar size > > + should be rounded down to the nearest integer and the > pattern can > > + repeat on the right side. Each bar's height must always > take up the > > + full sensor pixel array height. > > + - name: TestPatternModeColorBarsFadeToGray > > + value: 3 > > + description: | > > + The test pattern is similar to TestPatternModeColorBars, > > + except that each bar should start at its specified color at > the top > > + and fade to gray at the bottom. Furthermore each bar is > further > > + subdevided into a left and right half. The left half should > have a > > + smooth gradient, and the right half should have a quantized > > + gradient. In particular, the right half's should consist of > blocks > > + of the same color for 1/16th active sensor pixel array > width. The > > + least significant bits in the quantized gradient should be > copied > > + from the most significant bits of the smooth gradient. The > height of > > + each bar should always be a multiple of 128. When this is > not the > > + case, the pattern should repeat at the bottom of the image. > > + - name: TestPatternModePn9 > > + value: 4 > > + description: | > > + All pixel data is replaced by a pseudo-random sequence > generated > > + from a PN9 512-bit sequence (typically implemented in > hardware with > > + a linear feedback shift register). The generator should be > reset at > > + the beginning of each frame, and thus each subsequent raw > frame with > > + this test pattern should be exactly the same as the last. > > + - name: TestPatternModeCustom1 > > + value: 5 > > This starts at 256 in the Android camera HAL API, I'd do the same as the > control is documented as being identical to > ANDROID_SENSOR_TEST_PATTERN_MODE. > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > + description: | > > + The first custom test pattern. All custom patterns that are > > + available only on this camera device are at least this > numeric > > + value. All of the custom test patterns will be static (that > is the > > + raw image must not vary from frame to frame). > > + > > ... > > -- > Regards, > > Laurent Pinchart >
Hi Hiro, On Mon, Jun 07, 2021 at 10:07:55AM +0900, Hirokazu Honda wrote: > On Mon, Jun 7, 2021 at 9:01 AM Laurent Pinchart wrote: > > On Fri, May 28, 2021 at 12:05:26PM +0900, Hirokazu Honda wrote: > > > The control is used to report available sensor test pattern modes > > > and also specify the mode to sensor. > > > > > > Signed-off-by: Hirokazu Honda <hiroh@chromium.org> > > > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> > > > --- > > > src/libcamera/control_ids.yaml | 58 ++++++++++++++++++++++++++++++++++ > > > 1 file changed, 58 insertions(+) > > > > > > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml > > > index 88d81ac4..b1fe1dc6 100644 > > > --- a/src/libcamera/control_ids.yaml > > > +++ b/src/libcamera/control_ids.yaml > > > @@ -615,4 +615,62 @@ controls: > > > detection, additional format conversions etc) count as an additional > > > pipeline stage. > > > > > > + - TestPatternMode: > > > + type: int32_t > > > + draft: true > > > + description: | > > > + Control to select the test pattern mode. Currently identical to > > > + ANDROID_SENSOR_TEST_PATTERN_MODE. > > > > I'd add that the test pattern modes are identical to the ones defined by > > the CCS specification. Maybe something along the lines of: > > > > The modes are described in a pixel-perfect way in the MIPI > > Camera Command Set (https://www.mipi.org/specifications/camera-command-set) > > specification. > > > > I've wondered for a long time how we could standardize test pattern > > modes when there's no standard at the hardware level, and I've been > > pointed out recently to the fact that CCS defines pixel-perfect test > > patterns (they are parametrized by a set of read-only parameters > > reported by the sensor though, the specification is worth a read). This > > comes from the SMIA++ specification that also standardized test pattern > > modes, but from what I've been told, not in a pixel-perfect way. > > Thanks Laurent for the info. The test pattern modes defined in CCS is > almost one-to-one correspondence to Android test pattern modes though 100% > color tile is not defined in Android. I think that's because both are based on SMIA++ (I have no proof for this though). > To test the test pattern modes in a pixel-perfect way, we have to tell a > client the parameters. > I am fine to re-declare the test pattern mode control based on CCS > definition, but testing them in a pixel-perfect way seems to require much > more work. Agreed, I think it's not worth the extra work for now. > > Apart from that, this looks good to me for now as it's a draft control. > > We'll have to decide how to handle custom patterns once we destage this > > control. > > > > > + enum: > > > + - name: TestPatternModeOff > > > + value: 0 > > > + description: | > > > + No test pattern mode is used. The camera device returns frames from > > > + the image sensor. > > > + - name: TestPatternModeSolidColor > > > + value: 1 > > > + description: | > > > + Each pixel in [R, G_even, G_odd, B] is replaced by its respective > > > + color channel provided in test pattern data. > > > + \todo Add control for test pattern data. > > > + - name: TestPatternModeColorBars > > > + value: 2 > > > + description: | > > > + All pixel data is replaced with an 8-bar color pattern. The vertical > > > + bars (left-to-right) are as follows; white, yellow, cyan, green, > > > + magenta, red, blue and black. Each bar should take up 1/8 of the > > > + sensor pixel array width. When this is not possible, the bar size > > > + should be rounded down to the nearest integer and the pattern can > > > + repeat on the right side. Each bar's height must always take up the > > > + full sensor pixel array height. > > > + - name: TestPatternModeColorBarsFadeToGray > > > + value: 3 > > > + description: | > > > + The test pattern is similar to TestPatternModeColorBars, > > > + except that each bar should start at its specified color at the top > > > + and fade to gray at the bottom. Furthermore each bar is further > > > + subdevided into a left and right half. The left half should have a > > > + smooth gradient, and the right half should have a quantized > > > + gradient. In particular, the right half's should consist of blocks > > > + of the same color for 1/16th active sensor pixel array width. The > > > + least significant bits in the quantized gradient should be copied > > > + from the most significant bits of the smooth gradient. The height of > > > + each bar should always be a multiple of 128. When this is not the > > > + case, the pattern should repeat at the bottom of the image. > > > + - name: TestPatternModePn9 > > > + value: 4 > > > + description: | > > > + All pixel data is replaced by a pseudo-random sequence generated > > > + from a PN9 512-bit sequence (typically implemented in hardware with > > > + a linear feedback shift register). The generator should be reset at > > > + the beginning of each frame, and thus each subsequent raw frame with > > > + this test pattern should be exactly the same as the last. > > > + - name: TestPatternModeCustom1 > > > + value: 5 > > > > This starts at 256 in the Android camera HAL API, I'd do the same as > > the control is documented as being identical to > > ANDROID_SENSOR_TEST_PATTERN_MODE. > > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > > > + description: | > > > + The first custom test pattern. All custom patterns that are > > > + available only on this camera device are at least this numeric > > > + value. All of the custom test patterns will be static (that is the > > > + raw image must not vary from frame to frame). > > > + > > > ...
diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml index 88d81ac4..b1fe1dc6 100644 --- a/src/libcamera/control_ids.yaml +++ b/src/libcamera/control_ids.yaml @@ -615,4 +615,62 @@ controls: detection, additional format conversions etc) count as an additional pipeline stage. + - TestPatternMode: + type: int32_t + draft: true + description: | + Control to select the test pattern mode. Currently identical to + ANDROID_SENSOR_TEST_PATTERN_MODE. + enum: + - name: TestPatternModeOff + value: 0 + description: | + No test pattern mode is used. The camera device returns frames from + the image sensor. + - name: TestPatternModeSolidColor + value: 1 + description: | + Each pixel in [R, G_even, G_odd, B] is replaced by its respective + color channel provided in test pattern data. + \todo Add control for test pattern data. + - name: TestPatternModeColorBars + value: 2 + description: | + All pixel data is replaced with an 8-bar color pattern. The vertical + bars (left-to-right) are as follows; white, yellow, cyan, green, + magenta, red, blue and black. Each bar should take up 1/8 of the + sensor pixel array width. When this is not possible, the bar size + should be rounded down to the nearest integer and the pattern can + repeat on the right side. Each bar's height must always take up the + full sensor pixel array height. + - name: TestPatternModeColorBarsFadeToGray + value: 3 + description: | + The test pattern is similar to TestPatternModeColorBars, + except that each bar should start at its specified color at the top + and fade to gray at the bottom. Furthermore each bar is further + subdevided into a left and right half. The left half should have a + smooth gradient, and the right half should have a quantized + gradient. In particular, the right half's should consist of blocks + of the same color for 1/16th active sensor pixel array width. The + least significant bits in the quantized gradient should be copied + from the most significant bits of the smooth gradient. The height of + each bar should always be a multiple of 128. When this is not the + case, the pattern should repeat at the bottom of the image. + - name: TestPatternModePn9 + value: 4 + description: | + All pixel data is replaced by a pseudo-random sequence generated + from a PN9 512-bit sequence (typically implemented in hardware with + a linear feedback shift register). The generator should be reset at + the beginning of each frame, and thus each subsequent raw frame with + this test pattern should be exactly the same as the last. + - name: TestPatternModeCustom1 + value: 5 + description: | + The first custom test pattern. All custom patterns that are + available only on this camera device are at least this numeric + value. All of the custom test patterns will be static (that is the + raw image must not vary from frame to frame). + ...