linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Tomasz Stanislawski <t.stanislaws@samsung.com>
To: Mauro Carvalho Chehab <mchehab@redhat.com>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>,
	linux-media@vger.kernel.org,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Hans Verkuil <hverkuil@xs4all.nl>
Subject: Re: [GIT PULL] Selection API and fixes for v3.2
Date: Tue, 27 Sep 2011 18:46:10 +0200	[thread overview]
Message-ID: <4E81FDD2.3090501@samsung.com> (raw)
In-Reply-To: <4E81D93E.1060107@redhat.com>

Hi Mauro,

On 09/27/2011 04:10 PM, Mauro Carvalho Chehab wrote:
> Em 27-09-2011 10:02, Tomasz Stanislawski escreveu:
>> On 09/26/2011 02:10 PM, Mauro Carvalho Chehab wrote:
>>> Em 26-09-2011 05:42, Tomasz Stanislawski escreveu:
>>>> On 09/24/2011 05:58 AM, Mauro Carvalho Chehab wrote:
>>>>> Em 22-09-2011 12:13, Marek Szyprowski escreveu:

[snip]

>>
>> What do you mean by 'scale type'? Do you mean types like 'shrink', 'enlarge', 'no scale'?
>
> I mean: what's the scale that the application should expect for cropping? pixel, sub-mixel, percentage, etc.
>

Hans suggest to use pixels as units. I bear to a very similar but 
slightly different idea that the driver should use units system that 
guarantees that no scaling is applied while the composing and cropping 
rectangle are equal. I mean rectangle's width and height.

[snip]

>>>> The selection API introduced the idea
>>>> of the constraints flags, that are used for precise control of the coordinates' rounding policy.
>>>
>>> Ok, but I fail to see where a rounding policy would be
>>> needed on an input device.
>>
>> Please refer to following use case:
>> - there is a video capture hardware and a grabber application
>> - a face-detection is implemented in capture hardware
>> - the application obtains the position of the face using extended control
>> - application would like to grab only the face, avoiding any extra content
>> - the application configures cropping rectangle with V4L2_SEL_SIZE_GE flag to assure that no part of the face is lost
>
> On all cases I can think with for input devices, the rounding policy should be GE.
>

OK. I think I know the use case for the LE flag.
Please, analyze the following scenario:
- video capture HW - TV card
- application - controls TV streaming on the framebuffer
- the image data are inserted directly into the framebuffer
- application gets information through extended controls that 
letterboxed signal is received
- application is working in full-screen mode. There must be no black 
border on the screen.
- application sets crop using the rectangle that covers whole useful 
image (without borders)
- application uses V4L2_SEL_SIZE_LE flags while setting crop rectangle 
to assure that no black border appears on the screen

>>>
>>> In other words, I think we should split the issues with
>>> the crop api into two groups:
>>>
>>> 1) for input devices
>>> 2) for output devices.
>>>
>>
>> No. I strongly prefer to keep consistent API for both capture and output devices. We should avoid adding extra ioctls and structures.
>
> Your proposal is to add extra ioctl's and structures.

 From the point of view of developers of new drivers:
- implement s_selection and g_selection to support selection API
- implement s_crop, g_crop, cropcap to support old API

 From application point of view:
- use VIDIOC_S_SELECTION, VIDIOC_G_SELECTION, struct v4l2_selection for 
selection API
- use VIDIOC_S_CROP, VIDIOC_G_CROP, VIDIOC_CROPCAP, struct v4l2_crop, 
v4l2_cropcap for old API

If new applications and drivers support only for selection API then we 
will have:
- less ioctl
- less structures
- more functionality

The legacy applications would be supported by simulation of old API 
using selection API.

I'm not saying that we should have
> different API's for input and for outputs. I'm just telling that we need to analyze each
> case in separate.
>

Please refer to summary of discussion about pipeline configuration.
The selection API seams to suit well to both types of devices.

>> Moreover, there are mem2mem devices approaching from horizon.
>> They combine features of both types of queues. It would be much simpler for developers and application to use the same API.
>
> On complex cases, like mem2mem and devices with output queues, your proposal
> makes sense, but simpler devices can just use the crop API for their
> input nodes.

Right. I agree that old API should be kept for backward compatibility 
reasons.

BTW, till now I understood that state 'deprecated' just means to avoid 
using it in the new code.

[snip]

> Deprecating an API means that it will be removed. So, drivers will need
> to be ported, and also userspace applications.
>

Ok.. I agree that it is to early to deprecate the old API.

>>> So, please explain us why the above drivers would need to be ported to
>>> the selection API, or why new input drivers/applications would need
>>> the new API instead of the old one.
>>
>> Some of presented hardware might be capable of composing into memory buffers. This functionality is not available by the current API.
>> The support for composing operation may justify porting the drivers to the selection API.
>
> It is doubtful that any of the above hardware would support composing.
> Maybe only cx18 might have this capability, but I don't think that the
> existing devices support it anyway.
>

