Re: [PATCH v4][media] Exynos4 JPEG codec v4l2 driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Andrzej and others,

On Fri, Nov 25, 2011 at 10:38:13AM +0100, Andrzej Pietrasiewicz wrote:
...
> +static int s5p_jpeg_s_jpegcomp(struct file *file, void *priv,
> +			       struct v4l2_jpegcompression *compr)
> +{
> +	struct s5p_jpeg_ctx *ctx = priv;
> +
> +	if (ctx->mode == S5P_JPEG_DECODE)
> +		return -ENOTTY;
> +
> +	compr->quality = clamp(compr->quality, S5P_JPEG_COMPR_QUAL_BEST,
> +			       S5P_JPEG_COMPR_QUAL_WORST);
> +
> +	ctx->compr_quality = S5P_JPEG_COMPR_QUAL_WORST - compr->quality;
> +
> +	return 0;

The quality paramaeter of VIDIOC_S_JPEGCOMP is badly documented and its
value range is unspecified. To make the matter worse, VIDIOC_S_JPEGCOMP is a
write-only IOCTL, so the user won't be able to know the value the driver
uses. This forces the user space to know the value range for quality. I
think we have a good change to resolve the matter properly now.

I can think of two alternatives, both of which are very simple.

1) Define the value range for v4l2_jpegcompression. The driver implements
four, so they essentially would be 0, 33, 66 and 100, if 0--100 is chosen as
the standard range. This is what I have seen is often used by jpeg
compression programs.

2) Define a new control for jpeg quality. Its value range can be what the
hardware supports and the user space gets much better information on the
capabilities of the hardware and the granularity of the quality setting.

I might even favour the second one. I also wonder how many user space
applications use this IOCTL, so if we're breaking anything by not supporting
it.

Or we could decide to do option 1 right now and implement 2) later on. I can
write a patch to change the documentation.

Kind regards,

-- 
Sakari Ailus
e-mail: sakari.ailus@xxxxxx	jabber/XMPP/Gmail: sailus@xxxxxxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux