All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sylwester Nawrocki <snjw23@gmail.com>
To: Sakari Ailus <sakari.ailus@iki.fi>
Cc: linux-media@vger.kernel.org, laurent.pinchart@ideasonboard.com,
	dacohen@gmail.com, andriy.shevchenko@linux.intel.com,
	t.stanislaws@samsung.com, tuukkat76@gmail.com,
	k.debski@samsung.com, riverful@gmail.com, hverkuil@xs4all.nl,
	teturtia@gmail.com
Subject: Re: [PATCH v2 04/31] v4l: VIDIOC_SUBDEV_S_SELECTION and VIDIOC_SUBDEV_G_SELECTION IOCTLs
Date: Sun, 05 Feb 2012 15:24:41 +0100	[thread overview]
Message-ID: <4F2E9129.10102@gmail.com> (raw)
In-Reply-To: <20120205090414.GB7784@valkosipuli.localdomain>

Hi Sakari,

On 02/05/2012 10:04 AM, Sakari Ailus wrote:
>>> I wanted to keep the target numbers the same since we're still using
>>> exactly the same as the V4L2.
>>
>> You're right, keeping the numbers same for subdevs and regular video
>> nodes may be important. I'm wondering whether we should use same
>> definitions for subdevs, rather than inventing new ones ? In case we
>> associate the selection targets with controls some target identifiers
>> must not actually be different. Whether the control belongs directly
>> to a video node control handler or is inherited from a sub-device the
>> selection target would have to be same. I'm referring here to an auto
>> focus rectangle selection target base for instance.
>> I guess including videodev2.h from v4l2-subdev.h is not an option, if
>> at all necessary ?
> 
> I think you're right; there would be advantages of using the same
> definitions. On the other hand, there may be subtle and not so subtle
> differences what these rectangles actually mean between the two interfaces.

Until now those analogous rectangles mean different things for subdevs and 
for video devnodes. It then doesn't matter that much what numbers identify
them, or even the numbers should be made distinct by design to make any 
attempts to use them interchangeably fail right away.

A little worrying is that with this approach we diverge from a situation
where a selection target id _do_ actually mean the same for the two 
interfaces.

Ideally I would see all the targets defined centrally with definitions that 
would be telling explicitly what is different and what is not for both
interfaces.

> The interface is quite similar to controls but the purpose it is used for is
> quite different: not many interdependencies are expected with controls
> whereas selections have many. The reason for this is we're using them to
> control various kinds of image processing functionality which might not be
> even similar on V4L2 subdev nodes and V4L2 nodes: the former is a superset
> of the latter.

I see your point. Maybe using selections with controls is a Bad Idea(tm) ? :)
However inventing a new ioctls for auto exposure/focus/wb seems a bit 
pointless..

> I'd like to have Laurent's opinion on this.

--

