| Message ID | 20190912160122.5545-3-p.zabel@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] media: imx: enable V4L2_PIX_FMT_XBGR32, _BGRX32, and _RGBX32 | expand |
On 9/12/19 6:01 PM, Philipp Zabel wrote: > Iterate over all media bus formats, not just over the first format in > each imx_media_pixfmt entry. > > Before: > > $ v4l2-ctl -d $(media-ctl -e ipu1_csi0) --list-subdev-mbus-codes 0 > ioctl: VIDIOC_SUBDEV_ENUM_MBUS_CODE (pad=0) > 0x2006: MEDIA_BUS_FMT_UYVY8_2X8 > 0x2008: MEDIA_BUS_FMT_YUYV8_2X8 > 0x1008: MEDIA_BUS_FMT_RGB565_2X8_LE > 0x100a: MEDIA_BUS_FMT_RGB888_1X24 > 0x100d: MEDIA_BUS_FMT_ARGB8888_1X32 > 0x3001: MEDIA_BUS_FMT_SBGGR8_1X8 > 0x3013: MEDIA_BUS_FMT_SGBRG8_1X8 > 0x3002: MEDIA_BUS_FMT_SGRBG8_1X8 > 0x3014: MEDIA_BUS_FMT_SRGGB8_1X8 > 0x3007: MEDIA_BUS_FMT_SBGGR10_1X10 > 0x300e: MEDIA_BUS_FMT_SGBRG10_1X10 > 0x300a: MEDIA_BUS_FMT_SGRBG10_1X10 > 0x300f: MEDIA_BUS_FMT_SRGGB10_1X10 > 0x2001: MEDIA_BUS_FMT_Y8_1X8 > 0x200a: MEDIA_BUS_FMT_Y10_1X10 > > After: > > $ v4l2-ctl -d $(media-ctl -e ipu1_csi0) --list-subdev-mbus-codes 0 > ioctl: VIDIOC_SUBDEV_ENUM_MBUS_CODE (pad=0) > 0x2006: MEDIA_BUS_FMT_UYVY8_2X8 > 0x200f: MEDIA_BUS_FMT_UYVY8_1X16 > 0x2008: MEDIA_BUS_FMT_YUYV8_2X8 > 0x2011: MEDIA_BUS_FMT_YUYV8_1X16 > 0x1008: MEDIA_BUS_FMT_RGB565_2X8_LE > 0x100a: MEDIA_BUS_FMT_RGB888_1X24 > 0x100c: MEDIA_BUS_FMT_RGB888_2X12_LE > 0x100d: MEDIA_BUS_FMT_ARGB8888_1X32 > 0x3001: MEDIA_BUS_FMT_SBGGR8_1X8 > 0x3013: MEDIA_BUS_FMT_SGBRG8_1X8 > 0x3002: MEDIA_BUS_FMT_SGRBG8_1X8 > 0x3014: MEDIA_BUS_FMT_SRGGB8_1X8 > 0x3007: MEDIA_BUS_FMT_SBGGR10_1X10 > 0x3008: MEDIA_BUS_FMT_SBGGR12_1X12 > 0x3019: MEDIA_BUS_FMT_SBGGR14_1X14 > 0x301d: MEDIA_BUS_FMT_SBGGR16_1X16 > 0x300e: MEDIA_BUS_FMT_SGBRG10_1X10 > 0x3010: MEDIA_BUS_FMT_SGBRG12_1X12 > 0x301a: MEDIA_BUS_FMT_SGBRG14_1X14 > 0x301e: MEDIA_BUS_FMT_SGBRG16_1X16 > 0x300a: MEDIA_BUS_FMT_SGRBG10_1X10 > 0x3011: MEDIA_BUS_FMT_SGRBG12_1X12 > 0x301b: MEDIA_BUS_FMT_SGRBG14_1X14 > 0x301f: MEDIA_BUS_FMT_SGRBG16_1X16 > 0x300f: MEDIA_BUS_FMT_SRGGB10_1X10 > 0x3012: MEDIA_BUS_FMT_SRGGB12_1X12 > 0x301c: MEDIA_BUS_FMT_SRGGB14_1X14 > 0x3020: MEDIA_BUS_FMT_SRGGB16_1X16 > 0x2001: MEDIA_BUS_FMT_Y8_1X8 > 0x200a: MEDIA_BUS_FMT_Y10_1X10 > 0x2013: MEDIA_BUS_FMT_Y12_1X12 > > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de> > --- > drivers/staging/media/imx/imx-media-utils.c | 19 +++++++++++++++---- > 1 file changed, 15 insertions(+), 4 deletions(-) > > diff --git a/drivers/staging/media/imx/imx-media-utils.c b/drivers/staging/media/imx/imx-media-utils.c > index d61a8f4533dc..5f8604db4dd4 100644 > --- a/drivers/staging/media/imx/imx-media-utils.c > +++ b/drivers/staging/media/imx/imx-media-utils.c > @@ -254,7 +254,7 @@ static int enum_format(u32 *fourcc, u32 *code, u32 index, > bool allow_bayer) > { This function is becoming confusing. I think you should add some comments explaining what the function does. Specifically the fourcc and code arguments. Can both be non-NULL? Or only one of the two? I think that if fourcc is non-NULL you enumerate over the V4L2 pixelformats, if code is non-NULL, then you enumerate over the mediabus codes. If so, then I think it would be easier to understand if you just make two functions: enum_formats and enum_codes, rather than trying to merge them into one. Patches 1 and 2 of this series look good, so I'll take those. Regards, Hans > const struct imx_media_pixfmt *fmt; > - unsigned int i, j = 0; > + unsigned int i, j, k = 0; > > for (i = 0; i < ARRAY_SIZE(pixel_formats); i++) { > fmt = &pixel_formats[i]; > @@ -264,18 +264,29 @@ static int enum_format(u32 *fourcc, u32 *code, u32 index, > (!allow_bayer && fmt->bayer)) > continue; > > - if (index == j) > + if (fourcc && index == k) > break; > > - j++; > + if (!code) { > + k++; > + continue; > + } > + > + for (j = 0; j < ARRAY_SIZE(fmt->codes) && fmt->codes[j]; j++) { > + if (index == k) > + goto out; > + > + k++; > + } > } > if (i == ARRAY_SIZE(pixel_formats)) > return -EINVAL; > > +out: > if (fourcc) > *fourcc = fmt->fourcc; > if (code) > - *code = fmt->codes[0]; > + *code = fmt->codes[j]; > > return 0; > } >
Hi Hans, On 9/27/19 12:33 AM, Hans Verkuil wrote: > On 9/12/19 6:01 PM, Philipp Zabel wrote: >> Iterate over all media bus formats, not just over the first format in >> each imx_media_pixfmt entry. >> >> Before: >> >> $ v4l2-ctl -d $(media-ctl -e ipu1_csi0) --list-subdev-mbus-codes 0 >> ioctl: VIDIOC_SUBDEV_ENUM_MBUS_CODE (pad=0) >> 0x2006: MEDIA_BUS_FMT_UYVY8_2X8 >> 0x2008: MEDIA_BUS_FMT_YUYV8_2X8 >> 0x1008: MEDIA_BUS_FMT_RGB565_2X8_LE >> 0x100a: MEDIA_BUS_FMT_RGB888_1X24 >> 0x100d: MEDIA_BUS_FMT_ARGB8888_1X32 >> 0x3001: MEDIA_BUS_FMT_SBGGR8_1X8 >> 0x3013: MEDIA_BUS_FMT_SGBRG8_1X8 >> 0x3002: MEDIA_BUS_FMT_SGRBG8_1X8 >> 0x3014: MEDIA_BUS_FMT_SRGGB8_1X8 >> 0x3007: MEDIA_BUS_FMT_SBGGR10_1X10 >> 0x300e: MEDIA_BUS_FMT_SGBRG10_1X10 >> 0x300a: MEDIA_BUS_FMT_SGRBG10_1X10 >> 0x300f: MEDIA_BUS_FMT_SRGGB10_1X10 >> 0x2001: MEDIA_BUS_FMT_Y8_1X8 >> 0x200a: MEDIA_BUS_FMT_Y10_1X10 >> >> After: >> >> $ v4l2-ctl -d $(media-ctl -e ipu1_csi0) --list-subdev-mbus-codes 0 >> ioctl: VIDIOC_SUBDEV_ENUM_MBUS_CODE (pad=0) >> 0x2006: MEDIA_BUS_FMT_UYVY8_2X8 >> 0x200f: MEDIA_BUS_FMT_UYVY8_1X16 >> 0x2008: MEDIA_BUS_FMT_YUYV8_2X8 >> 0x2011: MEDIA_BUS_FMT_YUYV8_1X16 >> 0x1008: MEDIA_BUS_FMT_RGB565_2X8_LE >> 0x100a: MEDIA_BUS_FMT_RGB888_1X24 >> 0x100c: MEDIA_BUS_FMT_RGB888_2X12_LE >> 0x100d: MEDIA_BUS_FMT_ARGB8888_1X32 >> 0x3001: MEDIA_BUS_FMT_SBGGR8_1X8 >> 0x3013: MEDIA_BUS_FMT_SGBRG8_1X8 >> 0x3002: MEDIA_BUS_FMT_SGRBG8_1X8 >> 0x3014: MEDIA_BUS_FMT_SRGGB8_1X8 >> 0x3007: MEDIA_BUS_FMT_SBGGR10_1X10 >> 0x3008: MEDIA_BUS_FMT_SBGGR12_1X12 >> 0x3019: MEDIA_BUS_FMT_SBGGR14_1X14 >> 0x301d: MEDIA_BUS_FMT_SBGGR16_1X16 >> 0x300e: MEDIA_BUS_FMT_SGBRG10_1X10 >> 0x3010: MEDIA_BUS_FMT_SGBRG12_1X12 >> 0x301a: MEDIA_BUS_FMT_SGBRG14_1X14 >> 0x301e: MEDIA_BUS_FMT_SGBRG16_1X16 >> 0x300a: MEDIA_BUS_FMT_SGRBG10_1X10 >> 0x3011: MEDIA_BUS_FMT_SGRBG12_1X12 >> 0x301b: MEDIA_BUS_FMT_SGRBG14_1X14 >> 0x301f: MEDIA_BUS_FMT_SGRBG16_1X16 >> 0x300f: MEDIA_BUS_FMT_SRGGB10_1X10 >> 0x3012: MEDIA_BUS_FMT_SRGGB12_1X12 >> 0x301c: MEDIA_BUS_FMT_SRGGB14_1X14 >> 0x3020: MEDIA_BUS_FMT_SRGGB16_1X16 >> 0x2001: MEDIA_BUS_FMT_Y8_1X8 >> 0x200a: MEDIA_BUS_FMT_Y10_1X10 >> 0x2013: MEDIA_BUS_FMT_Y12_1X12 >> >> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de> >> --- >> drivers/staging/media/imx/imx-media-utils.c | 19 +++++++++++++++---- >> 1 file changed, 15 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/staging/media/imx/imx-media-utils.c b/drivers/staging/media/imx/imx-media-utils.c >> index d61a8f4533dc..5f8604db4dd4 100644 >> --- a/drivers/staging/media/imx/imx-media-utils.c >> +++ b/drivers/staging/media/imx/imx-media-utils.c >> @@ -254,7 +254,7 @@ static int enum_format(u32 *fourcc, u32 *code, u32 index, >> bool allow_bayer) >> { > This function is becoming confusing. I think you should add some comments explaining > what the function does. Specifically the fourcc and code arguments. > > Can both be non-NULL? Or only one of the two? I think that if fourcc is non-NULL you > enumerate over the V4L2 pixelformats, if code is non-NULL, then you enumerate over > the mediabus codes. > > If so, then I think it would be easier to understand if you just make two functions: > enum_formats and enum_codes, rather than trying to merge them into one. I don't think the function is that confusing, but I'm fine with splitting it into enum_formats() and enum_codes(). I do agree it needs some comments describing how it works. I think my suggestion to rename the index that counts entries that match the search criteria to "match_index" will also help to follow the code. Steve > > Patches 1 and 2 of this series look good, so I'll take those. > > Regards, > > Hans > >> const struct imx_media_pixfmt *fmt; >> - unsigned int i, j = 0; >> + unsigned int i, j, k = 0; >> >> for (i = 0; i < ARRAY_SIZE(pixel_formats); i++) { >> fmt = &pixel_formats[i]; >> @@ -264,18 +264,29 @@ static int enum_format(u32 *fourcc, u32 *code, u32 index, >> (!allow_bayer && fmt->bayer)) >> continue; >> >> - if (index == j) >> + if (fourcc && index == k) >> break; >> >> - j++; >> + if (!code) { >> + k++; >> + continue; >> + } >> + >> + for (j = 0; j < ARRAY_SIZE(fmt->codes) && fmt->codes[j]; j++) { >> + if (index == k) >> + goto out; >> + >> + k++; >> + } >> } >> if (i == ARRAY_SIZE(pixel_formats)) >> return -EINVAL; >> >> +out: >> if (fourcc) >> *fourcc = fmt->fourcc; >> if (code) >> - *code = fmt->codes[0]; >> + *code = fmt->codes[j]; >> >> return 0; >> } >>
Hi Hans, Philipp, On 10/25/19 2:48 PM, Steve Longerbeam wrote: > Hi Hans, > > On 9/27/19 12:33 AM, Hans Verkuil wrote: >> On 9/12/19 6:01 PM, Philipp Zabel wrote: >>> Iterate over all media bus formats, not just over the first format in >>> each imx_media_pixfmt entry. >>> >>> Before: >>> >>> $ v4l2-ctl -d $(media-ctl -e ipu1_csi0) --list-subdev-mbus-codes 0 >>> ioctl: VIDIOC_SUBDEV_ENUM_MBUS_CODE (pad=0) >>> 0x2006: MEDIA_BUS_FMT_UYVY8_2X8 >>> 0x2008: MEDIA_BUS_FMT_YUYV8_2X8 >>> 0x1008: MEDIA_BUS_FMT_RGB565_2X8_LE >>> 0x100a: MEDIA_BUS_FMT_RGB888_1X24 >>> 0x100d: MEDIA_BUS_FMT_ARGB8888_1X32 >>> 0x3001: MEDIA_BUS_FMT_SBGGR8_1X8 >>> 0x3013: MEDIA_BUS_FMT_SGBRG8_1X8 >>> 0x3002: MEDIA_BUS_FMT_SGRBG8_1X8 >>> 0x3014: MEDIA_BUS_FMT_SRGGB8_1X8 >>> 0x3007: MEDIA_BUS_FMT_SBGGR10_1X10 >>> 0x300e: MEDIA_BUS_FMT_SGBRG10_1X10 >>> 0x300a: MEDIA_BUS_FMT_SGRBG10_1X10 >>> 0x300f: MEDIA_BUS_FMT_SRGGB10_1X10 >>> 0x2001: MEDIA_BUS_FMT_Y8_1X8 >>> 0x200a: MEDIA_BUS_FMT_Y10_1X10 >>> >>> After: >>> >>> $ v4l2-ctl -d $(media-ctl -e ipu1_csi0) --list-subdev-mbus-codes 0 >>> ioctl: VIDIOC_SUBDEV_ENUM_MBUS_CODE (pad=0) >>> 0x2006: MEDIA_BUS_FMT_UYVY8_2X8 >>> 0x200f: MEDIA_BUS_FMT_UYVY8_1X16 >>> 0x2008: MEDIA_BUS_FMT_YUYV8_2X8 >>> 0x2011: MEDIA_BUS_FMT_YUYV8_1X16 >>> 0x1008: MEDIA_BUS_FMT_RGB565_2X8_LE >>> 0x100a: MEDIA_BUS_FMT_RGB888_1X24 >>> 0x100c: MEDIA_BUS_FMT_RGB888_2X12_LE >>> 0x100d: MEDIA_BUS_FMT_ARGB8888_1X32 >>> 0x3001: MEDIA_BUS_FMT_SBGGR8_1X8 >>> 0x3013: MEDIA_BUS_FMT_SGBRG8_1X8 >>> 0x3002: MEDIA_BUS_FMT_SGRBG8_1X8 >>> 0x3014: MEDIA_BUS_FMT_SRGGB8_1X8 >>> 0x3007: MEDIA_BUS_FMT_SBGGR10_1X10 >>> 0x3008: MEDIA_BUS_FMT_SBGGR12_1X12 >>> 0x3019: MEDIA_BUS_FMT_SBGGR14_1X14 >>> 0x301d: MEDIA_BUS_FMT_SBGGR16_1X16 >>> 0x300e: MEDIA_BUS_FMT_SGBRG10_1X10 >>> 0x3010: MEDIA_BUS_FMT_SGBRG12_1X12 >>> 0x301a: MEDIA_BUS_FMT_SGBRG14_1X14 >>> 0x301e: MEDIA_BUS_FMT_SGBRG16_1X16 >>> 0x300a: MEDIA_BUS_FMT_SGRBG10_1X10 >>> 0x3011: MEDIA_BUS_FMT_SGRBG12_1X12 >>> 0x301b: MEDIA_BUS_FMT_SGRBG14_1X14 >>> 0x301f: MEDIA_BUS_FMT_SGRBG16_1X16 >>> 0x300f: MEDIA_BUS_FMT_SRGGB10_1X10 >>> 0x3012: MEDIA_BUS_FMT_SRGGB12_1X12 >>> 0x301c: MEDIA_BUS_FMT_SRGGB14_1X14 >>> 0x3020: MEDIA_BUS_FMT_SRGGB16_1X16 >>> 0x2001: MEDIA_BUS_FMT_Y8_1X8 >>> 0x200a: MEDIA_BUS_FMT_Y10_1X10 >>> 0x2013: MEDIA_BUS_FMT_Y12_1X12 >>> >>> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de> >>> --- >>> drivers/staging/media/imx/imx-media-utils.c | 19 +++++++++++++++---- >>> 1 file changed, 15 insertions(+), 4 deletions(-) >>> >>> diff --git a/drivers/staging/media/imx/imx-media-utils.c >>> b/drivers/staging/media/imx/imx-media-utils.c >>> index d61a8f4533dc..5f8604db4dd4 100644 >>> --- a/drivers/staging/media/imx/imx-media-utils.c >>> +++ b/drivers/staging/media/imx/imx-media-utils.c >>> @@ -254,7 +254,7 @@ static int enum_format(u32 *fourcc, u32 *code, >>> u32 index, >>> bool allow_bayer) >>> { >> This function is becoming confusing. I think you should add some >> comments explaining >> what the function does. Specifically the fourcc and code arguments. >> >> Can both be non-NULL? Or only one of the two? I think that if fourcc >> is non-NULL you >> enumerate over the V4L2 pixelformats, if code is non-NULL, then you >> enumerate over >> the mediabus codes. >> >> If so, then I think it would be easier to understand if you just make >> two functions: >> enum_formats and enum_codes, rather than trying to merge them into one. > > I don't think the function is that confusing, but I'm fine with > splitting it into enum_formats() and enum_codes(). > > I do agree it needs some comments describing how it works. I think my > suggestion to rename the index that counts entries that match the > search criteria to "match_index" will also help to follow the code. > I added a comment block for find_format() and enum_format() in my media-tree github fork: git@github.com:slongerbeam/mediatree.git, branch imx/philipp-pixfmts-cleanup Steve
diff --git a/drivers/staging/media/imx/imx-media-utils.c b/drivers/staging/media/imx/imx-media-utils.c index d61a8f4533dc..5f8604db4dd4 100644 --- a/drivers/staging/media/imx/imx-media-utils.c +++ b/drivers/staging/media/imx/imx-media-utils.c @@ -254,7 +254,7 @@ static int enum_format(u32 *fourcc, u32 *code, u32 index, bool allow_bayer) { const struct imx_media_pixfmt *fmt; - unsigned int i, j = 0; + unsigned int i, j, k = 0; for (i = 0; i < ARRAY_SIZE(pixel_formats); i++) { fmt = &pixel_formats[i]; @@ -264,18 +264,29 @@ static int enum_format(u32 *fourcc, u32 *code, u32 index, (!allow_bayer && fmt->bayer)) continue; - if (index == j) + if (fourcc && index == k) break; - j++; + if (!code) { + k++; + continue; + } + + for (j = 0; j < ARRAY_SIZE(fmt->codes) && fmt->codes[j]; j++) { + if (index == k) + goto out; + + k++; + } } if (i == ARRAY_SIZE(pixel_formats)) return -EINVAL; +out: if (fourcc) *fourcc = fmt->fourcc; if (code) - *code = fmt->codes[0]; + *code = fmt->codes[j]; return 0; }
Iterate over all media bus formats, not just over the first format in each imx_media_pixfmt entry. Before: $ v4l2-ctl -d $(media-ctl -e ipu1_csi0) --list-subdev-mbus-codes 0 ioctl: VIDIOC_SUBDEV_ENUM_MBUS_CODE (pad=0) 0x2006: MEDIA_BUS_FMT_UYVY8_2X8 0x2008: MEDIA_BUS_FMT_YUYV8_2X8 0x1008: MEDIA_BUS_FMT_RGB565_2X8_LE 0x100a: MEDIA_BUS_FMT_RGB888_1X24 0x100d: MEDIA_BUS_FMT_ARGB8888_1X32 0x3001: MEDIA_BUS_FMT_SBGGR8_1X8 0x3013: MEDIA_BUS_FMT_SGBRG8_1X8 0x3002: MEDIA_BUS_FMT_SGRBG8_1X8 0x3014: MEDIA_BUS_FMT_SRGGB8_1X8 0x3007: MEDIA_BUS_FMT_SBGGR10_1X10 0x300e: MEDIA_BUS_FMT_SGBRG10_1X10 0x300a: MEDIA_BUS_FMT_SGRBG10_1X10 0x300f: MEDIA_BUS_FMT_SRGGB10_1X10 0x2001: MEDIA_BUS_FMT_Y8_1X8 0x200a: MEDIA_BUS_FMT_Y10_1X10 After: $ v4l2-ctl -d $(media-ctl -e ipu1_csi0) --list-subdev-mbus-codes 0 ioctl: VIDIOC_SUBDEV_ENUM_MBUS_CODE (pad=0) 0x2006: MEDIA_BUS_FMT_UYVY8_2X8 0x200f: MEDIA_BUS_FMT_UYVY8_1X16 0x2008: MEDIA_BUS_FMT_YUYV8_2X8 0x2011: MEDIA_BUS_FMT_YUYV8_1X16 0x1008: MEDIA_BUS_FMT_RGB565_2X8_LE 0x100a: MEDIA_BUS_FMT_RGB888_1X24 0x100c: MEDIA_BUS_FMT_RGB888_2X12_LE 0x100d: MEDIA_BUS_FMT_ARGB8888_1X32 0x3001: MEDIA_BUS_FMT_SBGGR8_1X8 0x3013: MEDIA_BUS_FMT_SGBRG8_1X8 0x3002: MEDIA_BUS_FMT_SGRBG8_1X8 0x3014: MEDIA_BUS_FMT_SRGGB8_1X8 0x3007: MEDIA_BUS_FMT_SBGGR10_1X10 0x3008: MEDIA_BUS_FMT_SBGGR12_1X12 0x3019: MEDIA_BUS_FMT_SBGGR14_1X14 0x301d: MEDIA_BUS_FMT_SBGGR16_1X16 0x300e: MEDIA_BUS_FMT_SGBRG10_1X10 0x3010: MEDIA_BUS_FMT_SGBRG12_1X12 0x301a: MEDIA_BUS_FMT_SGBRG14_1X14 0x301e: MEDIA_BUS_FMT_SGBRG16_1X16 0x300a: MEDIA_BUS_FMT_SGRBG10_1X10 0x3011: MEDIA_BUS_FMT_SGRBG12_1X12 0x301b: MEDIA_BUS_FMT_SGRBG14_1X14 0x301f: MEDIA_BUS_FMT_SGRBG16_1X16 0x300f: MEDIA_BUS_FMT_SRGGB10_1X10 0x3012: MEDIA_BUS_FMT_SRGGB12_1X12 0x301c: MEDIA_BUS_FMT_SRGGB14_1X14 0x3020: MEDIA_BUS_FMT_SRGGB16_1X16 0x2001: MEDIA_BUS_FMT_Y8_1X8 0x200a: MEDIA_BUS_FMT_Y10_1X10 0x2013: MEDIA_BUS_FMT_Y12_1X12 Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de> --- drivers/staging/media/imx/imx-media-utils.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-)