| Message ID | 20230612153934.5618-1-hdegoede@redhat.com |
|---|---|
| State | Accepted |
| Commit | c49958d0b456619c32a04d074f592cc1665a24b5 |
| Headers | show |
| Series |
|
| Related | show |
Hi Hans, Thank you for the patch. On Mon, Jun 12, 2023 at 05:39:34PM +0200, Hans de Goede via libcamera-devel wrote: > Recent media-ctl versions include the framerate in the fmt property output: > > - entity 37: ov5693 4-0036 (1 pad, 1 link) > type V4L2 subdev subtype Sensor flags 0 > device node name /dev/v4l-subdev6 > pad0: Source > [fmt:SBGGR10_1X10/2592x1944@1/30 > crop.bounds:(16,6)/2592x1944 > crop:(16,6)/2592x1944] > -> "ipu3-csi2 1":0 [ENABLED] > > This resulted in $sensor_size getting set to: "2592x1944@1 30", which > causes the script to fail. > > Fix this by: > > 1. Replacing the gsub() to remove the '/' between e.g. SBGGR10_1X10 > and 2592x1944 with a sub() so that only that first '/' gets replaced > (resulting in a $sensor_size of "2592x1944@1/30" instead). > > 2. Adding a new sub() to remove the @1/30 suffix. > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> Does this mean you have a use for this script ? I was actually considering dropping it, as it's clearly unmaintained, and has long outlived its initial purpose of helping developing the IPU3 pipeline handler. > --- > utils/ipu3/ipu3-capture.sh | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/utils/ipu3/ipu3-capture.sh b/utils/ipu3/ipu3-capture.sh > index ba6147b4..9294d025 100755 > --- a/utils/ipu3/ipu3-capture.sh > +++ b/utils/ipu3/ipu3-capture.sh > @@ -63,7 +63,8 @@ parse_pipeline() { > if (sensor) { > gsub(\".*fmt:\", \"\"); > gsub(\"[] ].*\", \"\"); > - gsub(\"/\", \" \"); > + sub(\"/\", \" \"); > + sub(\"@[0-9]+/[0-9]+\", \"\"); > format=\$0; > } > }
Hi Laurent, On 6/12/23 18:07, Laurent Pinchart wrote: > Hi Hans, > > Thank you for the patch. > > On Mon, Jun 12, 2023 at 05:39:34PM +0200, Hans de Goede via libcamera-devel wrote: >> Recent media-ctl versions include the framerate in the fmt property output: >> >> - entity 37: ov5693 4-0036 (1 pad, 1 link) >> type V4L2 subdev subtype Sensor flags 0 >> device node name /dev/v4l-subdev6 >> pad0: Source >> [fmt:SBGGR10_1X10/2592x1944@1/30 >> crop.bounds:(16,6)/2592x1944 >> crop:(16,6)/2592x1944] >> -> "ipu3-csi2 1":0 [ENABLED] >> >> This resulted in $sensor_size getting set to: "2592x1944@1 30", which >> causes the script to fail. >> >> Fix this by: >> >> 1. Replacing the gsub() to remove the '/' between e.g. SBGGR10_1X10 >> and 2592x1944 with a sub() so that only that first '/' gets replaced >> (resulting in a $sensor_size of "2592x1944@1/30" instead). >> >> 2. Adding a new sub() to remove the @1/30 suffix. >> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> > > Does this mean you have a use for this script ? I was actually > considering dropping it, as it's clearly unmaintained, and has long > outlived its initial purpose of helping developing the IPU3 pipeline > handler. I am working on getting the ov2680 on a Lenovo Miix 510 to work (1) after verifying + fixing this script by capturing some raw frames on a Surface Go with OV5693 my test plan is to first get the capturing of raw frames working before moving to adding libcamera support for the ov2680 sensor (which will require adding the mandatory controls to the driver). TL;DR: yes I have a use for this, being able to capture raw frames is useful to separately test the sensor driver bringup from hooking things up in libcamera. I was actually thinking that it would be nice to get something similar working on IPU6 . Regards, Hans 1) Both as a standalone project and because it is another way to test the drivers/media/i2c/ov2680.c fixes and updates I've been working on for atomisp. > >> --- >> utils/ipu3/ipu3-capture.sh | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/utils/ipu3/ipu3-capture.sh b/utils/ipu3/ipu3-capture.sh >> index ba6147b4..9294d025 100755 >> --- a/utils/ipu3/ipu3-capture.sh >> +++ b/utils/ipu3/ipu3-capture.sh >> @@ -63,7 +63,8 @@ parse_pipeline() { >> if (sensor) { >> gsub(\".*fmt:\", \"\"); >> gsub(\"[] ].*\", \"\"); >> - gsub(\"/\", \" \"); >> + sub(\"/\", \" \"); >> + sub(\"@[0-9]+/[0-9]+\", \"\"); >> format=\$0; >> } >> } >
Quoting Hans de Goede via libcamera-devel (2023-06-12 19:23:10) > Hi Laurent, > > On 6/12/23 18:07, Laurent Pinchart wrote: > > Hi Hans, > > > > Thank you for the patch. > > > > On Mon, Jun 12, 2023 at 05:39:34PM +0200, Hans de Goede via libcamera-devel wrote: > >> Recent media-ctl versions include the framerate in the fmt property output: > >> > >> - entity 37: ov5693 4-0036 (1 pad, 1 link) > >> type V4L2 subdev subtype Sensor flags 0 > >> device node name /dev/v4l-subdev6 > >> pad0: Source > >> [fmt:SBGGR10_1X10/2592x1944@1/30 > >> crop.bounds:(16,6)/2592x1944 > >> crop:(16,6)/2592x1944] > >> -> "ipu3-csi2 1":0 [ENABLED] > >> > >> This resulted in $sensor_size getting set to: "2592x1944@1 30", which > >> causes the script to fail. > >> > >> Fix this by: > >> > >> 1. Replacing the gsub() to remove the '/' between e.g. SBGGR10_1X10 > >> and 2592x1944 with a sub() so that only that first '/' gets replaced > >> (resulting in a $sensor_size of "2592x1944@1/30" instead). > >> > >> 2. Adding a new sub() to remove the @1/30 suffix. > >> > >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> > > > > Does this mean you have a use for this script ? I was actually > > considering dropping it, as it's clearly unmaintained, and has long > > outlived its initial purpose of helping developing the IPU3 pipeline > > handler. > > I am working on getting the ov2680 on a Lenovo Miix 510 to work (1) > after verifying + fixing this script by capturing some raw frames > on a Surface Go with OV5693 my test plan is to first get > the capturing of raw frames working before moving to adding > libcamera support for the ov2680 sensor (which will require > adding the mandatory controls to the driver). > > TL;DR: yes I have a use for this, being able to capture raw > frames is useful to separately test the sensor driver bringup > from hooking things up in libcamera. Well maybe we should pick this patch already then. I guess media-ctl shouldn't really be consumed by scripts like this - but if it's working now at least we can save that and it may still be useful in the future. Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > I was actually thinking that it would be nice to get something > similar working on IPU6 . > > Regards, > > Hans > > > 1) Both as a standalone project and because it is another way to > test the drivers/media/i2c/ov2680.c fixes and updates I've been > working on for atomisp. > > > > > > > > >> --- > >> utils/ipu3/ipu3-capture.sh | 3 ++- > >> 1 file changed, 2 insertions(+), 1 deletion(-) > >> > >> diff --git a/utils/ipu3/ipu3-capture.sh b/utils/ipu3/ipu3-capture.sh > >> index ba6147b4..9294d025 100755 > >> --- a/utils/ipu3/ipu3-capture.sh > >> +++ b/utils/ipu3/ipu3-capture.sh > >> @@ -63,7 +63,8 @@ parse_pipeline() { > >> if (sensor) { > >> gsub(\".*fmt:\", \"\"); > >> gsub(\"[] ].*\", \"\"); > >> - gsub(\"/\", \" \"); > >> + sub(\"/\", \" \"); > >> + sub(\"@[0-9]+/[0-9]+\", \"\"); > >> format=\$0; > >> } > >> } > > >
On Wed, Jul 05, 2023 at 12:38:06AM +0100, Kieran Bingham wrote: > Quoting Hans de Goede via libcamera-devel (2023-06-12 19:23:10) > > On 6/12/23 18:07, Laurent Pinchart wrote: > > > On Mon, Jun 12, 2023 at 05:39:34PM +0200, Hans de Goede via libcamera-devel wrote: > > >> Recent media-ctl versions include the framerate in the fmt property output: > > >> > > >> - entity 37: ov5693 4-0036 (1 pad, 1 link) > > >> type V4L2 subdev subtype Sensor flags 0 > > >> device node name /dev/v4l-subdev6 > > >> pad0: Source > > >> [fmt:SBGGR10_1X10/2592x1944@1/30 > > >> crop.bounds:(16,6)/2592x1944 > > >> crop:(16,6)/2592x1944] > > >> -> "ipu3-csi2 1":0 [ENABLED] > > >> > > >> This resulted in $sensor_size getting set to: "2592x1944@1 30", which > > >> causes the script to fail. > > >> > > >> Fix this by: > > >> > > >> 1. Replacing the gsub() to remove the '/' between e.g. SBGGR10_1X10 > > >> and 2592x1944 with a sub() so that only that first '/' gets replaced > > >> (resulting in a $sensor_size of "2592x1944@1/30" instead). > > >> > > >> 2. Adding a new sub() to remove the @1/30 suffix. > > >> > > >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> > > > > > > Does this mean you have a use for this script ? I was actually > > > considering dropping it, as it's clearly unmaintained, and has long > > > outlived its initial purpose of helping developing the IPU3 pipeline > > > handler. > > > > I am working on getting the ov2680 on a Lenovo Miix 510 to work (1) > > after verifying + fixing this script by capturing some raw frames > > on a Surface Go with OV5693 my test plan is to first get > > the capturing of raw frames working before moving to adding > > libcamera support for the ov2680 sensor (which will require > > adding the mandatory controls to the driver). > > > > TL;DR: yes I have a use for this, being able to capture raw > > frames is useful to separately test the sensor driver bringup > > from hooking things up in libcamera. > > Well maybe we should pick this patch already then. > > I guess media-ctl shouldn't really be consumed by scripts like this - > but if it's working now at least we can save that and it may still be > useful in the future. > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> Regardless of whether or not we decide to drop this script, I think we shouldn't leave it in a known broken state. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > I was actually thinking that it would be nice to get something > > similar working on IPU6 . > > > > Regards, > > > > Hans > > > > 1) Both as a standalone project and because it is another way to > > test the drivers/media/i2c/ov2680.c fixes and updates I've been > > working on for atomisp. > > > > >> --- > > >> utils/ipu3/ipu3-capture.sh | 3 ++- > > >> 1 file changed, 2 insertions(+), 1 deletion(-) > > >> > > >> diff --git a/utils/ipu3/ipu3-capture.sh b/utils/ipu3/ipu3-capture.sh > > >> index ba6147b4..9294d025 100755 > > >> --- a/utils/ipu3/ipu3-capture.sh > > >> +++ b/utils/ipu3/ipu3-capture.sh > > >> @@ -63,7 +63,8 @@ parse_pipeline() { > > >> if (sensor) { > > >> gsub(\".*fmt:\", \"\"); > > >> gsub(\"[] ].*\", \"\"); > > >> - gsub(\"/\", \" \"); > > >> + sub(\"/\", \" \"); > > >> + sub(\"@[0-9]+/[0-9]+\", \"\"); > > >> format=\$0; > > >> } > > >> }
diff --git a/utils/ipu3/ipu3-capture.sh b/utils/ipu3/ipu3-capture.sh index ba6147b4..9294d025 100755 --- a/utils/ipu3/ipu3-capture.sh +++ b/utils/ipu3/ipu3-capture.sh @@ -63,7 +63,8 @@ parse_pipeline() { if (sensor) { gsub(\".*fmt:\", \"\"); gsub(\"[] ].*\", \"\"); - gsub(\"/\", \" \"); + sub(\"/\", \" \"); + sub(\"@[0-9]+/[0-9]+\", \"\"); format=\$0; } }
Recent media-ctl versions include the framerate in the fmt property output: - entity 37: ov5693 4-0036 (1 pad, 1 link) type V4L2 subdev subtype Sensor flags 0 device node name /dev/v4l-subdev6 pad0: Source [fmt:SBGGR10_1X10/2592x1944@1/30 crop.bounds:(16,6)/2592x1944 crop:(16,6)/2592x1944] -> "ipu3-csi2 1":0 [ENABLED] This resulted in $sensor_size getting set to: "2592x1944@1 30", which causes the script to fail. Fix this by: 1. Replacing the gsub() to remove the '/' between e.g. SBGGR10_1X10 and 2592x1944 with a sub() so that only that first '/' gets replaced (resulting in a $sensor_size of "2592x1944@1/30" instead). 2. Adding a new sub() to remove the @1/30 suffix. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- utils/ipu3/ipu3-capture.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)