Regards,
Sylwester

  reply	other threads:[~2012-02-05 14:24 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-02 23:52 [PATCH v2 0/31] V4L2 subdev and sensor control changes, SMIA++ driver and N9 camera board code Sakari Ailus
2012-02-02 23:54 ` [PATCH v2 01/31] v4l: Introduce integer menu controls Sakari Ailus
2012-02-02 23:54 ` [PATCH v2 02/31] v4l: Document " Sakari Ailus
2012-02-02 23:54 ` [PATCH v2 03/31] vivi: Add an integer menu test control Sakari Ailus
2012-02-02 23:54 ` [PATCH v2 04/31] v4l: VIDIOC_SUBDEV_S_SELECTION and VIDIOC_SUBDEV_G_SELECTION IOCTLs Sakari Ailus
2012-02-04 19:02   ` Sylwester Nawrocki
2012-02-04 20:30     ` Sakari Ailus
2012-02-04 22:37       ` Sylwester Nawrocki
2012-02-05  9:04         ` Sakari Ailus
2012-02-05 14:24           ` Sylwester Nawrocki [this message]
2012-02-02 23:54 ` [PATCH v2 05/31] v4l: Support s_crop and g_crop through s/g_selection Sakari Ailus
2012-02-02 23:54 ` [PATCH v2 06/31] v4l: Add subdev selections documentation: svg and dia files Sakari Ailus
2012-02-02 23:54 ` [PATCH v2 07/31] v4l: Add subdev selections documentation Sakari Ailus
2012-02-02 23:54 ` [PATCH v2 08/31] v4l: Mark VIDIOC_SUBDEV_G_CROP and VIDIOC_SUBDEV_S_CROP obsolete Sakari Ailus
2012-02-02 23:54 ` [PATCH v2 09/31] v4l: Image source control class Sakari Ailus
2012-02-04 18:42   ` Sylwester Nawrocki
2012-02-04 20:23     ` Sakari Ailus
2012-02-02 23:54 ` [PATCH v2 10/31] v4l: Image processing " Sakari Ailus
2012-02-02 23:54 ` [PATCH v2 11/31] v4l: Document raw bayer 4CC codes Sakari Ailus
2012-02-05 12:46   ` Prabhakar Lad
2012-02-08 18:16     ` Sakari Ailus
2012-02-02 23:54 ` [PATCH v2 12/31] v4l: Add DPCM compressed formats Sakari Ailus
2012-02-02 23:54 ` [PATCH v2 13/31] media: Add link_validate() op to check links to the sink pad Sakari Ailus
2012-02-02 23:54 ` [PATCH v2 14/31] v4l: Improve sub-device documentation for pad ops Sakari Ailus
2012-02-02 23:54 ` [PATCH v2 15/31] v4l: Implement v4l2_subdev_link_validate() Sakari Ailus
2012-02-02 23:54 ` [PATCH v2 16/31] v4l: Allow changing control handler lock Sakari Ailus
2012-02-02 23:54 ` [PATCH v2 17/31] omap3isp: Support additional in-memory compressed bayer formats Sakari Ailus
2012-02-02 23:54 ` [PATCH v2 18/31] omap3isp: Move definitions required by board code under include/media Sakari Ailus
2012-02-02 23:54 ` [PATCH v2 19/31] omap3: add definition for CONTROL_CAMERA_PHY_CTRL Sakari Ailus
2012-02-02 23:54 ` [PATCH v2 20/31] omap3isp: Assume media_entity_pipeline_start may fail Sakari Ailus
2012-02-02 23:54 ` [PATCH v2 21/31] omap3isp: Add lane configuration to platform data Sakari Ailus
2012-02-02 23:54 ` [PATCH v2 22/31] omap3isp: Add information on external subdev to struct isp_pipeline Sakari Ailus
2012-02-02 23:54 ` [PATCH v2 23/31] omap3isp: Introduce omap3isp_get_external_info() Sakari Ailus
2012-02-02 23:54 ` [PATCH v2 24/31] omap3isp: Default link validation for ccp2, csi2, preview and resizer Sakari Ailus
2012-02-02 23:54 ` [PATCH v2 25/31] omap3isp: Implement proper CCDC link validation, check pixel rate Sakari Ailus
2012-02-02 23:54 ` [PATCH v2 26/31] omap3isp: Move setting constaints above media_entity_pipeline_start Sakari Ailus
2012-02-02 23:54 ` [PATCH v2 27/31] omap3isp: Configure CSI-2 phy based on platform data Sakari Ailus
2012-02-10  2:05   ` Aguirre, Sergio
2012-02-10 20:32     ` Sakari Ailus
2012-02-11 17:17       ` Aguirre, Sergio
2012-02-11 17:24         ` Aguirre, Sergio
2012-02-14  6:21           ` Sakari Ailus
2012-02-16  7:40         ` [PATCH v2.1 1/1] " Sakari Ailus
2012-02-02 23:54 ` [PATCH v2 28/31] omap3isp: Add resizer data rate configuration to resizer_set_stream Sakari Ailus
2012-02-02 23:54 ` [PATCH v2 29/31] omap3isp: Remove isp_validate_pipeline and other old stuff Sakari Ailus
2012-02-02 23:54 ` [PATCH v2 30/31] smiapp: Add driver Sakari Ailus
2012-02-02 23:54 ` [PATCH v2 31/31] rm680: Add camera init Sakari Ailus

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=4F2E9129.10102@gmail.com \
    --to=snjw23@gmail.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=dacohen@gmail.com \
    --cc=hverkuil@xs4all.nl \
    --cc=k.debski@samsung.com \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-media@vger.kernel.org \
    --cc=riverful@gmail.com \
    --cc=sakari.ailus@iki.fi \
    --cc=t.stanislaws@samsung.com \
    --cc=teturtia@gmail.com \
    --cc=tuukkat76@gmail.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.