Re: [PATCH v4 1/9] [media] Add signed 16-bit pixel format

[Nick Dyer <nick.dyer@itdev.co.uk>]

Hi Hans-

On 20/06/2016 12:00, Hans Verkuil wrote:
> On 06/17/2016 04:16 PM, Nick Dyer wrote:
>> This will be used for output of raw touch delta data. This format is
>> used by Atmel maXTouch (atmel_mxt_ts) and also Synaptics RMI4.
>>
>> Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
>> ---
>>  Documentation/DocBook/media/v4l/pixfmt-ys16.xml | 79 +++++++++++++++++++++++++
>>  Documentation/DocBook/media/v4l/pixfmt.xml      |  1 +
>>  drivers/media/v4l2-core/v4l2-ioctl.c            |  1 +
>>  include/uapi/linux/videodev2.h                  |  1 +
>>  4 files changed, 82 insertions(+)
>>  create mode 100644 Documentation/DocBook/media/v4l/pixfmt-ys16.xml
>>
>> diff --git a/Documentation/DocBook/media/v4l/pixfmt-ys16.xml b/Documentation/DocBook/media/v4l/pixfmt-ys16.xml
>> new file mode 100644
>> index 0000000..f92d65e
>> --- /dev/null
>> +++ b/Documentation/DocBook/media/v4l/pixfmt-ys16.xml
>> @@ -0,0 +1,79 @@
>> +<refentry id="V4L2-PIX-FMT-YS16">
>> +  <refmeta>
>> +    <refentrytitle>V4L2_PIX_FMT_YS16 ('YS16')</refentrytitle>
>> +    &manvol;
>> +  </refmeta>
>> +  <refnamediv>
>> +    <refname><constant>V4L2_PIX_FMT_YS16</constant></refname>
>> +    <refpurpose>Grey-scale image</refpurpose>
>> +  </refnamediv>
>> +  <refsect1>
>> +    <title>Description</title>
>> +
>> +    <para>This is a signed grey-scale image with a depth of 16 bits per
>> +pixel. The most significant byte is stored at higher memory addresses
>> +(little-endian).</para>
> 
> This is too generic. I think something like V4L2_TOUCH_FMT_DELTA_S16 is much
> more appropriate since this is neither luma (Y) data nor a picture in the
> classic sense. Since we already use V4L2_SDR_FMT_* defines for software defined
> radio formats, it makes sense to use V4L2_TOUCH_FMT_* for these touch panel
> formats.

OK, that sounds sensible to me.

> The description can be based around what you told here:
> 
> https://lkml.org/lkml/2016/5/27/278
> 
> It's also important that you clearly state what the delta is against. A delta
> implies a difference from something, but what that something is isn't explained.

To explain, in PCAP capacitive touch, a touch input is determined by
comparing the raw capacitance measurement to a no-touch reference (or
"baseline") measurement. Hence:

Delta = Raw - Reference

The reference measurement takes account of variations in the capacitance
characteristics across the nodes of the touch sensor, for example
manufacturing irregularities or edge effects.

I'll put something about this in the docs.

> I'm sorry for being pedantic about this, but it should be possible to make an
> application that can correctly interpret this data based on this format
> description. Otherwise there would be no point in documenting this...

No problem: thanks for your clear feedback.