All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sakari Ailus <sakari.ailus@iki.fi>
To: Prabhakar Lad <prabhakar.lad@ti.com>
Cc: LMML <linux-media@vger.kernel.org>,
	dlos <davinci-linux-open-source@linux.davincidsp.com>,
	linux-kernel@vger.kernel.org,
	Manjunath Hadli <manjunath.hadli@ti.com>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	linux-doc@vger.kernel.org, Hans Verkuil <hans.verkuil@cisco.com>,
	Sylwester Nawrocki <s.nawrocki@samsung.com>,
	Mauro Carvalho Chehab <mchehab@infradead.org>,
	Hans de Goede <hdegoede@redhat.com>,
	Kyungmin Park <kyungmin.park@samsung.com>,
	Rob Landley <rob@landley.net>
Subject: Re: [PATCH v4] media: v4l2-ctrls: add control for dpcm predictor
Date: Fri, 07 Sep 2012 21:46:44 +0300	[thread overview]
Message-ID: <504A4114.5010106@iki.fi> (raw)
In-Reply-To: <5046DEC1.6050704@ti.com>

Hi Prabhakar,

Prabhakar Lad wrote:
> Hi Sakari,
>
> Thanks for the review.
>
> On Wednesday 05 September 2012 12:42 AM, Sakari Ailus wrote:
>> Hi Prabhakar,
>>
>> Thanks for the patch. I've got a few comments below.
>>
>> On Tue, Sep 04, 2012 at 11:07:52AM +0530, Prabhakar Lad wrote:
>>> From: Lad, Prabhakar <prabhakar.lad@ti.com>
>>>
>>> add V4L2_CID_DPCM_PREDICTOR control of type menu, which
>>> determines the dpcm predictor. The predictor can be either
>>> simple or advanced.
>>>
>>> Signed-off-by: Lad, Prabhakar <prabhakar.lad@ti.com>
>>> Signed-off-by: Manjunath Hadli <manjunath.hadli@ti.com>
>>> Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
>>> Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
>>> Cc: Sakari Ailus <sakari.ailus@iki.fi>
>>> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>> Cc: Mauro Carvalho Chehab <mchehab@infradead.org>
>>> Cc: Hans de Goede <hdegoede@redhat.com>
>>> Cc: Kyungmin Park <kyungmin.park@samsung.com>
>>> Cc: Rob Landley <rob@landley.net>
>>> ---
>>> This patches has one checkpatch warning for line over
>>> 80 characters altough it can be avoided I have kept it
>>> for consistency.
>>>
>>> Changes for v4:
>>> 1: Aligned the description to fit appropriately in the
>>> para tag, pointed by Sylwester.
>>>
>>> Changes for v3:
>>> 1: Added better explanation for DPCM, pointed by Hans.
>>>
>>> Changes for v2:
>>> 1: Added documentaion in controls.xml pointed by Sylwester.
>>> 2: Chnaged V4L2_DPCM_PREDICTOR_ADVANCE to V4L2_DPCM_PREDICTOR_ADVANCED
>>>     pointed by Sakari.
>>>
>>>   Documentation/DocBook/media/v4l/controls.xml |   46 +++++++++++++++++++++++++-
>>>   drivers/media/v4l2-core/v4l2-ctrls.c         |    9 +++++
>>>   include/linux/videodev2.h                    |    5 +++
>>>   3 files changed, 59 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/Documentation/DocBook/media/v4l/controls.xml b/Documentation/DocBook/media/v4l/controls.xml
>>> index 93b9c68..ad873ea 100644
>>> --- a/Documentation/DocBook/media/v4l/controls.xml
>>> +++ b/Documentation/DocBook/media/v4l/controls.xml
>>> @@ -4267,7 +4267,51 @@ interface and may change in the future.</para>
>>>   	    pixels / second.
>>>   	    </entry>
>>>   	  </row>
>>> -	  <row><entry></entry></row>
>>> +	  <row>
>>> +	    <entry spanname="id"><constant>V4L2_CID_DPCM_PREDICTOR</constant></entry>
>>> +	    <entry>menu</entry>
>>> +	  </row>
>>> +	  <row id="v4l2-dpcm-predictor">
>>> +	    <entry spanname="descr"> Differential pulse-code modulation (DPCM) is a signal
>>> +	    encoder that uses the baseline of pulse-code modulation (PCM) but adds some
>>> +	    functionalities based on the prediction of the samples of the signal. The input
>>> +	    can be an analog signal or a digital signal.
>>> +
>>> +	    <para>If the input is a continuous-time analog signal, it needs to be sampled
>>> +	    first so that a discrete-time signal is the input to the DPCM encoder.</para>
>>> +
>>> +	    <para>Simple: take the values of two consecutive samples; if they are analog
>>> +	    samples, quantize them; calculate the difference between the first one and the
>>> +	    next; the output is the difference, and it can be further entropy coded.</para>
>>> +
>>> +	    <para>Advanced: instead of taking a difference relative to a previous input sample,
>>> +	    take the difference relative to the output of a local model of the decoder process;
>>> +	    in this option, the difference can be quantized, which allows a good way to
>>> +	    incorporate a controlled loss in the encoding.</para>
>>
>> This is directly from Wikipedia, isn't it?
>>
> Yes.
>
>> What comes to the content, DPCM in the context of V4L2 media bus codes, as a
>> digital interface, is always digital. So there's no need to document it.
>> Entropy coding is also out of the question: the samples of the currently
>> defined formats are equal in size.
>>
> Ok.

Could you replace the above with this text (with appropriate indentation 
etc.) while keeping the reference to Wikipedia?

------8<------
Differential pulse-code modulation (DPCM) compression can be used to 
compress the samples into fewer bits than they would otherwise require. 
This is done by calculating the difference between consecutive samples 
and outputting the difference which in average is much smaller than the 
values of the samples themselves since there is generally lots of 
correlation between adjacent pixels. In decompression the original 
samples are reconstructed. The process isn't lossless as the encoded 
sample size in bits is less than the original.

Formats using DPCM compression include <xref 
linkend="pixfmt-srggb10dpcm8" />.

This control is used to select the predictor used to encode the samples.

The main difference between the simple and the advanced predictors is 
image quality, with advanced predictor supposed to produce better 
quality images as a result. Simple predictor can be used e.g. for 
testing purposes.
------8<------

Kind regards,

-- 
Sakari Ailus
sakari.ailus@iki.fi

  reply	other threads:[~2012-09-07 18:45 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-04  5:37 [PATCH v4] media: v4l2-ctrls: add control for dpcm predictor Prabhakar Lad
2012-09-04 19:12 ` Sakari Ailus
2012-09-05  5:10   ` Prabhakar Lad
2012-09-07 18:46     ` Sakari Ailus [this message]
2012-09-07 19:04       ` Prabhakar Lad
2012-09-13  0:59       ` Laurent Pinchart
2012-09-18 12:49         ` Prabhakar Lad

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=504A4114.5010106@iki.fi \
    --to=sakari.ailus@iki.fi \
    --cc=davinci-linux-open-source@linux.davincidsp.com \
    --cc=hans.verkuil@cisco.com \
    --cc=hdegoede@redhat.com \
    --cc=kyungmin.park@samsung.com \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=manjunath.hadli@ti.com \
    --cc=mchehab@infradead.org \
    --cc=prabhakar.lad@ti.com \
    --cc=rob@landley.net \
    --cc=s.nawrocki@samsung.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.