S5P-FIMC supports this operation. OMAP3 does it too as I know.

>>> With respect to (2), the only TV device that (ab)used the crop API to
>>> control its output node is
>>>      drivers/media/video/ivtv/ivtv-ioctl.c
>>>
>>> To complete the drivers list, currently, the only SoC device currently
>>> implementing vidioc_s_crop for input/output is the davinci driver:
>>>      drivers/media/video/davinci/vpif_capture.c
>>>      drivers/media/video/davinci/vpif_display.c
>>>
>>> IMO, what it seems to be happening with the crop API is similar to what
>>> happened in the past with the control API: the existing API works fine
>>> on simple cases, but fails on more complex scenarios. In the case of
>>> the control API, several controls need to be grouped when selecting an
>>> mpeg compression parameters. So, the VIDIOC_[G|S]_EXT_CTRLS were added
>>> without deprecating the old ioctl's. This way, applications that were
>>> only supporting controls like bright, volume, etc won't need to be changed.
>>
>> Refer to thread:
>>
>> http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/33799/focus=34727
>>
>> The concept of the simple pipeline was described there. The selection is an important part
>> of this proposition. If this concept is accepted then 'simple pipeline' becomes a new V4L2 primitive.
>> All other more complex cases would be covered by the multimedia device API. The selection is
>> cropping/composing API dedicated for such a pipelines because old one was not good enough.
>>
>> I agree that most of the selection configuration could be moved to extended control API.
>> By applying the same logic a few further one could state that the whole configuration
>> (like VIDIOC_S_FMT, VIDIOC_S_STD, etc) could be moved to the extended controls. Maybe only
>> the memory and streaming control ioctl would survive.
>
> I never said that we should be using the ext control API. I'm just saying that the selection
> API x crop API resembles the control API x ext control API.

I agree. However these ioctls are strongly connect with memory 
management in the similar way as VIDIOC_S_FMT does. Moreover the 
selection is a simple and robust API that covers a wide range of 
build-in pipelines. Due to its generality, I think it is worth to be 
promoted with dedicated ioctl.

>
> In other words, for the same reason we didn't deprecate the control API when the ext control API
> were added, I don't think that we should deprecate the crop API in favor of the selection API.
>
>> I think that it would be a good start for V4L3 project.
>
> Are you proposing that we should start a V4L3 project?????? Why?
>
> Only this year we were able of get rid of V4L1 API, after 10+ years of efforts!
> Why? Moving from V4L2 to anything else will likely take even more time, as we
> now have much more drivers to take care with.

I don't want to start V4L3 project. I just hope that if it ever happens 
then V4L3 would a completely controls-based API with well defined 
pipeline configuration rules and memory management compatible with other 
popular APIs.

Best regards,
Tomasz Stanislawski

  reply	other threads:[~2011-09-27 16:46 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-09-22 15:13 [GIT PULL] Selection API and fixes for v3.2 Marek Szyprowski
2011-09-24  3:58 ` Mauro Carvalho Chehab
2011-09-26  8:42   ` Tomasz Stanislawski
2011-09-26 12:10     ` Mauro Carvalho Chehab
2011-09-26 12:17       ` Mauro Carvalho Chehab
2011-09-27 13:02       ` Tomasz Stanislawski
2011-09-27 14:10         ` Mauro Carvalho Chehab
2011-09-27 16:46           ` Tomasz Stanislawski [this message]
2011-09-28  8:01             ` Hans Verkuil
2011-09-28  8:29               ` Laurent Pinchart
2011-09-28  9:00                 ` Hans Verkuil
2011-09-28  9:59               ` Tomasz Stanislawski
2011-09-28 10:40                 ` Mauro Carvalho Chehab
2011-09-28 15:17                   ` Tomasz Stanislawski
2011-09-28 11:20                 ` Hans Verkuil
2011-09-26 10:03   ` Marek Szyprowski
2011-09-26 11:18     ` Mauro Carvalho Chehab
2011-09-26 12:45   ` Hans Verkuil
2011-09-26 11:13 ` Mauro Carvalho Chehab
2011-09-26 11:21   ` Marek Szyprowski
2011-09-26 13:30     ` Mauro Carvalho Chehab
2011-09-26 14:41       ` Laurent Pinchart
2011-09-27  8:23       ` Marek Szyprowski
2011-09-27 11:40         ` Mauro Carvalho Chehab

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=4E81FDD2.3090501@samsung.com \
    --to=t.stanislaws@samsung.com \
    --cc=hverkuil@xs4all.nl \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-media@vger.kernel.org \
    --cc=m.szyprowski@samsung.com \
    --cc=mchehab@redhat.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).