linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Hans Verkuil <hverkuil@xs4all.nl>
To: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
Cc: "linux-media@vger.kernel.org" <linux-media@vger.kernel.org>,
	Divneil Wadhawan <divneil.wadhawan@st.com>,
	Pawel Osciak <pawel@osciak.com>,
	Marek Szyprowski <m.szyprowski@samsung.com>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Subject: Re: [PATCH] vb2: replace VIDEO_MAX_FRAME with VB2_MAX_FRAME
Date: Wed, 29 Oct 2014 11:01:24 +0100	[thread overview]
Message-ID: <5450BAF4.6050008@xs4all.nl> (raw)
In-Reply-To: <20141029071312.08aef860@recife.lan>

On 10/29/14 10:13, Mauro Carvalho Chehab wrote:
> Em Wed, 29 Oct 2014 09:59:56 +0100
> Hans Verkuil <hverkuil@xs4all.nl> escreveu:
> 
>> On 10/29/14 09:29, Mauro Carvalho Chehab wrote:
>>> Em Wed, 29 Oct 2014 08:29:08 +0100
>>> Hans Verkuil <hverkuil@xs4all.nl> escreveu:
>>>
>>>> On 10/28/2014 07:26 PM, Mauro Carvalho Chehab wrote:
>>>>> Em Fri, 10 Oct 2014 10:04:58 +0200
>>>>> Hans Verkuil <hverkuil@xs4all.nl> escreveu:
>>>>>
>>>>>> (This patch is from Divneil except for the vivid changes which I added. He had
>>>>>> difficulties posting the patch without the mailer mangling it, so I'm reposting
>>>>>> it for him)
>>>>>>
>>>>>> - vb2 drivers to rely on VB2_MAX_FRAME.
>>>>>>
>>>>>> - VB2_MAX_FRAME bumps the value to 64 from current 32
>>>>>
>>>>> Hmm... what's the point of announcing a maximum of 32 buffers to userspace,
>>>>> but using internally 64?
>>>>
>>>> Where do we announce 32 buffers?
>>>
>>> VIDEO_MAX_FRAME is defined at videodev2.h:
>>>
>>> include/uapi/linux/videodev2.h:#define VIDEO_MAX_FRAME               32
>>>
>>> So, it is part of userspace API. Yeah, I know, it sucks, but apps
>>> may be using it to limit the max number of buffers.
>>
>> So? Userspace is free to ask for 32 buffers, and it will get 32 buffers if
>> memory allows. vb2 won't be returning more than 32, so I don't see how things
>> can break.
> 
> Well, VIDEO_MAX_FRAME has nothing to do with the max VB1 support. It is
> the maximum number of buffers supported by V4L2. Properly-written apps
> will never request more than 32 buffers, because we're telling them that
> this is not supported.
> 
> So, it makes no sense to change internally to 64, but keeping announcing
> that the maximum is 32. We're just wasting memory inside the Kernel with
> no reason.

Hmm, so you think VIDEO_MAX_FRAME should just be updated to 64?

I am a bit afraid that that might break applications (especially if there
are any that use bits in a 32-bit unsigned variable). Should userspace
know about this at all? I think that the maximum number of frames is
driver dependent, and in fact one of the future vb2 improvements would be
to stop hardcoding this and leave the maximum up to the driver.

Basically I would like to deprecate VIDEO_MAX_FRAME.

Regards,

	Hans

  reply	other threads:[~2014-10-29 10:01 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-10-10  8:04 [PATCH] vb2: replace VIDEO_MAX_FRAME with VB2_MAX_FRAME Hans Verkuil
2014-10-11 17:21 ` Laurent Pinchart
2014-10-11 20:45   ` Hans Verkuil
2014-10-28 18:26 ` Mauro Carvalho Chehab
2014-10-29  7:29   ` Hans Verkuil
2014-10-29  8:29     ` Mauro Carvalho Chehab
2014-10-29  8:59       ` Hans Verkuil
2014-10-29  9:13         ` Mauro Carvalho Chehab
2014-10-29 10:01           ` Hans Verkuil [this message]
2014-10-29 12:40             ` Mauro Carvalho Chehab
2014-10-29 12:46               ` Laurent Pinchart
2014-10-29 13:05                 ` Mauro Carvalho Chehab
2014-10-29 13:17                   ` Laurent Pinchart
2014-10-29 13:53                     ` Hans Verkuil
2014-10-29 18:07                       ` 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=5450BAF4.6050008@xs4all.nl \
    --to=hverkuil@xs4all.nl \
    --cc=divneil.wadhawan@st.com \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-media@vger.kernel.org \
    --cc=m.szyprowski@samsung.com \
    --cc=mchehab@osg.samsung.com \
    --cc=pawel@osciak.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).