| Message ID | 20221102103804.3933874-1-kieran.bingham@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Kieran On Wed, Nov 02, 2022 at 10:38:04AM +0000, Kieran Bingham via libcamera-devel wrote: > Some UVC cameras have been identified that can provide V4L2 menu > controls without any menu items. > > This leads to a segfault where we try to construct a > ControlInfo(Span<>,default) with an empty span. > > Returning ControlInfo(); instead results in a Fatal log error due to the > control validation failing. > > Provide the default value as the only acceptable menu control item, and > report the issue on the debug log. > > Bug: https://bugs.libcamera.org/show_bug.cgi?id=167 > Reported-by: Marian Buschsieweke <marian.buschsieweke@ovgu.de> > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > v2: > - Sigh, I forgot the SoB/Trailers in the commit message. > > src/libcamera/v4l2_device.cpp | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp > index c17b323f8af0..f35820271f51 100644 > --- a/src/libcamera/v4l2_device.cpp > +++ b/src/libcamera/v4l2_device.cpp > @@ -583,6 +583,16 @@ ControlInfo V4L2Device::v4l2MenuControlInfo(const struct v4l2_query_ext_ctrl &ct > indices.push_back(index); > } > > + /* > + * Some faulty UVC drivers are known to return an empty menu control. > + * In that event, populate the indicies with the default value only. > + */ Not sure what this mean in practice... The bug report says: "Exposure Auto" menu has not a single supported item However it is reported as listing 3 menu entries by v4l2-ctl -l exposure_auto 0x009a0901 (menu) : min=0 max=3 default=0 value=0 I guess the issue is then that QUERYMENU is not supported ? How does this work in UVC, I guess the uvcdriver supports the ioctl, is it the device fw which wrongly implements the uvcvideo specification ? If that's the case why only push the default to indices ? Aren't min and max selectable too ? (yes, we're going to miss all other values in between min/max which are != default, but if the device is buggy we can't do more than this...) As this fix a user bug I don't want to get into the way of this fix, but just to validate my understanding can you clarify this last part ? > + if (indices.size() == 0) { > + LOG(V4L2, Debug) > + << "Menu control '" << ctrl.name << "' has no entries"; > + indices.push_back(static_cast<int32_t>(ctrl.default_value)); > + } > + > return ControlInfo(indices, > ControlValue(static_cast<int32_t>(ctrl.default_value))); > } > -- > 2.34.1 >
Hi, > If that's the case why only push the default to indices? see the note at https://www.kernel.org/doc/html/v6.0/userspace-api/media/v4l/vidioc-queryctrl.html#description It is possible for VIDIOC_QUERYMENU to return an EINVAL error code for some indices between minimum and maximum. In that case that particular menu item is not supported by this driver. [...] I interpret this as "if VIDIOC_QUERYMENU fails, that menu item is not supported" and not as "if VIDIOC_QUERYMENU fails, that menu item is supported but has no label". This seems to also be the interpretation behind the implementation in libcamera that just removes menu items for which VIDIOC_QUERYMENU fail. I can test this tomorrow to confirm that exposure_auto cannot be set to any of [0..3], or whether indeed just the labels are missing. Assuming the interpretation in the code is correct and in fact the menu exposure_auto contains no supported item to pick, it is IMO most sensible to either not expose the control for it, or to provide a bogus single-item menu. The latter approach resulted in the cleaner and less complex patch (: Kind regards, Marian On Wed, 2 Nov 2022 17:22:37 +0100 Jacopo Mondi <jacopo@jmondi.org> wrote: > Hi Kieran > > On Wed, Nov 02, 2022 at 10:38:04AM +0000, Kieran Bingham via libcamera-devel > wrote: [...] > > Not sure what this mean in practice... The bug report says: > > "Exposure Auto" menu has not a single supported item > > However it is reported as listing 3 menu entries by v4l2-ctl -l > > exposure_auto 0x009a0901 (menu) : min=0 max=3 default=0 value=0 > > I guess the issue is then that QUERYMENU is not supported ? How does this > work in UVC, I guess the uvcdriver supports the ioctl, is it the > device fw which wrongly implements the uvcvideo specification ? > > If that's the case why only push the default to indices ? Aren't min > and max selectable too ? (yes, we're going to miss all other values in > between min/max which are != default, but if the device is buggy we > can't do more than this...) > > As this fix a user bug I don't want to get into the way of this fix, > but just to validate my understanding can you clarify this last part ? > > [...]
Hi Marian On Wed, Nov 02, 2022 at 09:44:41PM +0100, Marian Buschsieweke wrote: > Hi, > > > If that's the case why only push the default to indices? > > see the note at > https://www.kernel.org/doc/html/v6.0/userspace-api/media/v4l/vidioc-queryctrl.html#description > > It is possible for VIDIOC_QUERYMENU to return an EINVAL error code for some > indices between minimum and maximum. In that case that particular menu item > is not supported by this driver. [...] > > I interpret this as "if VIDIOC_QUERYMENU fails, that menu item is not > supported" and not as "if VIDIOC_QUERYMENU fails, that menu item is supported > but has no label". This seems to also be the interpretation behind the > implementation in libcamera that just removes menu items for which > VIDIOC_QUERYMENU fail. > > I can test this tomorrow to confirm that exposure_auto cannot be set to any of > [0..3], or whether indeed just the labels are missing. Thanks this will help understanding what's going wrong here > > Assuming the interpretation in the code is correct and in fact the menu > exposure_auto contains no supported item to pick, it is IMO most sensible to > either not expose the control for it, or to provide a bogus single-item menu. > The latter approach resulted in the cleaner and less complex patch (: Again, not an UVC expert (we have a few here that might provide some guidance) but I'm surprised the control gets even registered by the driver... Ofc, if not value can be set on the control without hitting an error, I would rather not register the control to libcamera at all... > > Kind regards, > Marian > > > On Wed, 2 Nov 2022 17:22:37 +0100 > Jacopo Mondi <jacopo@jmondi.org> wrote: > > > Hi Kieran > > > > On Wed, Nov 02, 2022 at 10:38:04AM +0000, Kieran Bingham via libcamera-devel > > wrote: [...] > > > > Not sure what this mean in practice... The bug report says: > > > > "Exposure Auto" menu has not a single supported item > > > > However it is reported as listing 3 menu entries by v4l2-ctl -l > > > > exposure_auto 0x009a0901 (menu) : min=0 max=3 default=0 value=0 > > > > I guess the issue is then that QUERYMENU is not supported ? How does this > > work in UVC, I guess the uvcdriver supports the ioctl, is it the > > device fw which wrongly implements the uvcvideo specification ? > > > > If that's the case why only push the default to indices ? Aren't min > > and max selectable too ? (yes, we're going to miss all other values in > > between min/max which are != default, but if the device is buggy we > > can't do more than this...) > > > > As this fix a user bug I don't want to get into the way of this fix, > > but just to validate my understanding can you clarify this last part ? > > > > [...] >
Hi, On Wed, 2 Nov 2022 21:44:41 +0100 Marian Buschsieweke <marian.buschsieweke@ovgu.de> wrote: > I can test this tomorrow to confirm that exposure_auto cannot be set to any of > [0..3], or whether indeed just the labels are missing. at least for my K20 device it does indeed not support any of the menu items: ~ $ v4l2-ctl --list-devices K20 USB CAMERA: K20 USB CAMERA (usb-0000:00:14.0-1): /dev/video2 /dev/video3 /dev/media1 Integrated Camera: Integrated C (usb-0000:00:14.0-8): /dev/video0 /dev/video1 /dev/media0 ~ $ v4l2-ctl -d /dev/video2 -lL brightness 0x00980900 (int) : min=-64 max=64 step=1 default=0 value=0 contrast 0x00980901 (int) : min=0 max=64 step=1 default=33 value=33 saturation 0x00980902 (int) : min=0 max=128 step=1 default=64 value=64 hue 0x00980903 (int) : min=-40 max=40 step=1 default=0 value=0 white_balance_temperature_auto 0x0098090c (bool) : default=1 value=1 gamma 0x00980910 (int) : min=72 max=500 step=1 default=120 value=120 gain 0x00980913 (int) : min=0 max=100 step=1 default=0 value=0 power_line_frequency 0x00980918 (menu) : min=0 max=2 default=1 value=1 (50 Hz) 0: Disabled 1: 50 Hz 2: 60 Hz white_balance_temperature 0x0098091a (int) : min=2800 max=6500 step=1 default=4600 value=4600 flags=inactive sharpness 0x0098091b (int) : min=0 max=6 step=1 default=6 value=6 backlight_compensation 0x0098091c (int) : min=0 max=2 step=1 default=1 value=1 exposure_auto 0x009a0901 (menu) : min=0 max=3 default=0 value=0 exposure_absolute 0x009a0902 (int) : min=1 max=5000 step=1 default=2500 value=2500 flags=inactive exposure_auto_priority 0x009a0903 (bool) : default=0 value=1 focus_absolute 0x009a090a (int) : min=1 max=1023 step=1 default=1 value=230 flags=inactive focus_auto 0x009a090c (bool) : default=1 value=1 ~ $ v4l2-ctl -d /dev/video2 -c exposure_auto=0 VIDIOC_S_EXT_CTRLS: failed: Invalid argument Error setting controls: Invalid argument ~ $ v4l2-ctl -d /dev/video2 -c exposure_auto=1 VIDIOC_S_EXT_CTRLS: failed: Invalid argument Error setting controls: Invalid argument ~ $ v4l2-ctl -d /dev/video2 -c exposure_auto=2 VIDIOC_S_EXT_CTRLS: failed: Invalid argument Error setting controls: Invalid argument ~ $ v4l2-ctl -d /dev/video2 -c exposure_auto=3 VIDIOC_S_EXT_CTRLS: failed: Invalid argument Error setting controls: Invalid argument Kind regards, Marian
Quoting Marian Buschsieweke (2022-11-03 09:42:35) > Hi, > > On Wed, 2 Nov 2022 21:44:41 +0100 > Marian Buschsieweke <marian.buschsieweke@ovgu.de> wrote: > > > I can test this tomorrow to confirm that exposure_auto cannot be set to any of > > [0..3], or whether indeed just the labels are missing. > > at least for my K20 device it does indeed not support any of the menu items: > > ~ $ v4l2-ctl --list-devices > K20 USB CAMERA: K20 USB CAMERA (usb-0000:00:14.0-1): > /dev/video2 > /dev/video3 > /dev/media1 > > Integrated Camera: Integrated C (usb-0000:00:14.0-8): > /dev/video0 > /dev/video1 > /dev/media0 > > ~ $ v4l2-ctl -d /dev/video2 -lL > brightness 0x00980900 (int) : min=-64 max=64 step=1 default=0 value=0 > contrast 0x00980901 (int) : min=0 max=64 step=1 default=33 value=33 > saturation 0x00980902 (int) : min=0 max=128 step=1 default=64 value=64 > hue 0x00980903 (int) : min=-40 max=40 step=1 default=0 value=0 > white_balance_temperature_auto 0x0098090c (bool) : default=1 value=1 > gamma 0x00980910 (int) : min=72 max=500 step=1 default=120 value=120 > gain 0x00980913 (int) : min=0 max=100 step=1 default=0 value=0 > power_line_frequency 0x00980918 (menu) : min=0 max=2 default=1 value=1 (50 Hz) > 0: Disabled > 1: 50 Hz > 2: 60 Hz > white_balance_temperature 0x0098091a (int) : min=2800 max=6500 step=1 default=4600 value=4600 flags=inactive > sharpness 0x0098091b (int) : min=0 max=6 step=1 default=6 value=6 > backlight_compensation 0x0098091c (int) : min=0 max=2 step=1 default=1 value=1 > exposure_auto 0x009a0901 (menu) : min=0 max=3 default=0 value=0 > exposure_absolute 0x009a0902 (int) : min=1 max=5000 step=1 default=2500 value=2500 flags=inactive > exposure_auto_priority 0x009a0903 (bool) : default=0 value=1 > focus_absolute 0x009a090a (int) : min=1 max=1023 step=1 default=1 value=230 flags=inactive > focus_auto 0x009a090c (bool) : default=1 value=1 > ~ $ v4l2-ctl -d /dev/video2 -c exposure_auto=0 > VIDIOC_S_EXT_CTRLS: failed: Invalid argument > Error setting controls: Invalid argument > ~ $ v4l2-ctl -d /dev/video2 -c exposure_auto=1 > VIDIOC_S_EXT_CTRLS: failed: Invalid argument > Error setting controls: Invalid argument > ~ $ v4l2-ctl -d /dev/video2 -c exposure_auto=2 > VIDIOC_S_EXT_CTRLS: failed: Invalid argument > Error setting controls: Invalid argument > ~ $ v4l2-ctl -d /dev/video2 -c exposure_auto=3 > VIDIOC_S_EXT_CTRLS: failed: Invalid argument > Error setting controls: Invalid argument Ok - so that makes me believe we're indeed better off returning ControlInfo(); That will require a bit more work to fix the upper layer so that it correctly skips controls that are returned as empty - but actually looking at the code - that's already something we need to do. Higher in the function there is the statement: if (ctrl.minimum < 0) return ControlInfo(); Which I expect will crash in the caller right now, as we discovered. So lets drop this version of the patch and fix things up to be able to return ControlInfo(); when indicies are empty - and make sure the higher layers realise this and do not add the control. -- Kieran > > Kind regards, > Marian
Hi Kieran On Thu, Nov 03, 2022 at 09:57:31AM +0000, Kieran Bingham wrote: > Quoting Marian Buschsieweke (2022-11-03 09:42:35) > > Hi, > > > > On Wed, 2 Nov 2022 21:44:41 +0100 > > Marian Buschsieweke <marian.buschsieweke@ovgu.de> wrote: > > > > > I can test this tomorrow to confirm that exposure_auto cannot be set to any of > > > [0..3], or whether indeed just the labels are missing. > > > > at least for my K20 device it does indeed not support any of the menu items: > > > > ~ $ v4l2-ctl --list-devices > > K20 USB CAMERA: K20 USB CAMERA (usb-0000:00:14.0-1): > > /dev/video2 > > /dev/video3 > > /dev/media1 > > > > Integrated Camera: Integrated C (usb-0000:00:14.0-8): > > /dev/video0 > > /dev/video1 > > /dev/media0 > > > > ~ $ v4l2-ctl -d /dev/video2 -lL > > brightness 0x00980900 (int) : min=-64 max=64 step=1 default=0 value=0 > > contrast 0x00980901 (int) : min=0 max=64 step=1 default=33 value=33 > > saturation 0x00980902 (int) : min=0 max=128 step=1 default=64 value=64 > > hue 0x00980903 (int) : min=-40 max=40 step=1 default=0 value=0 > > white_balance_temperature_auto 0x0098090c (bool) : default=1 value=1 > > gamma 0x00980910 (int) : min=72 max=500 step=1 default=120 value=120 > > gain 0x00980913 (int) : min=0 max=100 step=1 default=0 value=0 > > power_line_frequency 0x00980918 (menu) : min=0 max=2 default=1 value=1 (50 Hz) > > 0: Disabled > > 1: 50 Hz > > 2: 60 Hz > > white_balance_temperature 0x0098091a (int) : min=2800 max=6500 step=1 default=4600 value=4600 flags=inactive > > sharpness 0x0098091b (int) : min=0 max=6 step=1 default=6 value=6 > > backlight_compensation 0x0098091c (int) : min=0 max=2 step=1 default=1 value=1 > > exposure_auto 0x009a0901 (menu) : min=0 max=3 default=0 value=0 > > exposure_absolute 0x009a0902 (int) : min=1 max=5000 step=1 default=2500 value=2500 flags=inactive > > exposure_auto_priority 0x009a0903 (bool) : default=0 value=1 > > focus_absolute 0x009a090a (int) : min=1 max=1023 step=1 default=1 value=230 flags=inactive > > focus_auto 0x009a090c (bool) : default=1 value=1 > > ~ $ v4l2-ctl -d /dev/video2 -c exposure_auto=0 > > VIDIOC_S_EXT_CTRLS: failed: Invalid argument > > Error setting controls: Invalid argument > > ~ $ v4l2-ctl -d /dev/video2 -c exposure_auto=1 > > VIDIOC_S_EXT_CTRLS: failed: Invalid argument > > Error setting controls: Invalid argument > > ~ $ v4l2-ctl -d /dev/video2 -c exposure_auto=2 > > VIDIOC_S_EXT_CTRLS: failed: Invalid argument > > Error setting controls: Invalid argument > > ~ $ v4l2-ctl -d /dev/video2 -c exposure_auto=3 > > VIDIOC_S_EXT_CTRLS: failed: Invalid argument > > Error setting controls: Invalid argument > > Ok - so that makes me believe we're indeed better off returning > ControlInfo(); Do you have any clue why UVC reports the control as available even if it can't actually be set ? Is there any value in reporting it with a single fixed value, or the control as advertised by the kernel is completely useless and we can't even trust its default value ? > > That will require a bit more work to fix the upper layer so that it > correctly skips controls that are returned as empty - but actually > looking at the code - that's already something we need to do. > > Higher in the function there is the statement: > > if (ctrl.minimum < 0) > return ControlInfo(); > > Which I expect will crash in the caller right now, as we discovered. > > So lets drop this version of the patch and fix things up to be able to > return ControlInfo(); when indicies are empty - and make sure the higher > layers realise this and do not add the control. > -- > Kieran > > > > > > Kind regards, > > Marian
Quoting Jacopo Mondi (2022-11-03 10:21:54) > Hi Kieran > > On Thu, Nov 03, 2022 at 09:57:31AM +0000, Kieran Bingham wrote: > > Quoting Marian Buschsieweke (2022-11-03 09:42:35) > > > Hi, > > > > > > On Wed, 2 Nov 2022 21:44:41 +0100 > > > Marian Buschsieweke <marian.buschsieweke@ovgu.de> wrote: > > > > > > > I can test this tomorrow to confirm that exposure_auto cannot be set to any of > > > > [0..3], or whether indeed just the labels are missing. > > > > > > at least for my K20 device it does indeed not support any of the menu items: > > > > > > ~ $ v4l2-ctl --list-devices > > > K20 USB CAMERA: K20 USB CAMERA (usb-0000:00:14.0-1): > > > /dev/video2 > > > /dev/video3 > > > /dev/media1 > > > > > > Integrated Camera: Integrated C (usb-0000:00:14.0-8): > > > /dev/video0 > > > /dev/video1 > > > /dev/media0 > > > > > > ~ $ v4l2-ctl -d /dev/video2 -lL > > > brightness 0x00980900 (int) : min=-64 max=64 step=1 default=0 value=0 > > > contrast 0x00980901 (int) : min=0 max=64 step=1 default=33 value=33 > > > saturation 0x00980902 (int) : min=0 max=128 step=1 default=64 value=64 > > > hue 0x00980903 (int) : min=-40 max=40 step=1 default=0 value=0 > > > white_balance_temperature_auto 0x0098090c (bool) : default=1 value=1 > > > gamma 0x00980910 (int) : min=72 max=500 step=1 default=120 value=120 > > > gain 0x00980913 (int) : min=0 max=100 step=1 default=0 value=0 > > > power_line_frequency 0x00980918 (menu) : min=0 max=2 default=1 value=1 (50 Hz) > > > 0: Disabled > > > 1: 50 Hz > > > 2: 60 Hz > > > white_balance_temperature 0x0098091a (int) : min=2800 max=6500 step=1 default=4600 value=4600 flags=inactive > > > sharpness 0x0098091b (int) : min=0 max=6 step=1 default=6 value=6 > > > backlight_compensation 0x0098091c (int) : min=0 max=2 step=1 default=1 value=1 > > > exposure_auto 0x009a0901 (menu) : min=0 max=3 default=0 value=0 > > > exposure_absolute 0x009a0902 (int) : min=1 max=5000 step=1 default=2500 value=2500 flags=inactive > > > exposure_auto_priority 0x009a0903 (bool) : default=0 value=1 > > > focus_absolute 0x009a090a (int) : min=1 max=1023 step=1 default=1 value=230 flags=inactive > > > focus_auto 0x009a090c (bool) : default=1 value=1 > > > ~ $ v4l2-ctl -d /dev/video2 -c exposure_auto=0 > > > VIDIOC_S_EXT_CTRLS: failed: Invalid argument > > > Error setting controls: Invalid argument > > > ~ $ v4l2-ctl -d /dev/video2 -c exposure_auto=1 > > > VIDIOC_S_EXT_CTRLS: failed: Invalid argument > > > Error setting controls: Invalid argument > > > ~ $ v4l2-ctl -d /dev/video2 -c exposure_auto=2 > > > VIDIOC_S_EXT_CTRLS: failed: Invalid argument > > > Error setting controls: Invalid argument > > > ~ $ v4l2-ctl -d /dev/video2 -c exposure_auto=3 > > > VIDIOC_S_EXT_CTRLS: failed: Invalid argument > > > Error setting controls: Invalid argument > > > > Ok - so that makes me believe we're indeed better off returning > > ControlInfo(); > > Do you have any clue why UVC reports the control as available even if > it can't actually be set ? Is there any value in reporting it with a > single fixed value, or the control as advertised by the kernel is > completely useless and we can't even trust its default value ? Ok - so digging into the kernel driver to see if I can see what happened: drivers/media/usb/uvc/uvc_ctrl.c has: static const struct uvc_menu_info exposure_auto_controls[] = { { 2, "Auto Mode" }, { 1, "Manual Mode" }, { 4, "Shutter Priority Mode" }, { 8, "Aperture Priority Mode" }, }; This is mapped/handled with the UVC_CT_AE_MODE_CONTROL selector. and seems to be a bitmask. So far so good. 4.2.2.1.2 Auto-Exposure Mode Control of the UVC 1.5 Class specification states: The Auto-Exposure Mode Control determines whether the device will provide automatic adjustment of the Exposure Time and Iris controls. Attempts to programmatically set the auto- adjusted controls shall result in a protocol STALL and an error code of bRequestErrorCode = “Wrong state”. A GET_RES request issued to this control will return a bitmap of the modes supported by this control. A valid request to this control would have only one bit set (a single mode selected). This control must accept the GET_DEF request and return its default value. So I would anticipate that the device in question has not returned a valid bitmask from the GET_RES request. We would need to get a debug log from a kernel probing the device in question to look further, but for libcamera, as this problem 'exists' we need to workaround / solve it here regardless. I'll continue to update the code to be aware of menu controls that may have zero menu items, and consider that control as invalid (unsettable), and make sure that works in libcamera. -- Kieran > > That will require a bit more work to fix the upper layer so that it > > correctly skips controls that are returned as empty - but actually > > looking at the code - that's already something we need to do. > > > > Higher in the function there is the statement: > > > > if (ctrl.minimum < 0) > > return ControlInfo(); > > > > Which I expect will crash in the caller right now, as we discovered. > > > > So lets drop this version of the patch and fix things up to be able to > > return ControlInfo(); when indicies are empty - and make sure the higher > > layers realise this and do not add the control. > > -- > > Kieran > > > > > > > > > > Kind regards, > > > Marian
diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp index c17b323f8af0..f35820271f51 100644 --- a/src/libcamera/v4l2_device.cpp +++ b/src/libcamera/v4l2_device.cpp @@ -583,6 +583,16 @@ ControlInfo V4L2Device::v4l2MenuControlInfo(const struct v4l2_query_ext_ctrl &ct indices.push_back(index); } + /* + * Some faulty UVC drivers are known to return an empty menu control. + * In that event, populate the indicies with the default value only. + */ + if (indices.size() == 0) { + LOG(V4L2, Debug) + << "Menu control '" << ctrl.name << "' has no entries"; + indices.push_back(static_cast<int32_t>(ctrl.default_value)); + } + return ControlInfo(indices, ControlValue(static_cast<int32_t>(ctrl.default_value))); }
Some UVC cameras have been identified that can provide V4L2 menu controls without any menu items. This leads to a segfault where we try to construct a ControlInfo(Span<>,default) with an empty span. Returning ControlInfo(); instead results in a Fatal log error due to the control validation failing. Provide the default value as the only acceptable menu control item, and report the issue on the debug log. Bug: https://bugs.libcamera.org/show_bug.cgi?id=167 Reported-by: Marian Buschsieweke <marian.buschsieweke@ovgu.de> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> --- v2: - Sigh, I forgot the SoB/Trailers in the commit message. src/libcamera/v4l2_device.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+)