[PATCH] vb2: replace VIDEO_MAX_FRAME with VB2_MAX_FRAME

[PATCH] vb2: replace VIDEO_MAX_FRAME with VB2_MAX_FRAME

[Hans Verkuil]

(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

Signed-off-by: Divneil Wadhawan <divneil.wadhawan@st.com>
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
  drivers/media/pci/saa7134/saa7134-ts.c       | 4 ++--
  drivers/media/pci/saa7134/saa7134-vbi.c      | 4 ++--
  drivers/media/pci/saa7134/saa7134-video.c    | 2 +-
  drivers/media/platform/mem2mem_testdev.c     | 2 +-
  drivers/media/platform/ti-vpe/vpe.c          | 2 +-
  drivers/media/platform/vivid/vivid-core.h    | 2 +-
  drivers/media/platform/vivid/vivid-ctrls.c   | 2 +-
  drivers/media/platform/vivid/vivid-vid-cap.c | 2 +-
  drivers/media/v4l2-core/videobuf2-core.c     | 8 ++++----
  include/media/videobuf2-core.h               | 4 +++-
  10 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/drivers/media/pci/saa7134/saa7134-ts.c b/drivers/media/pci/saa7134/saa7134-ts.c
index bd25323..0d04995 100644
--- a/drivers/media/pci/saa7134/saa7134-ts.c
+++ b/drivers/media/pci/saa7134/saa7134-ts.c
@@ -227,8 +227,8 @@ int saa7134_ts_init1(struct saa7134_dev *dev)
  	/* sanitycheck insmod options */
  	if (tsbufs < 2)
  		tsbufs = 2;
-	if (tsbufs > VIDEO_MAX_FRAME)
-		tsbufs = VIDEO_MAX_FRAME;
+	if (tsbufs > VB2_MAX_FRAME)
+		tsbufs = VB2_MAX_FRAME;
  	if (ts_nr_packets < 4)
  		ts_nr_packets = 4;
  	if (ts_nr_packets > 312)
diff --git a/drivers/media/pci/saa7134/saa7134-vbi.c b/drivers/media/pci/saa7134/saa7134-vbi.c
index 4f0b101..2269837 100644
--- a/drivers/media/pci/saa7134/saa7134-vbi.c
+++ b/drivers/media/pci/saa7134/saa7134-vbi.c
@@ -203,8 +203,8 @@ int saa7134_vbi_init1(struct saa7134_dev *dev)
  
  	if (vbibufs < 2)
  		vbibufs = 2;
-	if (vbibufs > VIDEO_MAX_FRAME)
-		vbibufs = VIDEO_MAX_FRAME;
+	if (vbibufs > VB2_MAX_FRAME)
+		vbibufs = VB2_MAX_FRAME;
  	return 0;
  }
  
diff --git a/drivers/media/pci/saa7134/saa7134-video.c b/drivers/media/pci/saa7134/saa7134-video.c
index fc4a427..c7f39be 100644
--- a/drivers/media/pci/saa7134/saa7134-video.c
+++ b/drivers/media/pci/saa7134/saa7134-video.c
@@ -2030,7 +2030,7 @@ int saa7134_video_init1(struct saa7134_dev *dev)
  	int ret;
  
  	/* sanitycheck insmod options */
-	if (gbuffers < 2 || gbuffers > VIDEO_MAX_FRAME)
+	if (gbuffers < 2 || gbuffers > VB2_MAX_FRAME)
  		gbuffers = 2;
  	if (gbufsize > gbufsize_max)
  		gbufsize = gbufsize_max;
diff --git a/drivers/media/platform/mem2mem_testdev.c b/drivers/media/platform/mem2mem_testdev.c
index c1b03cf..e1ff7e0 100644
--- a/drivers/media/platform/mem2mem_testdev.c
+++ b/drivers/media/platform/mem2mem_testdev.c
@@ -55,7 +55,7 @@ MODULE_PARM_DESC(debug, "activates debug info");
  #define MEM2MEM_NAME		"m2m-testdev"
  
  /* Per queue */
-#define MEM2MEM_DEF_NUM_BUFS	VIDEO_MAX_FRAME
+#define MEM2MEM_DEF_NUM_BUFS	VB2_MAX_FRAME
  /* In bytes, per queue */
  #define MEM2MEM_VID_MEM_LIMIT	(16 * 1024 * 1024)
  
diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c
index 9a081c2..04e37c0 100644
--- a/drivers/media/platform/ti-vpe/vpe.c
+++ b/drivers/media/platform/ti-vpe/vpe.c
@@ -1971,7 +1971,7 @@ static const struct v4l2_ctrl_config vpe_bufs_per_job = {
  	.type = V4L2_CTRL_TYPE_INTEGER,
  	.def = VPE_DEF_BUFS_PER_JOB,
  	.min = 1,
-	.max = VIDEO_MAX_FRAME,
+	.max = VB2_MAX_FRAME,
  	.step = 1,
  };
  
diff --git a/drivers/media/platform/vivid/vivid-core.h b/drivers/media/platform/vivid/vivid-core.h
index 811c286..c0375a1 100644
--- a/drivers/media/platform/vivid/vivid-core.h
+++ b/drivers/media/platform/vivid/vivid-core.h
@@ -346,7 +346,7 @@ struct vivid_dev {
  	/* video capture */
  	struct tpg_data			tpg;
  	unsigned			ms_vid_cap;
-	bool				must_blank[VIDEO_MAX_FRAME];
+	bool				must_blank[VB2_MAX_FRAME];
  
  	const struct vivid_fmt		*fmt_cap;
  	struct v4l2_fract		timeperframe_vid_cap;
diff --git a/drivers/media/platform/vivid/vivid-ctrls.c b/drivers/media/platform/vivid/vivid-ctrls.c
index d5cbf00..7162f97 100644
--- a/drivers/media/platform/vivid/vivid-ctrls.c
+++ b/drivers/media/platform/vivid/vivid-ctrls.c
@@ -332,7 +332,7 @@ static int vivid_vid_cap_s_ctrl(struct v4l2_ctrl *ctrl)
  		break;
  	case VIVID_CID_PERCENTAGE_FILL:
  		tpg_s_perc_fill(&dev->tpg, ctrl->val);
-		for (i = 0; i < VIDEO_MAX_FRAME; i++)
+		for (i = 0; i < VB2_MAX_FRAME; i++)
  			dev->must_blank[i] = ctrl->val < 100;
  		break;
  	case VIVID_CID_INSERT_SAV:
diff --git a/drivers/media/platform/vivid/vivid-vid-cap.c b/drivers/media/platform/vivid/vivid-vid-cap.c
index 331c544..55e8158 100644
--- a/drivers/media/platform/vivid/vivid-vid-cap.c
+++ b/drivers/media/platform/vivid/vivid-vid-cap.c
@@ -252,7 +252,7 @@ static int vid_cap_start_streaming(struct vb2_queue *vq, unsigned count)
  
  	dev->vid_cap_seq_count = 0;
  	dprintk(dev, 1, "%s\n", __func__);
-	for (i = 0; i < VIDEO_MAX_FRAME; i++)
+	for (i = 0; i < VB2_MAX_FRAME; i++)
  		dev->must_blank[i] = tpg_g_perc_fill(&dev->tpg) < 100;
  	if (dev->start_streaming_error) {
  		dev->start_streaming_error = false;
diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
index 15b02f9..60354b4 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -911,7 +911,7 @@ static int __reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
  	/*
  	 * Make sure the requested values and current defaults are sane.
  	 */
-	num_buffers = min_t(unsigned int, req->count, VIDEO_MAX_FRAME);
+	num_buffers = min_t(unsigned int, req->count, VB2_MAX_FRAME);
  	num_buffers = max_t(unsigned int, num_buffers, q->min_buffers_needed);
  	memset(q->plane_sizes, 0, sizeof(q->plane_sizes));
  	memset(q->alloc_ctx, 0, sizeof(q->alloc_ctx));
@@ -1015,7 +1015,7 @@ static int __create_bufs(struct vb2_queue *q, struct v4l2_create_buffers *create
  	unsigned int num_planes = 0, num_buffers, allocated_buffers;
  	int ret;
  
-	if (q->num_buffers == VIDEO_MAX_FRAME) {
+	if (q->num_buffers == VB2_MAX_FRAME) {
  		dprintk(1, "maximum number of buffers already allocated\n");
  		return -ENOBUFS;
  	}
@@ -1026,7 +1026,7 @@ static int __create_bufs(struct vb2_queue *q, struct v4l2_create_buffers *create
  		q->memory = create->memory;
  	}
  
-	num_buffers = min(create->count, VIDEO_MAX_FRAME - q->num_buffers);
+	num_buffers = min(create->count, VB2_MAX_FRAME - q->num_buffers);
  
  	/*
  	 * Ask the driver, whether the requested number of buffers, planes per
@@ -2725,7 +2725,7 @@ struct vb2_fileio_data {
  	struct v4l2_requestbuffers req;
  	struct v4l2_plane p;
  	struct v4l2_buffer b;
-	struct vb2_fileio_buf bufs[VIDEO_MAX_FRAME];
+	struct vb2_fileio_buf bufs[VB2_MAX_FRAME];
  	unsigned int cur_index;
  	unsigned int initial_index;
  	unsigned int q_count;
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index a8608ce..66dcc40 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -18,6 +18,8 @@
  #include <linux/videodev2.h>
  #include <linux/dma-buf.h>
  
+#define VB2_MAX_FRAME		64
+
  struct vb2_alloc_ctx;
  struct vb2_fileio_data;
  struct vb2_threadio_data;
@@ -402,7 +404,7 @@ struct vb2_queue {
  /* private: internal use only */
  	struct mutex			mmap_lock;
  	enum v4l2_memory		memory;
-	struct vb2_buffer		*bufs[VIDEO_MAX_FRAME];
+	struct vb2_buffer		*bufs[VB2_MAX_FRAME];
  	unsigned int			num_buffers;
  
  	struct list_head		queued_list;
-- 
2.1.1

Re: [PATCH] vb2: replace VIDEO_MAX_FRAME with VB2_MAX_FRAME

[Laurent Pinchart]

Hi Hans,

On Friday 10 October 2014 10:04:58 Hans Verkuil wrote:
> (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

Just curious, in which use case(s) is 32 frames too little ?

By the way, should we discuss at some point a way to remove that limitation 
and manage buffers fully dynamically ? That's something we would need for a 
VIDIOC_DELETE_BUFS ioctl.

> Signed-off-by: Divneil Wadhawan <divneil.wadhawan@st.com>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> ---
>   drivers/media/pci/saa7134/saa7134-ts.c       | 4 ++--
>   drivers/media/pci/saa7134/saa7134-vbi.c      | 4 ++--
>   drivers/media/pci/saa7134/saa7134-video.c    | 2 +-
>   drivers/media/platform/mem2mem_testdev.c     | 2 +-
>   drivers/media/platform/ti-vpe/vpe.c          | 2 +-
>   drivers/media/platform/vivid/vivid-core.h    | 2 +-
>   drivers/media/platform/vivid/vivid-ctrls.c   | 2 +-
>   drivers/media/platform/vivid/vivid-vid-cap.c | 2 +-
>   drivers/media/v4l2-core/videobuf2-core.c     | 8 ++++----
>   include/media/videobuf2-core.h               | 4 +++-
>   10 files changed, 17 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/media/pci/saa7134/saa7134-ts.c
> b/drivers/media/pci/saa7134/saa7134-ts.c index bd25323..0d04995 100644
> --- a/drivers/media/pci/saa7134/saa7134-ts.c
> +++ b/drivers/media/pci/saa7134/saa7134-ts.c
> @@ -227,8 +227,8 @@ int saa7134_ts_init1(struct saa7134_dev *dev)
>   	/* sanitycheck insmod options */
>   	if (tsbufs < 2)
>   		tsbufs = 2;
> -	if (tsbufs > VIDEO_MAX_FRAME)
> -		tsbufs = VIDEO_MAX_FRAME;
> +	if (tsbufs > VB2_MAX_FRAME)
> +		tsbufs = VB2_MAX_FRAME;
>   	if (ts_nr_packets < 4)
>   		ts_nr_packets = 4;
>   	if (ts_nr_packets > 312)
> diff --git a/drivers/media/pci/saa7134/saa7134-vbi.c
> b/drivers/media/pci/saa7134/saa7134-vbi.c index 4f0b101..2269837 100644
> --- a/drivers/media/pci/saa7134/saa7134-vbi.c
> +++ b/drivers/media/pci/saa7134/saa7134-vbi.c
> @@ -203,8 +203,8 @@ int saa7134_vbi_init1(struct saa7134_dev *dev)
> 
>   	if (vbibufs < 2)
>   		vbibufs = 2;
> -	if (vbibufs > VIDEO_MAX_FRAME)
> -		vbibufs = VIDEO_MAX_FRAME;
> +	if (vbibufs > VB2_MAX_FRAME)
> +		vbibufs = VB2_MAX_FRAME;
>   	return 0;
>   }
> 
> diff --git a/drivers/media/pci/saa7134/saa7134-video.c
> b/drivers/media/pci/saa7134/saa7134-video.c index fc4a427..c7f39be 100644
> --- a/drivers/media/pci/saa7134/saa7134-video.c
> +++ b/drivers/media/pci/saa7134/saa7134-video.c
> @@ -2030,7 +2030,7 @@ int saa7134_video_init1(struct saa7134_dev *dev)
>   	int ret;
> 
>   	/* sanitycheck insmod options */
> -	if (gbuffers < 2 || gbuffers > VIDEO_MAX_FRAME)
> +	if (gbuffers < 2 || gbuffers > VB2_MAX_FRAME)
>   		gbuffers = 2;
>   	if (gbufsize > gbufsize_max)
>   		gbufsize = gbufsize_max;
> diff --git a/drivers/media/platform/mem2mem_testdev.c
> b/drivers/media/platform/mem2mem_testdev.c index c1b03cf..e1ff7e0 100644
> --- a/drivers/media/platform/mem2mem_testdev.c
> +++ b/drivers/media/platform/mem2mem_testdev.c
> @@ -55,7 +55,7 @@ MODULE_PARM_DESC(debug, "activates debug info");
>   #define MEM2MEM_NAME		"m2m-testdev"
> 
>   /* Per queue */
> -#define MEM2MEM_DEF_NUM_BUFS	VIDEO_MAX_FRAME
> +#define MEM2MEM_DEF_NUM_BUFS	VB2_MAX_FRAME
>   /* In bytes, per queue */
>   #define MEM2MEM_VID_MEM_LIMIT	(16 * 1024 * 1024)
> 
> diff --git a/drivers/media/platform/ti-vpe/vpe.c
> b/drivers/media/platform/ti-vpe/vpe.c index 9a081c2..04e37c0 100644
> --- a/drivers/media/platform/ti-vpe/vpe.c
> +++ b/drivers/media/platform/ti-vpe/vpe.c
> @@ -1971,7 +1971,7 @@ static const struct v4l2_ctrl_config vpe_bufs_per_job
> = { .type = V4L2_CTRL_TYPE_INTEGER,
>   	.def = VPE_DEF_BUFS_PER_JOB,
>   	.min = 1,
> -	.max = VIDEO_MAX_FRAME,
> +	.max = VB2_MAX_FRAME,
>   	.step = 1,
>   };
> 
> diff --git a/drivers/media/platform/vivid/vivid-core.h
> b/drivers/media/platform/vivid/vivid-core.h index 811c286..c0375a1 100644
> --- a/drivers/media/platform/vivid/vivid-core.h
> +++ b/drivers/media/platform/vivid/vivid-core.h
> @@ -346,7 +346,7 @@ struct vivid_dev {
>   	/* video capture */
>   	struct tpg_data			tpg;
>   	unsigned			ms_vid_cap;
> -	bool				must_blank[VIDEO_MAX_FRAME];
> +	bool				must_blank[VB2_MAX_FRAME];
> 
>   	const struct vivid_fmt		*fmt_cap;
>   	struct v4l2_fract		timeperframe_vid_cap;
> diff --git a/drivers/media/platform/vivid/vivid-ctrls.c
> b/drivers/media/platform/vivid/vivid-ctrls.c index d5cbf00..7162f97 100644
> --- a/drivers/media/platform/vivid/vivid-ctrls.c
> +++ b/drivers/media/platform/vivid/vivid-ctrls.c
> @@ -332,7 +332,7 @@ static int vivid_vid_cap_s_ctrl(struct v4l2_ctrl *ctrl)
>   		break;
>   	case VIVID_CID_PERCENTAGE_FILL:
>   		tpg_s_perc_fill(&dev->tpg, ctrl->val);
> -		for (i = 0; i < VIDEO_MAX_FRAME; i++)
> +		for (i = 0; i < VB2_MAX_FRAME; i++)
>   			dev->must_blank[i] = ctrl->val < 100;
>   		break;
>   	case VIVID_CID_INSERT_SAV:
> diff --git a/drivers/media/platform/vivid/vivid-vid-cap.c
> b/drivers/media/platform/vivid/vivid-vid-cap.c index 331c544..55e8158
> 100644
> --- a/drivers/media/platform/vivid/vivid-vid-cap.c
> +++ b/drivers/media/platform/vivid/vivid-vid-cap.c
> @@ -252,7 +252,7 @@ static int vid_cap_start_streaming(struct vb2_queue *vq,
> unsigned count)
> 
>   	dev->vid_cap_seq_count = 0;
>   	dprintk(dev, 1, "%s\n", __func__);
> -	for (i = 0; i < VIDEO_MAX_FRAME; i++)
> +	for (i = 0; i < VB2_MAX_FRAME; i++)
>   		dev->must_blank[i] = tpg_g_perc_fill(&dev->tpg) < 100;
>   	if (dev->start_streaming_error) {
>   		dev->start_streaming_error = false;
> diff --git a/drivers/media/v4l2-core/videobuf2-core.c
> b/drivers/media/v4l2-core/videobuf2-core.c index 15b02f9..60354b4 100644
> --- a/drivers/media/v4l2-core/videobuf2-core.c
> +++ b/drivers/media/v4l2-core/videobuf2-core.c
> @@ -911,7 +911,7 @@ static int __reqbufs(struct vb2_queue *q, struct
> v4l2_requestbuffers *req) /*
>   	 * Make sure the requested values and current defaults are sane.
>   	 */
> -	num_buffers = min_t(unsigned int, req->count, VIDEO_MAX_FRAME);
> +	num_buffers = min_t(unsigned int, req->count, VB2_MAX_FRAME);
>   	num_buffers = max_t(unsigned int, num_buffers, q->min_buffers_needed);
>   	memset(q->plane_sizes, 0, sizeof(q->plane_sizes));
>   	memset(q->alloc_ctx, 0, sizeof(q->alloc_ctx));
> @@ -1015,7 +1015,7 @@ static int __create_bufs(struct vb2_queue *q, struct
> v4l2_create_buffers *create unsigned int num_planes = 0, num_buffers,
> allocated_buffers;
>   	int ret;
> 
> -	if (q->num_buffers == VIDEO_MAX_FRAME) {
> +	if (q->num_buffers == VB2_MAX_FRAME) {
>   		dprintk(1, "maximum number of buffers already allocated\n");
>   		return -ENOBUFS;
>   	}
> @@ -1026,7 +1026,7 @@ static int __create_bufs(struct vb2_queue *q, struct
> v4l2_create_buffers *create q->memory = create->memory;
>   	}
> 
> -	num_buffers = min(create->count, VIDEO_MAX_FRAME - q->num_buffers);
> +	num_buffers = min(create->count, VB2_MAX_FRAME - q->num_buffers);
> 
>   	/*
>   	 * Ask the driver, whether the requested number of buffers, planes per
> @@ -2725,7 +2725,7 @@ struct vb2_fileio_data {
>   	struct v4l2_requestbuffers req;
>   	struct v4l2_plane p;
>   	struct v4l2_buffer b;
> -	struct vb2_fileio_buf bufs[VIDEO_MAX_FRAME];
> +	struct vb2_fileio_buf bufs[VB2_MAX_FRAME];
>   	unsigned int cur_index;
>   	unsigned int initial_index;
>   	unsigned int q_count;
> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> index a8608ce..66dcc40 100644
> --- a/include/media/videobuf2-core.h
> +++ b/include/media/videobuf2-core.h
> @@ -18,6 +18,8 @@
>   #include <linux/videodev2.h>
>   #include <linux/dma-buf.h>
> 
> +#define VB2_MAX_FRAME		64
> +
>   struct vb2_alloc_ctx;
>   struct vb2_fileio_data;
>   struct vb2_threadio_data;
> @@ -402,7 +404,7 @@ struct vb2_queue {
>   /* private: internal use only */
>   	struct mutex			mmap_lock;
>   	enum v4l2_memory		memory;
> -	struct vb2_buffer		*bufs[VIDEO_MAX_FRAME];
> +	struct vb2_buffer		*bufs[VB2_MAX_FRAME];
>   	unsigned int			num_buffers;
> 
>   	struct list_head		queued_list;

-- 
Regards,

Laurent Pinchart

Re: [PATCH] vb2: replace VIDEO_MAX_FRAME with VB2_MAX_FRAME

[Hans Verkuil]

Hi Laurent,

On 10/11/2014 07:21 PM, Laurent Pinchart wrote:
> Hi Hans,
>
> On Friday 10 October 2014 10:04:58 Hans Verkuil wrote:
>> (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
>
> Just curious, in which use case(s) is 32 frames too little ?

See http://www.spinics.net/lists/linux-media/msg76316.html.

>
> By the way, should we discuss at some point a way to remove that limitation
> and manage buffers fully dynamically ? That's something we would need for a
> VIDIOC_DELETE_BUFS ioctl.

It would be sufficient for this if the driver can pass on how many buffers
should be allocated, rather than it being a vb2 hardcoded size. It's not a
real issue yet, but the next time we need to increase, then we certainly
need to go for a better solution.

Regards,

	Hans

>
>> Signed-off-by: Divneil Wadhawan <divneil.wadhawan@st.com>
>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>> ---
>>    drivers/media/pci/saa7134/saa7134-ts.c       | 4 ++--
>>    drivers/media/pci/saa7134/saa7134-vbi.c      | 4 ++--
>>    drivers/media/pci/saa7134/saa7134-video.c    | 2 +-
>>    drivers/media/platform/mem2mem_testdev.c     | 2 +-
>>    drivers/media/platform/ti-vpe/vpe.c          | 2 +-
>>    drivers/media/platform/vivid/vivid-core.h    | 2 +-
>>    drivers/media/platform/vivid/vivid-ctrls.c   | 2 +-
>>    drivers/media/platform/vivid/vivid-vid-cap.c | 2 +-
>>    drivers/media/v4l2-core/videobuf2-core.c     | 8 ++++----
>>    include/media/videobuf2-core.h               | 4 +++-
>>    10 files changed, 17 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/media/pci/saa7134/saa7134-ts.c
>> b/drivers/media/pci/saa7134/saa7134-ts.c index bd25323..0d04995 100644
>> --- a/drivers/media/pci/saa7134/saa7134-ts.c
>> +++ b/drivers/media/pci/saa7134/saa7134-ts.c
>> @@ -227,8 +227,8 @@ int saa7134_ts_init1(struct saa7134_dev *dev)
>>    	/* sanitycheck insmod options */
>>    	if (tsbufs < 2)
>>    		tsbufs = 2;
>> -	if (tsbufs > VIDEO_MAX_FRAME)
>> -		tsbufs = VIDEO_MAX_FRAME;
>> +	if (tsbufs > VB2_MAX_FRAME)
>> +		tsbufs = VB2_MAX_FRAME;
>>    	if (ts_nr_packets < 4)
>>    		ts_nr_packets = 4;
>>    	if (ts_nr_packets > 312)
>> diff --git a/drivers/media/pci/saa7134/saa7134-vbi.c
>> b/drivers/media/pci/saa7134/saa7134-vbi.c index 4f0b101..2269837 100644
>> --- a/drivers/media/pci/saa7134/saa7134-vbi.c
>> +++ b/drivers/media/pci/saa7134/saa7134-vbi.c
>> @@ -203,8 +203,8 @@ int saa7134_vbi_init1(struct saa7134_dev *dev)
>>
>>    	if (vbibufs < 2)
>>    		vbibufs = 2;
>> -	if (vbibufs > VIDEO_MAX_FRAME)
>> -		vbibufs = VIDEO_MAX_FRAME;
>> +	if (vbibufs > VB2_MAX_FRAME)
>> +		vbibufs = VB2_MAX_FRAME;
>>    	return 0;
>>    }
>>
>> diff --git a/drivers/media/pci/saa7134/saa7134-video.c
>> b/drivers/media/pci/saa7134/saa7134-video.c index fc4a427..c7f39be 100644
>> --- a/drivers/media/pci/saa7134/saa7134-video.c
>> +++ b/drivers/media/pci/saa7134/saa7134-video.c
>> @@ -2030,7 +2030,7 @@ int saa7134_video_init1(struct saa7134_dev *dev)
>>    	int ret;
>>
>>    	/* sanitycheck insmod options */
>> -	if (gbuffers < 2 || gbuffers > VIDEO_MAX_FRAME)
>> +	if (gbuffers < 2 || gbuffers > VB2_MAX_FRAME)
>>    		gbuffers = 2;
>>    	if (gbufsize > gbufsize_max)
>>    		gbufsize = gbufsize_max;
>> diff --git a/drivers/media/platform/mem2mem_testdev.c
>> b/drivers/media/platform/mem2mem_testdev.c index c1b03cf..e1ff7e0 100644
>> --- a/drivers/media/platform/mem2mem_testdev.c
>> +++ b/drivers/media/platform/mem2mem_testdev.c
>> @@ -55,7 +55,7 @@ MODULE_PARM_DESC(debug, "activates debug info");
>>    #define MEM2MEM_NAME		"m2m-testdev"
>>
>>    /* Per queue */
>> -#define MEM2MEM_DEF_NUM_BUFS	VIDEO_MAX_FRAME
>> +#define MEM2MEM_DEF_NUM_BUFS	VB2_MAX_FRAME
>>    /* In bytes, per queue */
>>    #define MEM2MEM_VID_MEM_LIMIT	(16 * 1024 * 1024)
>>
>> diff --git a/drivers/media/platform/ti-vpe/vpe.c
>> b/drivers/media/platform/ti-vpe/vpe.c index 9a081c2..04e37c0 100644
>> --- a/drivers/media/platform/ti-vpe/vpe.c
>> +++ b/drivers/media/platform/ti-vpe/vpe.c
>> @@ -1971,7 +1971,7 @@ static const struct v4l2_ctrl_config vpe_bufs_per_job
>> = { .type = V4L2_CTRL_TYPE_INTEGER,
>>    	.def = VPE_DEF_BUFS_PER_JOB,
>>    	.min = 1,
>> -	.max = VIDEO_MAX_FRAME,
>> +	.max = VB2_MAX_FRAME,
>>    	.step = 1,
>>    };
>>
>> diff --git a/drivers/media/platform/vivid/vivid-core.h
>> b/drivers/media/platform/vivid/vivid-core.h index 811c286..c0375a1 100644
>> --- a/drivers/media/platform/vivid/vivid-core.h
>> +++ b/drivers/media/platform/vivid/vivid-core.h
>> @@ -346,7 +346,7 @@ struct vivid_dev {
>>    	/* video capture */
>>    	struct tpg_data			tpg;
>>    	unsigned			ms_vid_cap;
>> -	bool				must_blank[VIDEO_MAX_FRAME];
>> +	bool				must_blank[VB2_MAX_FRAME];
>>
>>    	const struct vivid_fmt		*fmt_cap;
>>    	struct v4l2_fract		timeperframe_vid_cap;
>> diff --git a/drivers/media/platform/vivid/vivid-ctrls.c
>> b/drivers/media/platform/vivid/vivid-ctrls.c index d5cbf00..7162f97 100644
>> --- a/drivers/media/platform/vivid/vivid-ctrls.c
>> +++ b/drivers/media/platform/vivid/vivid-ctrls.c
>> @@ -332,7 +332,7 @@ static int vivid_vid_cap_s_ctrl(struct v4l2_ctrl *ctrl)
>>    		break;
>>    	case VIVID_CID_PERCENTAGE_FILL:
>>    		tpg_s_perc_fill(&dev->tpg, ctrl->val);
>> -		for (i = 0; i < VIDEO_MAX_FRAME; i++)
>> +		for (i = 0; i < VB2_MAX_FRAME; i++)
>>    			dev->must_blank[i] = ctrl->val < 100;
>>    		break;
>>    	case VIVID_CID_INSERT_SAV:
>> diff --git a/drivers/media/platform/vivid/vivid-vid-cap.c
>> b/drivers/media/platform/vivid/vivid-vid-cap.c index 331c544..55e8158
>> 100644
>> --- a/drivers/media/platform/vivid/vivid-vid-cap.c
>> +++ b/drivers/media/platform/vivid/vivid-vid-cap.c
>> @@ -252,7 +252,7 @@ static int vid_cap_start_streaming(struct vb2_queue *vq,
>> unsigned count)
>>
>>    	dev->vid_cap_seq_count = 0;
>>    	dprintk(dev, 1, "%s\n", __func__);
>> -	for (i = 0; i < VIDEO_MAX_FRAME; i++)
>> +	for (i = 0; i < VB2_MAX_FRAME; i++)
>>    		dev->must_blank[i] = tpg_g_perc_fill(&dev->tpg) < 100;
>>    	if (dev->start_streaming_error) {
>>    		dev->start_streaming_error = false;
>> diff --git a/drivers/media/v4l2-core/videobuf2-core.c
>> b/drivers/media/v4l2-core/videobuf2-core.c index 15b02f9..60354b4 100644
>> --- a/drivers/media/v4l2-core/videobuf2-core.c
>> +++ b/drivers/media/v4l2-core/videobuf2-core.c
>> @@ -911,7 +911,7 @@ static int __reqbufs(struct vb2_queue *q, struct
>> v4l2_requestbuffers *req) /*
>>    	 * Make sure the requested values and current defaults are sane.
>>    	 */
>> -	num_buffers = min_t(unsigned int, req->count, VIDEO_MAX_FRAME);
>> +	num_buffers = min_t(unsigned int, req->count, VB2_MAX_FRAME);
>>    	num_buffers = max_t(unsigned int, num_buffers, q->min_buffers_needed);
>>    	memset(q->plane_sizes, 0, sizeof(q->plane_sizes));
>>    	memset(q->alloc_ctx, 0, sizeof(q->alloc_ctx));
>> @@ -1015,7 +1015,7 @@ static int __create_bufs(struct vb2_queue *q, struct
>> v4l2_create_buffers *create unsigned int num_planes = 0, num_buffers,
>> allocated_buffers;
>>    	int ret;
>>
>> -	if (q->num_buffers == VIDEO_MAX_FRAME) {
>> +	if (q->num_buffers == VB2_MAX_FRAME) {
>>    		dprintk(1, "maximum number of buffers already allocated\n");
>>    		return -ENOBUFS;
>>    	}
>> @@ -1026,7 +1026,7 @@ static int __create_bufs(struct vb2_queue *q, struct
>> v4l2_create_buffers *create q->memory = create->memory;
>>    	}
>>
>> -	num_buffers = min(create->count, VIDEO_MAX_FRAME - q->num_buffers);
>> +	num_buffers = min(create->count, VB2_MAX_FRAME - q->num_buffers);
>>
>>    	/*
>>    	 * Ask the driver, whether the requested number of buffers, planes per
>> @@ -2725,7 +2725,7 @@ struct vb2_fileio_data {
>>    	struct v4l2_requestbuffers req;
>>    	struct v4l2_plane p;
>>    	struct v4l2_buffer b;
>> -	struct vb2_fileio_buf bufs[VIDEO_MAX_FRAME];
>> +	struct vb2_fileio_buf bufs[VB2_MAX_FRAME];
>>    	unsigned int cur_index;
>>    	unsigned int initial_index;
>>    	unsigned int q_count;
>> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
>> index a8608ce..66dcc40 100644
>> --- a/include/media/videobuf2-core.h
>> +++ b/include/media/videobuf2-core.h
>> @@ -18,6 +18,8 @@
>>    #include <linux/videodev2.h>
>>    #include <linux/dma-buf.h>
>>
>> +#define VB2_MAX_FRAME		64
>> +
>>    struct vb2_alloc_ctx;
>>    struct vb2_fileio_data;
>>    struct vb2_threadio_data;
>> @@ -402,7 +404,7 @@ struct vb2_queue {
>>    /* private: internal use only */
>>    	struct mutex			mmap_lock;
>>    	enum v4l2_memory		memory;
>> -	struct vb2_buffer		*bufs[VIDEO_MAX_FRAME];
>> +	struct vb2_buffer		*bufs[VB2_MAX_FRAME];
>>    	unsigned int			num_buffers;
>>
>>    	struct list_head		queued_list;
>

Re: [PATCH] vb2: replace VIDEO_MAX_FRAME with VB2_MAX_FRAME

[Mauro Carvalho Chehab]

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?

Btw, wouldn't this change break for DaVinci:
	include/media/davinci/vpfe_capture.h:   u8 *fbuffers[VIDEO_MAX_FRAME];

Regards,
Mauro

> 
> Signed-off-by: Divneil Wadhawan <divneil.wadhawan@st.com>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> ---
>   drivers/media/pci/saa7134/saa7134-ts.c       | 4 ++--
>   drivers/media/pci/saa7134/saa7134-vbi.c      | 4 ++--
>   drivers/media/pci/saa7134/saa7134-video.c    | 2 +-
>   drivers/media/platform/mem2mem_testdev.c     | 2 +-
>   drivers/media/platform/ti-vpe/vpe.c          | 2 +-
>   drivers/media/platform/vivid/vivid-core.h    | 2 +-
>   drivers/media/platform/vivid/vivid-ctrls.c   | 2 +-
>   drivers/media/platform/vivid/vivid-vid-cap.c | 2 +-
>   drivers/media/v4l2-core/videobuf2-core.c     | 8 ++++----
>   include/media/videobuf2-core.h               | 4 +++-
>   10 files changed, 17 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/media/pci/saa7134/saa7134-ts.c b/drivers/media/pci/saa7134/saa7134-ts.c
> index bd25323..0d04995 100644
> --- a/drivers/media/pci/saa7134/saa7134-ts.c
> +++ b/drivers/media/pci/saa7134/saa7134-ts.c
> @@ -227,8 +227,8 @@ int saa7134_ts_init1(struct saa7134_dev *dev)
>   	/* sanitycheck insmod options */
>   	if (tsbufs < 2)
>   		tsbufs = 2;
> -	if (tsbufs > VIDEO_MAX_FRAME)
> -		tsbufs = VIDEO_MAX_FRAME;
> +	if (tsbufs > VB2_MAX_FRAME)
> +		tsbufs = VB2_MAX_FRAME;
>   	if (ts_nr_packets < 4)
>   		ts_nr_packets = 4;
>   	if (ts_nr_packets > 312)
> diff --git a/drivers/media/pci/saa7134/saa7134-vbi.c b/drivers/media/pci/saa7134/saa7134-vbi.c
> index 4f0b101..2269837 100644
> --- a/drivers/media/pci/saa7134/saa7134-vbi.c
> +++ b/drivers/media/pci/saa7134/saa7134-vbi.c
> @@ -203,8 +203,8 @@ int saa7134_vbi_init1(struct saa7134_dev *dev)
>   
>   	if (vbibufs < 2)
>   		vbibufs = 2;
> -	if (vbibufs > VIDEO_MAX_FRAME)
> -		vbibufs = VIDEO_MAX_FRAME;
> +	if (vbibufs > VB2_MAX_FRAME)
> +		vbibufs = VB2_MAX_FRAME;
>   	return 0;
>   }
>   
> diff --git a/drivers/media/pci/saa7134/saa7134-video.c b/drivers/media/pci/saa7134/saa7134-video.c
> index fc4a427..c7f39be 100644
> --- a/drivers/media/pci/saa7134/saa7134-video.c
> +++ b/drivers/media/pci/saa7134/saa7134-video.c
> @@ -2030,7 +2030,7 @@ int saa7134_video_init1(struct saa7134_dev *dev)
>   	int ret;
>   
>   	/* sanitycheck insmod options */
> -	if (gbuffers < 2 || gbuffers > VIDEO_MAX_FRAME)
> +	if (gbuffers < 2 || gbuffers > VB2_MAX_FRAME)
>   		gbuffers = 2;
>   	if (gbufsize > gbufsize_max)
>   		gbufsize = gbufsize_max;
> diff --git a/drivers/media/platform/mem2mem_testdev.c b/drivers/media/platform/mem2mem_testdev.c
> index c1b03cf..e1ff7e0 100644
> --- a/drivers/media/platform/mem2mem_testdev.c
> +++ b/drivers/media/platform/mem2mem_testdev.c
> @@ -55,7 +55,7 @@ MODULE_PARM_DESC(debug, "activates debug info");
>   #define MEM2MEM_NAME		"m2m-testdev"
>   
>   /* Per queue */
> -#define MEM2MEM_DEF_NUM_BUFS	VIDEO_MAX_FRAME
> +#define MEM2MEM_DEF_NUM_BUFS	VB2_MAX_FRAME
>   /* In bytes, per queue */
>   #define MEM2MEM_VID_MEM_LIMIT	(16 * 1024 * 1024)
>   
> diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c
> index 9a081c2..04e37c0 100644
> --- a/drivers/media/platform/ti-vpe/vpe.c
> +++ b/drivers/media/platform/ti-vpe/vpe.c
> @@ -1971,7 +1971,7 @@ static const struct v4l2_ctrl_config vpe_bufs_per_job = {
>   	.type = V4L2_CTRL_TYPE_INTEGER,
>   	.def = VPE_DEF_BUFS_PER_JOB,
>   	.min = 1,
> -	.max = VIDEO_MAX_FRAME,
> +	.max = VB2_MAX_FRAME,
>   	.step = 1,
>   };
>   
> diff --git a/drivers/media/platform/vivid/vivid-core.h b/drivers/media/platform/vivid/vivid-core.h
> index 811c286..c0375a1 100644
> --- a/drivers/media/platform/vivid/vivid-core.h
> +++ b/drivers/media/platform/vivid/vivid-core.h
> @@ -346,7 +346,7 @@ struct vivid_dev {
>   	/* video capture */
>   	struct tpg_data			tpg;
>   	unsigned			ms_vid_cap;
> -	bool				must_blank[VIDEO_MAX_FRAME];
> +	bool				must_blank[VB2_MAX_FRAME];
>   
>   	const struct vivid_fmt		*fmt_cap;
>   	struct v4l2_fract		timeperframe_vid_cap;
> diff --git a/drivers/media/platform/vivid/vivid-ctrls.c b/drivers/media/platform/vivid/vivid-ctrls.c
> index d5cbf00..7162f97 100644
> --- a/drivers/media/platform/vivid/vivid-ctrls.c
> +++ b/drivers/media/platform/vivid/vivid-ctrls.c
> @@ -332,7 +332,7 @@ static int vivid_vid_cap_s_ctrl(struct v4l2_ctrl *ctrl)
>   		break;
>   	case VIVID_CID_PERCENTAGE_FILL:
>   		tpg_s_perc_fill(&dev->tpg, ctrl->val);
> -		for (i = 0; i < VIDEO_MAX_FRAME; i++)
> +		for (i = 0; i < VB2_MAX_FRAME; i++)
>   			dev->must_blank[i] = ctrl->val < 100;
>   		break;
>   	case VIVID_CID_INSERT_SAV:
> diff --git a/drivers/media/platform/vivid/vivid-vid-cap.c b/drivers/media/platform/vivid/vivid-vid-cap.c
> index 331c544..55e8158 100644
> --- a/drivers/media/platform/vivid/vivid-vid-cap.c
> +++ b/drivers/media/platform/vivid/vivid-vid-cap.c
> @@ -252,7 +252,7 @@ static int vid_cap_start_streaming(struct vb2_queue *vq, unsigned count)
>   
>   	dev->vid_cap_seq_count = 0;
>   	dprintk(dev, 1, "%s\n", __func__);
> -	for (i = 0; i < VIDEO_MAX_FRAME; i++)
> +	for (i = 0; i < VB2_MAX_FRAME; i++)
>   		dev->must_blank[i] = tpg_g_perc_fill(&dev->tpg) < 100;
>   	if (dev->start_streaming_error) {
>   		dev->start_streaming_error = false;
> diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
> index 15b02f9..60354b4 100644
> --- a/drivers/media/v4l2-core/videobuf2-core.c
> +++ b/drivers/media/v4l2-core/videobuf2-core.c
> @@ -911,7 +911,7 @@ static int __reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
>   	/*
>   	 * Make sure the requested values and current defaults are sane.
>   	 */
> -	num_buffers = min_t(unsigned int, req->count, VIDEO_MAX_FRAME);
> +	num_buffers = min_t(unsigned int, req->count, VB2_MAX_FRAME);
>   	num_buffers = max_t(unsigned int, num_buffers, q->min_buffers_needed);
>   	memset(q->plane_sizes, 0, sizeof(q->plane_sizes));
>   	memset(q->alloc_ctx, 0, sizeof(q->alloc_ctx));
> @@ -1015,7 +1015,7 @@ static int __create_bufs(struct vb2_queue *q, struct v4l2_create_buffers *create
>   	unsigned int num_planes = 0, num_buffers, allocated_buffers;
>   	int ret;
>   
> -	if (q->num_buffers == VIDEO_MAX_FRAME) {
> +	if (q->num_buffers == VB2_MAX_FRAME) {
>   		dprintk(1, "maximum number of buffers already allocated\n");
>   		return -ENOBUFS;
>   	}
> @@ -1026,7 +1026,7 @@ static int __create_bufs(struct vb2_queue *q, struct v4l2_create_buffers *create
>   		q->memory = create->memory;
>   	}
>   
> -	num_buffers = min(create->count, VIDEO_MAX_FRAME - q->num_buffers);
> +	num_buffers = min(create->count, VB2_MAX_FRAME - q->num_buffers);
>   
>   	/*
>   	 * Ask the driver, whether the requested number of buffers, planes per
> @@ -2725,7 +2725,7 @@ struct vb2_fileio_data {
>   	struct v4l2_requestbuffers req;
>   	struct v4l2_plane p;
>   	struct v4l2_buffer b;
> -	struct vb2_fileio_buf bufs[VIDEO_MAX_FRAME];
> +	struct vb2_fileio_buf bufs[VB2_MAX_FRAME];
>   	unsigned int cur_index;
>   	unsigned int initial_index;
>   	unsigned int q_count;
> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> index a8608ce..66dcc40 100644
> --- a/include/media/videobuf2-core.h
> +++ b/include/media/videobuf2-core.h
> @@ -18,6 +18,8 @@
>   #include <linux/videodev2.h>
>   #include <linux/dma-buf.h>
>   
> +#define VB2_MAX_FRAME		64
> +
>   struct vb2_alloc_ctx;
>   struct vb2_fileio_data;
>   struct vb2_threadio_data;
> @@ -402,7 +404,7 @@ struct vb2_queue {
>   /* private: internal use only */
>   	struct mutex			mmap_lock;
>   	enum v4l2_memory		memory;
> -	struct vb2_buffer		*bufs[VIDEO_MAX_FRAME];
> +	struct vb2_buffer		*bufs[VB2_MAX_FRAME];
>   	unsigned int			num_buffers;
>   
>   	struct list_head		queued_list;

Re: [PATCH] vb2: replace VIDEO_MAX_FRAME with VB2_MAX_FRAME

[Hans Verkuil]

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? The only place where a maximum is enforced is
in REQBUFS/CREATE_BUFS. Most (all?) vb2 drivers leave enforcing the maximum to
vb2, they only care about the minimum, so they will automatically use the new
maximum.

> 
> Btw, wouldn't this change break for DaVinci:
> 	include/media/davinci/vpfe_capture.h:   u8 *fbuffers[VIDEO_MAX_FRAME];

That driver still uses vb1, so this patch won't break it.

Regards,

	Hans

> 
> Regards,
> Mauro
> 
>>
>> Signed-off-by: Divneil Wadhawan <divneil.wadhawan@st.com>
>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>> ---
>>   drivers/media/pci/saa7134/saa7134-ts.c       | 4 ++--
>>   drivers/media/pci/saa7134/saa7134-vbi.c      | 4 ++--
>>   drivers/media/pci/saa7134/saa7134-video.c    | 2 +-
>>   drivers/media/platform/mem2mem_testdev.c     | 2 +-
>>   drivers/media/platform/ti-vpe/vpe.c          | 2 +-
>>   drivers/media/platform/vivid/vivid-core.h    | 2 +-
>>   drivers/media/platform/vivid/vivid-ctrls.c   | 2 +-
>>   drivers/media/platform/vivid/vivid-vid-cap.c | 2 +-
>>   drivers/media/v4l2-core/videobuf2-core.c     | 8 ++++----
>>   include/media/videobuf2-core.h               | 4 +++-
>>   10 files changed, 17 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/media/pci/saa7134/saa7134-ts.c b/drivers/media/pci/saa7134/saa7134-ts.c
>> index bd25323..0d04995 100644
>> --- a/drivers/media/pci/saa7134/saa7134-ts.c
>> +++ b/drivers/media/pci/saa7134/saa7134-ts.c
>> @@ -227,8 +227,8 @@ int saa7134_ts_init1(struct saa7134_dev *dev)
>>   	/* sanitycheck insmod options */
>>   	if (tsbufs < 2)
>>   		tsbufs = 2;
>> -	if (tsbufs > VIDEO_MAX_FRAME)
>> -		tsbufs = VIDEO_MAX_FRAME;
>> +	if (tsbufs > VB2_MAX_FRAME)
>> +		tsbufs = VB2_MAX_FRAME;
>>   	if (ts_nr_packets < 4)
>>   		ts_nr_packets = 4;
>>   	if (ts_nr_packets > 312)
>> diff --git a/drivers/media/pci/saa7134/saa7134-vbi.c b/drivers/media/pci/saa7134/saa7134-vbi.c
>> index 4f0b101..2269837 100644
>> --- a/drivers/media/pci/saa7134/saa7134-vbi.c
>> +++ b/drivers/media/pci/saa7134/saa7134-vbi.c
>> @@ -203,8 +203,8 @@ int saa7134_vbi_init1(struct saa7134_dev *dev)
>>   
>>   	if (vbibufs < 2)
>>   		vbibufs = 2;
>> -	if (vbibufs > VIDEO_MAX_FRAME)
>> -		vbibufs = VIDEO_MAX_FRAME;
>> +	if (vbibufs > VB2_MAX_FRAME)
>> +		vbibufs = VB2_MAX_FRAME;
>>   	return 0;
>>   }
>>   
>> diff --git a/drivers/media/pci/saa7134/saa7134-video.c b/drivers/media/pci/saa7134/saa7134-video.c
>> index fc4a427..c7f39be 100644
>> --- a/drivers/media/pci/saa7134/saa7134-video.c
>> +++ b/drivers/media/pci/saa7134/saa7134-video.c
>> @@ -2030,7 +2030,7 @@ int saa7134_video_init1(struct saa7134_dev *dev)
>>   	int ret;
>>   
>>   	/* sanitycheck insmod options */
>> -	if (gbuffers < 2 || gbuffers > VIDEO_MAX_FRAME)
>> +	if (gbuffers < 2 || gbuffers > VB2_MAX_FRAME)
>>   		gbuffers = 2;
>>   	if (gbufsize > gbufsize_max)
>>   		gbufsize = gbufsize_max;
>> diff --git a/drivers/media/platform/mem2mem_testdev.c b/drivers/media/platform/mem2mem_testdev.c
>> index c1b03cf..e1ff7e0 100644
>> --- a/drivers/media/platform/mem2mem_testdev.c
>> +++ b/drivers/media/platform/mem2mem_testdev.c
>> @@ -55,7 +55,7 @@ MODULE_PARM_DESC(debug, "activates debug info");
>>   #define MEM2MEM_NAME		"m2m-testdev"
>>   
>>   /* Per queue */
>> -#define MEM2MEM_DEF_NUM_BUFS	VIDEO_MAX_FRAME
>> +#define MEM2MEM_DEF_NUM_BUFS	VB2_MAX_FRAME
>>   /* In bytes, per queue */
>>   #define MEM2MEM_VID_MEM_LIMIT	(16 * 1024 * 1024)
>>   
>> diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c
>> index 9a081c2..04e37c0 100644
>> --- a/drivers/media/platform/ti-vpe/vpe.c
>> +++ b/drivers/media/platform/ti-vpe/vpe.c
>> @@ -1971,7 +1971,7 @@ static const struct v4l2_ctrl_config vpe_bufs_per_job = {
>>   	.type = V4L2_CTRL_TYPE_INTEGER,
>>   	.def = VPE_DEF_BUFS_PER_JOB,
>>   	.min = 1,
>> -	.max = VIDEO_MAX_FRAME,
>> +	.max = VB2_MAX_FRAME,
>>   	.step = 1,
>>   };
>>   
>> diff --git a/drivers/media/platform/vivid/vivid-core.h b/drivers/media/platform/vivid/vivid-core.h
>> index 811c286..c0375a1 100644
>> --- a/drivers/media/platform/vivid/vivid-core.h
>> +++ b/drivers/media/platform/vivid/vivid-core.h
>> @@ -346,7 +346,7 @@ struct vivid_dev {
>>   	/* video capture */
>>   	struct tpg_data			tpg;
>>   	unsigned			ms_vid_cap;
>> -	bool				must_blank[VIDEO_MAX_FRAME];
>> +	bool				must_blank[VB2_MAX_FRAME];
>>   
>>   	const struct vivid_fmt		*fmt_cap;
>>   	struct v4l2_fract		timeperframe_vid_cap;
>> diff --git a/drivers/media/platform/vivid/vivid-ctrls.c b/drivers/media/platform/vivid/vivid-ctrls.c
>> index d5cbf00..7162f97 100644
>> --- a/drivers/media/platform/vivid/vivid-ctrls.c
>> +++ b/drivers/media/platform/vivid/vivid-ctrls.c
>> @@ -332,7 +332,7 @@ static int vivid_vid_cap_s_ctrl(struct v4l2_ctrl *ctrl)
>>   		break;
>>   	case VIVID_CID_PERCENTAGE_FILL:
>>   		tpg_s_perc_fill(&dev->tpg, ctrl->val);
>> -		for (i = 0; i < VIDEO_MAX_FRAME; i++)
>> +		for (i = 0; i < VB2_MAX_FRAME; i++)
>>   			dev->must_blank[i] = ctrl->val < 100;
>>   		break;
>>   	case VIVID_CID_INSERT_SAV:
>> diff --git a/drivers/media/platform/vivid/vivid-vid-cap.c b/drivers/media/platform/vivid/vivid-vid-cap.c
>> index 331c544..55e8158 100644
>> --- a/drivers/media/platform/vivid/vivid-vid-cap.c
>> +++ b/drivers/media/platform/vivid/vivid-vid-cap.c
>> @@ -252,7 +252,7 @@ static int vid_cap_start_streaming(struct vb2_queue *vq, unsigned count)
>>   
>>   	dev->vid_cap_seq_count = 0;
>>   	dprintk(dev, 1, "%s\n", __func__);
>> -	for (i = 0; i < VIDEO_MAX_FRAME; i++)
>> +	for (i = 0; i < VB2_MAX_FRAME; i++)
>>   		dev->must_blank[i] = tpg_g_perc_fill(&dev->tpg) < 100;
>>   	if (dev->start_streaming_error) {
>>   		dev->start_streaming_error = false;
>> diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
>> index 15b02f9..60354b4 100644
>> --- a/drivers/media/v4l2-core/videobuf2-core.c
>> +++ b/drivers/media/v4l2-core/videobuf2-core.c
>> @@ -911,7 +911,7 @@ static int __reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
>>   	/*
>>   	 * Make sure the requested values and current defaults are sane.
>>   	 */
>> -	num_buffers = min_t(unsigned int, req->count, VIDEO_MAX_FRAME);
>> +	num_buffers = min_t(unsigned int, req->count, VB2_MAX_FRAME);
>>   	num_buffers = max_t(unsigned int, num_buffers, q->min_buffers_needed);
>>   	memset(q->plane_sizes, 0, sizeof(q->plane_sizes));
>>   	memset(q->alloc_ctx, 0, sizeof(q->alloc_ctx));
>> @@ -1015,7 +1015,7 @@ static int __create_bufs(struct vb2_queue *q, struct v4l2_create_buffers *create
>>   	unsigned int num_planes = 0, num_buffers, allocated_buffers;
>>   	int ret;
>>   
>> -	if (q->num_buffers == VIDEO_MAX_FRAME) {
>> +	if (q->num_buffers == VB2_MAX_FRAME) {
>>   		dprintk(1, "maximum number of buffers already allocated\n");
>>   		return -ENOBUFS;
>>   	}
>> @@ -1026,7 +1026,7 @@ static int __create_bufs(struct vb2_queue *q, struct v4l2_create_buffers *create
>>   		q->memory = create->memory;
>>   	}
>>   
>> -	num_buffers = min(create->count, VIDEO_MAX_FRAME - q->num_buffers);
>> +	num_buffers = min(create->count, VB2_MAX_FRAME - q->num_buffers);
>>   
>>   	/*
>>   	 * Ask the driver, whether the requested number of buffers, planes per
>> @@ -2725,7 +2725,7 @@ struct vb2_fileio_data {
>>   	struct v4l2_requestbuffers req;
>>   	struct v4l2_plane p;
>>   	struct v4l2_buffer b;
>> -	struct vb2_fileio_buf bufs[VIDEO_MAX_FRAME];
>> +	struct vb2_fileio_buf bufs[VB2_MAX_FRAME];
>>   	unsigned int cur_index;
>>   	unsigned int initial_index;
>>   	unsigned int q_count;
>> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
>> index a8608ce..66dcc40 100644
>> --- a/include/media/videobuf2-core.h
>> +++ b/include/media/videobuf2-core.h
>> @@ -18,6 +18,8 @@
>>   #include <linux/videodev2.h>
>>   #include <linux/dma-buf.h>
>>   
>> +#define VB2_MAX_FRAME		64
>> +
>>   struct vb2_alloc_ctx;
>>   struct vb2_fileio_data;
>>   struct vb2_threadio_data;
>> @@ -402,7 +404,7 @@ struct vb2_queue {
>>   /* private: internal use only */
>>   	struct mutex			mmap_lock;
>>   	enum v4l2_memory		memory;
>> -	struct vb2_buffer		*bufs[VIDEO_MAX_FRAME];
>> +	struct vb2_buffer		*bufs[VB2_MAX_FRAME];
>>   	unsigned int			num_buffers;
>>   
>>   	struct list_head		queued_list;

Re: [PATCH] vb2: replace VIDEO_MAX_FRAME with VB2_MAX_FRAME

[Mauro Carvalho Chehab]

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.

> The only place where a maximum is enforced is
> in REQBUFS/CREATE_BUFS. Most (all?) vb2 drivers leave enforcing the maximum to
> vb2, they only care about the minimum, so they will automatically use the new
> maximum.
> 
> > 
> > Btw, wouldn't this change break for DaVinci:
> > 	include/media/davinci/vpfe_capture.h:   u8 *fbuffers[VIDEO_MAX_FRAME];
> 
> That driver still uses vb1, so this patch won't break it.

No, it doesn't. Check the DaVinci Kconfig/Makefile. I merged a patchset
that converted it to VB2.

Regards,
Mauro

> 
> Regards,
> 
> 	Hans
> 
> > 
> > Regards,
> > Mauro
> > 
> >>
> >> Signed-off-by: Divneil Wadhawan <divneil.wadhawan@st.com>
> >> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> >> ---
> >>   drivers/media/pci/saa7134/saa7134-ts.c       | 4 ++--
> >>   drivers/media/pci/saa7134/saa7134-vbi.c      | 4 ++--
> >>   drivers/media/pci/saa7134/saa7134-video.c    | 2 +-
> >>   drivers/media/platform/mem2mem_testdev.c     | 2 +-
> >>   drivers/media/platform/ti-vpe/vpe.c          | 2 +-
> >>   drivers/media/platform/vivid/vivid-core.h    | 2 +-
> >>   drivers/media/platform/vivid/vivid-ctrls.c   | 2 +-
> >>   drivers/media/platform/vivid/vivid-vid-cap.c | 2 +-
> >>   drivers/media/v4l2-core/videobuf2-core.c     | 8 ++++----
> >>   include/media/videobuf2-core.h               | 4 +++-
> >>   10 files changed, 17 insertions(+), 15 deletions(-)
> >>
> >> diff --git a/drivers/media/pci/saa7134/saa7134-ts.c b/drivers/media/pci/saa7134/saa7134-ts.c
> >> index bd25323..0d04995 100644
> >> --- a/drivers/media/pci/saa7134/saa7134-ts.c
> >> +++ b/drivers/media/pci/saa7134/saa7134-ts.c
> >> @@ -227,8 +227,8 @@ int saa7134_ts_init1(struct saa7134_dev *dev)
> >>   	/* sanitycheck insmod options */
> >>   	if (tsbufs < 2)
> >>   		tsbufs = 2;
> >> -	if (tsbufs > VIDEO_MAX_FRAME)
> >> -		tsbufs = VIDEO_MAX_FRAME;
> >> +	if (tsbufs > VB2_MAX_FRAME)
> >> +		tsbufs = VB2_MAX_FRAME;
> >>   	if (ts_nr_packets < 4)
> >>   		ts_nr_packets = 4;
> >>   	if (ts_nr_packets > 312)
> >> diff --git a/drivers/media/pci/saa7134/saa7134-vbi.c b/drivers/media/pci/saa7134/saa7134-vbi.c
> >> index 4f0b101..2269837 100644
> >> --- a/drivers/media/pci/saa7134/saa7134-vbi.c
> >> +++ b/drivers/media/pci/saa7134/saa7134-vbi.c
> >> @@ -203,8 +203,8 @@ int saa7134_vbi_init1(struct saa7134_dev *dev)
> >>   
> >>   	if (vbibufs < 2)
> >>   		vbibufs = 2;
> >> -	if (vbibufs > VIDEO_MAX_FRAME)
> >> -		vbibufs = VIDEO_MAX_FRAME;
> >> +	if (vbibufs > VB2_MAX_FRAME)
> >> +		vbibufs = VB2_MAX_FRAME;
> >>   	return 0;
> >>   }
> >>   
> >> diff --git a/drivers/media/pci/saa7134/saa7134-video.c b/drivers/media/pci/saa7134/saa7134-video.c
> >> index fc4a427..c7f39be 100644
> >> --- a/drivers/media/pci/saa7134/saa7134-video.c
> >> +++ b/drivers/media/pci/saa7134/saa7134-video.c
> >> @@ -2030,7 +2030,7 @@ int saa7134_video_init1(struct saa7134_dev *dev)
> >>   	int ret;
> >>   
> >>   	/* sanitycheck insmod options */
> >> -	if (gbuffers < 2 || gbuffers > VIDEO_MAX_FRAME)
> >> +	if (gbuffers < 2 || gbuffers > VB2_MAX_FRAME)
> >>   		gbuffers = 2;
> >>   	if (gbufsize > gbufsize_max)
> >>   		gbufsize = gbufsize_max;
> >> diff --git a/drivers/media/platform/mem2mem_testdev.c b/drivers/media/platform/mem2mem_testdev.c
> >> index c1b03cf..e1ff7e0 100644
> >> --- a/drivers/media/platform/mem2mem_testdev.c
> >> +++ b/drivers/media/platform/mem2mem_testdev.c
> >> @@ -55,7 +55,7 @@ MODULE_PARM_DESC(debug, "activates debug info");
> >>   #define MEM2MEM_NAME		"m2m-testdev"
> >>   
> >>   /* Per queue */
> >> -#define MEM2MEM_DEF_NUM_BUFS	VIDEO_MAX_FRAME
> >> +#define MEM2MEM_DEF_NUM_BUFS	VB2_MAX_FRAME
> >>   /* In bytes, per queue */
> >>   #define MEM2MEM_VID_MEM_LIMIT	(16 * 1024 * 1024)
> >>   
> >> diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c
> >> index 9a081c2..04e37c0 100644
> >> --- a/drivers/media/platform/ti-vpe/vpe.c
> >> +++ b/drivers/media/platform/ti-vpe/vpe.c
> >> @@ -1971,7 +1971,7 @@ static const struct v4l2_ctrl_config vpe_bufs_per_job = {
> >>   	.type = V4L2_CTRL_TYPE_INTEGER,
> >>   	.def = VPE_DEF_BUFS_PER_JOB,
> >>   	.min = 1,
> >> -	.max = VIDEO_MAX_FRAME,
> >> +	.max = VB2_MAX_FRAME,
> >>   	.step = 1,
> >>   };
> >>   
> >> diff --git a/drivers/media/platform/vivid/vivid-core.h b/drivers/media/platform/vivid/vivid-core.h
> >> index 811c286..c0375a1 100644
> >> --- a/drivers/media/platform/vivid/vivid-core.h
> >> +++ b/drivers/media/platform/vivid/vivid-core.h
> >> @@ -346,7 +346,7 @@ struct vivid_dev {
> >>   	/* video capture */
> >>   	struct tpg_data			tpg;
> >>   	unsigned			ms_vid_cap;
> >> -	bool				must_blank[VIDEO_MAX_FRAME];
> >> +	bool				must_blank[VB2_MAX_FRAME];
> >>   
> >>   	const struct vivid_fmt		*fmt_cap;
> >>   	struct v4l2_fract		timeperframe_vid_cap;
> >> diff --git a/drivers/media/platform/vivid/vivid-ctrls.c b/drivers/media/platform/vivid/vivid-ctrls.c
> >> index d5cbf00..7162f97 100644
> >> --- a/drivers/media/platform/vivid/vivid-ctrls.c
> >> +++ b/drivers/media/platform/vivid/vivid-ctrls.c
> >> @@ -332,7 +332,7 @@ static int vivid_vid_cap_s_ctrl(struct v4l2_ctrl *ctrl)
> >>   		break;
> >>   	case VIVID_CID_PERCENTAGE_FILL:
> >>   		tpg_s_perc_fill(&dev->tpg, ctrl->val);
> >> -		for (i = 0; i < VIDEO_MAX_FRAME; i++)
> >> +		for (i = 0; i < VB2_MAX_FRAME; i++)
> >>   			dev->must_blank[i] = ctrl->val < 100;
> >>   		break;
> >>   	case VIVID_CID_INSERT_SAV:
> >> diff --git a/drivers/media/platform/vivid/vivid-vid-cap.c b/drivers/media/platform/vivid/vivid-vid-cap.c
> >> index 331c544..55e8158 100644
> >> --- a/drivers/media/platform/vivid/vivid-vid-cap.c
> >> +++ b/drivers/media/platform/vivid/vivid-vid-cap.c
> >> @@ -252,7 +252,7 @@ static int vid_cap_start_streaming(struct vb2_queue *vq, unsigned count)
> >>   
> >>   	dev->vid_cap_seq_count = 0;
> >>   	dprintk(dev, 1, "%s\n", __func__);
> >> -	for (i = 0; i < VIDEO_MAX_FRAME; i++)
> >> +	for (i = 0; i < VB2_MAX_FRAME; i++)
> >>   		dev->must_blank[i] = tpg_g_perc_fill(&dev->tpg) < 100;
> >>   	if (dev->start_streaming_error) {
> >>   		dev->start_streaming_error = false;
> >> diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
> >> index 15b02f9..60354b4 100644
> >> --- a/drivers/media/v4l2-core/videobuf2-core.c
> >> +++ b/drivers/media/v4l2-core/videobuf2-core.c
> >> @@ -911,7 +911,7 @@ static int __reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
> >>   	/*
> >>   	 * Make sure the requested values and current defaults are sane.
> >>   	 */
> >> -	num_buffers = min_t(unsigned int, req->count, VIDEO_MAX_FRAME);
> >> +	num_buffers = min_t(unsigned int, req->count, VB2_MAX_FRAME);
> >>   	num_buffers = max_t(unsigned int, num_buffers, q->min_buffers_needed);
> >>   	memset(q->plane_sizes, 0, sizeof(q->plane_sizes));
> >>   	memset(q->alloc_ctx, 0, sizeof(q->alloc_ctx));
> >> @@ -1015,7 +1015,7 @@ static int __create_bufs(struct vb2_queue *q, struct v4l2_create_buffers *create
> >>   	unsigned int num_planes = 0, num_buffers, allocated_buffers;
> >>   	int ret;
> >>   
> >> -	if (q->num_buffers == VIDEO_MAX_FRAME) {
> >> +	if (q->num_buffers == VB2_MAX_FRAME) {
> >>   		dprintk(1, "maximum number of buffers already allocated\n");
> >>   		return -ENOBUFS;
> >>   	}
> >> @@ -1026,7 +1026,7 @@ static int __create_bufs(struct vb2_queue *q, struct v4l2_create_buffers *create
> >>   		q->memory = create->memory;
> >>   	}
> >>   
> >> -	num_buffers = min(create->count, VIDEO_MAX_FRAME - q->num_buffers);
> >> +	num_buffers = min(create->count, VB2_MAX_FRAME - q->num_buffers);
> >>   
> >>   	/*
> >>   	 * Ask the driver, whether the requested number of buffers, planes per
> >> @@ -2725,7 +2725,7 @@ struct vb2_fileio_data {
> >>   	struct v4l2_requestbuffers req;
> >>   	struct v4l2_plane p;
> >>   	struct v4l2_buffer b;
> >> -	struct vb2_fileio_buf bufs[VIDEO_MAX_FRAME];
> >> +	struct vb2_fileio_buf bufs[VB2_MAX_FRAME];
> >>   	unsigned int cur_index;
> >>   	unsigned int initial_index;
> >>   	unsigned int q_count;
> >> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> >> index a8608ce..66dcc40 100644
> >> --- a/include/media/videobuf2-core.h
> >> +++ b/include/media/videobuf2-core.h
> >> @@ -18,6 +18,8 @@
> >>   #include <linux/videodev2.h>
> >>   #include <linux/dma-buf.h>
> >>   
> >> +#define VB2_MAX_FRAME		64
> >> +
> >>   struct vb2_alloc_ctx;
> >>   struct vb2_fileio_data;
> >>   struct vb2_threadio_data;
> >> @@ -402,7 +404,7 @@ struct vb2_queue {
> >>   /* private: internal use only */
> >>   	struct mutex			mmap_lock;
> >>   	enum v4l2_memory		memory;
> >> -	struct vb2_buffer		*bufs[VIDEO_MAX_FRAME];
> >> +	struct vb2_buffer		*bufs[VB2_MAX_FRAME];
> >>   	unsigned int			num_buffers;
> >>   
> >>   	struct list_head		queued_list;
> 

Re: [PATCH] vb2: replace VIDEO_MAX_FRAME with VB2_MAX_FRAME

[Hans Verkuil]

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.

Regards,

	Hans

> 
>> The only place where a maximum is enforced is
>> in REQBUFS/CREATE_BUFS. Most (all?) vb2 drivers leave enforcing the maximum to
>> vb2, they only care about the minimum, so they will automatically use the new
>> maximum.
>>
>>>
>>> Btw, wouldn't this change break for DaVinci:
>>> 	include/media/davinci/vpfe_capture.h:   u8 *fbuffers[VIDEO_MAX_FRAME];
>>
>> That driver still uses vb1, so this patch won't break it.
> 
> No, it doesn't. Check the DaVinci Kconfig/Makefile. I merged a patchset
> that converted it to VB2.
> 
> Regards,
> Mauro
> 
>>
>> Regards,
>>
>> 	Hans
>>
>>>
>>> Regards,
>>> Mauro
>>>
>>>>
>>>> Signed-off-by: Divneil Wadhawan <divneil.wadhawan@st.com>
>>>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>>>> ---
>>>>   drivers/media/pci/saa7134/saa7134-ts.c       | 4 ++--
>>>>   drivers/media/pci/saa7134/saa7134-vbi.c      | 4 ++--
>>>>   drivers/media/pci/saa7134/saa7134-video.c    | 2 +-
>>>>   drivers/media/platform/mem2mem_testdev.c     | 2 +-
>>>>   drivers/media/platform/ti-vpe/vpe.c          | 2 +-
>>>>   drivers/media/platform/vivid/vivid-core.h    | 2 +-
>>>>   drivers/media/platform/vivid/vivid-ctrls.c   | 2 +-
>>>>   drivers/media/platform/vivid/vivid-vid-cap.c | 2 +-
>>>>   drivers/media/v4l2-core/videobuf2-core.c     | 8 ++++----
>>>>   include/media/videobuf2-core.h               | 4 +++-
>>>>   10 files changed, 17 insertions(+), 15 deletions(-)
>>>>
>>>> diff --git a/drivers/media/pci/saa7134/saa7134-ts.c b/drivers/media/pci/saa7134/saa7134-ts.c
>>>> index bd25323..0d04995 100644
>>>> --- a/drivers/media/pci/saa7134/saa7134-ts.c
>>>> +++ b/drivers/media/pci/saa7134/saa7134-ts.c
>>>> @@ -227,8 +227,8 @@ int saa7134_ts_init1(struct saa7134_dev *dev)
>>>>   	/* sanitycheck insmod options */
>>>>   	if (tsbufs < 2)
>>>>   		tsbufs = 2;
>>>> -	if (tsbufs > VIDEO_MAX_FRAME)
>>>> -		tsbufs = VIDEO_MAX_FRAME;
>>>> +	if (tsbufs > VB2_MAX_FRAME)
>>>> +		tsbufs = VB2_MAX_FRAME;
>>>>   	if (ts_nr_packets < 4)
>>>>   		ts_nr_packets = 4;
>>>>   	if (ts_nr_packets > 312)
>>>> diff --git a/drivers/media/pci/saa7134/saa7134-vbi.c b/drivers/media/pci/saa7134/saa7134-vbi.c
>>>> index 4f0b101..2269837 100644
>>>> --- a/drivers/media/pci/saa7134/saa7134-vbi.c
>>>> +++ b/drivers/media/pci/saa7134/saa7134-vbi.c
>>>> @@ -203,8 +203,8 @@ int saa7134_vbi_init1(struct saa7134_dev *dev)
>>>>   
>>>>   	if (vbibufs < 2)
>>>>   		vbibufs = 2;
>>>> -	if (vbibufs > VIDEO_MAX_FRAME)
>>>> -		vbibufs = VIDEO_MAX_FRAME;
>>>> +	if (vbibufs > VB2_MAX_FRAME)
>>>> +		vbibufs = VB2_MAX_FRAME;
>>>>   	return 0;
>>>>   }
>>>>   
>>>> diff --git a/drivers/media/pci/saa7134/saa7134-video.c b/drivers/media/pci/saa7134/saa7134-video.c
>>>> index fc4a427..c7f39be 100644
>>>> --- a/drivers/media/pci/saa7134/saa7134-video.c
>>>> +++ b/drivers/media/pci/saa7134/saa7134-video.c
>>>> @@ -2030,7 +2030,7 @@ int saa7134_video_init1(struct saa7134_dev *dev)
>>>>   	int ret;
>>>>   
>>>>   	/* sanitycheck insmod options */
>>>> -	if (gbuffers < 2 || gbuffers > VIDEO_MAX_FRAME)
>>>> +	if (gbuffers < 2 || gbuffers > VB2_MAX_FRAME)
>>>>   		gbuffers = 2;
>>>>   	if (gbufsize > gbufsize_max)
>>>>   		gbufsize = gbufsize_max;
>>>> diff --git a/drivers/media/platform/mem2mem_testdev.c b/drivers/media/platform/mem2mem_testdev.c
>>>> index c1b03cf..e1ff7e0 100644
>>>> --- a/drivers/media/platform/mem2mem_testdev.c
>>>> +++ b/drivers/media/platform/mem2mem_testdev.c
>>>> @@ -55,7 +55,7 @@ MODULE_PARM_DESC(debug, "activates debug info");
>>>>   #define MEM2MEM_NAME		"m2m-testdev"
>>>>   
>>>>   /* Per queue */
>>>> -#define MEM2MEM_DEF_NUM_BUFS	VIDEO_MAX_FRAME
>>>> +#define MEM2MEM_DEF_NUM_BUFS	VB2_MAX_FRAME
>>>>   /* In bytes, per queue */
>>>>   #define MEM2MEM_VID_MEM_LIMIT	(16 * 1024 * 1024)
>>>>   
>>>> diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c
>>>> index 9a081c2..04e37c0 100644
>>>> --- a/drivers/media/platform/ti-vpe/vpe.c
>>>> +++ b/drivers/media/platform/ti-vpe/vpe.c
>>>> @@ -1971,7 +1971,7 @@ static const struct v4l2_ctrl_config vpe_bufs_per_job = {
>>>>   	.type = V4L2_CTRL_TYPE_INTEGER,
>>>>   	.def = VPE_DEF_BUFS_PER_JOB,
>>>>   	.min = 1,
>>>> -	.max = VIDEO_MAX_FRAME,
>>>> +	.max = VB2_MAX_FRAME,
>>>>   	.step = 1,
>>>>   };
>>>>   
>>>> diff --git a/drivers/media/platform/vivid/vivid-core.h b/drivers/media/platform/vivid/vivid-core.h
>>>> index 811c286..c0375a1 100644
>>>> --- a/drivers/media/platform/vivid/vivid-core.h
>>>> +++ b/drivers/media/platform/vivid/vivid-core.h
>>>> @@ -346,7 +346,7 @@ struct vivid_dev {
>>>>   	/* video capture */
>>>>   	struct tpg_data			tpg;
>>>>   	unsigned			ms_vid_cap;
>>>> -	bool				must_blank[VIDEO_MAX_FRAME];
>>>> +	bool				must_blank[VB2_MAX_FRAME];
>>>>   
>>>>   	const struct vivid_fmt		*fmt_cap;
>>>>   	struct v4l2_fract		timeperframe_vid_cap;
>>>> diff --git a/drivers/media/platform/vivid/vivid-ctrls.c b/drivers/media/platform/vivid/vivid-ctrls.c
>>>> index d5cbf00..7162f97 100644
>>>> --- a/drivers/media/platform/vivid/vivid-ctrls.c
>>>> +++ b/drivers/media/platform/vivid/vivid-ctrls.c
>>>> @@ -332,7 +332,7 @@ static int vivid_vid_cap_s_ctrl(struct v4l2_ctrl *ctrl)
>>>>   		break;
>>>>   	case VIVID_CID_PERCENTAGE_FILL:
>>>>   		tpg_s_perc_fill(&dev->tpg, ctrl->val);
>>>> -		for (i = 0; i < VIDEO_MAX_FRAME; i++)
>>>> +		for (i = 0; i < VB2_MAX_FRAME; i++)
>>>>   			dev->must_blank[i] = ctrl->val < 100;
>>>>   		break;
>>>>   	case VIVID_CID_INSERT_SAV:
>>>> diff --git a/drivers/media/platform/vivid/vivid-vid-cap.c b/drivers/media/platform/vivid/vivid-vid-cap.c
>>>> index 331c544..55e8158 100644
>>>> --- a/drivers/media/platform/vivid/vivid-vid-cap.c
>>>> +++ b/drivers/media/platform/vivid/vivid-vid-cap.c
>>>> @@ -252,7 +252,7 @@ static int vid_cap_start_streaming(struct vb2_queue *vq, unsigned count)
>>>>   
>>>>   	dev->vid_cap_seq_count = 0;
>>>>   	dprintk(dev, 1, "%s\n", __func__);
>>>> -	for (i = 0; i < VIDEO_MAX_FRAME; i++)
>>>> +	for (i = 0; i < VB2_MAX_FRAME; i++)
>>>>   		dev->must_blank[i] = tpg_g_perc_fill(&dev->tpg) < 100;
>>>>   	if (dev->start_streaming_error) {
>>>>   		dev->start_streaming_error = false;
>>>> diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
>>>> index 15b02f9..60354b4 100644
>>>> --- a/drivers/media/v4l2-core/videobuf2-core.c
>>>> +++ b/drivers/media/v4l2-core/videobuf2-core.c
>>>> @@ -911,7 +911,7 @@ static int __reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
>>>>   	/*
>>>>   	 * Make sure the requested values and current defaults are sane.
>>>>   	 */
>>>> -	num_buffers = min_t(unsigned int, req->count, VIDEO_MAX_FRAME);
>>>> +	num_buffers = min_t(unsigned int, req->count, VB2_MAX_FRAME);
>>>>   	num_buffers = max_t(unsigned int, num_buffers, q->min_buffers_needed);
>>>>   	memset(q->plane_sizes, 0, sizeof(q->plane_sizes));
>>>>   	memset(q->alloc_ctx, 0, sizeof(q->alloc_ctx));
>>>> @@ -1015,7 +1015,7 @@ static int __create_bufs(struct vb2_queue *q, struct v4l2_create_buffers *create
>>>>   	unsigned int num_planes = 0, num_buffers, allocated_buffers;
>>>>   	int ret;
>>>>   
>>>> -	if (q->num_buffers == VIDEO_MAX_FRAME) {
>>>> +	if (q->num_buffers == VB2_MAX_FRAME) {
>>>>   		dprintk(1, "maximum number of buffers already allocated\n");
>>>>   		return -ENOBUFS;
>>>>   	}
>>>> @@ -1026,7 +1026,7 @@ static int __create_bufs(struct vb2_queue *q, struct v4l2_create_buffers *create
>>>>   		q->memory = create->memory;
>>>>   	}
>>>>   
>>>> -	num_buffers = min(create->count, VIDEO_MAX_FRAME - q->num_buffers);
>>>> +	num_buffers = min(create->count, VB2_MAX_FRAME - q->num_buffers);
>>>>   
>>>>   	/*
>>>>   	 * Ask the driver, whether the requested number of buffers, planes per
>>>> @@ -2725,7 +2725,7 @@ struct vb2_fileio_data {
>>>>   	struct v4l2_requestbuffers req;
>>>>   	struct v4l2_plane p;
>>>>   	struct v4l2_buffer b;
>>>> -	struct vb2_fileio_buf bufs[VIDEO_MAX_FRAME];
>>>> +	struct vb2_fileio_buf bufs[VB2_MAX_FRAME];
>>>>   	unsigned int cur_index;
>>>>   	unsigned int initial_index;
>>>>   	unsigned int q_count;
>>>> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
>>>> index a8608ce..66dcc40 100644
>>>> --- a/include/media/videobuf2-core.h
>>>> +++ b/include/media/videobuf2-core.h
>>>> @@ -18,6 +18,8 @@
>>>>   #include <linux/videodev2.h>
>>>>   #include <linux/dma-buf.h>
>>>>   
>>>> +#define VB2_MAX_FRAME		64
>>>> +
>>>>   struct vb2_alloc_ctx;
>>>>   struct vb2_fileio_data;
>>>>   struct vb2_threadio_data;
>>>> @@ -402,7 +404,7 @@ struct vb2_queue {
>>>>   /* private: internal use only */
>>>>   	struct mutex			mmap_lock;
>>>>   	enum v4l2_memory		memory;
>>>> -	struct vb2_buffer		*bufs[VIDEO_MAX_FRAME];
>>>> +	struct vb2_buffer		*bufs[VB2_MAX_FRAME];
>>>>   	unsigned int			num_buffers;
>>>>   
>>>>   	struct list_head		queued_list;
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH] vb2: replace VIDEO_MAX_FRAME with VB2_MAX_FRAME

[Mauro Carvalho Chehab]

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.

> 
> Regards,
> 
> 	Hans
> 
> > 
> >> The only place where a maximum is enforced is
> >> in REQBUFS/CREATE_BUFS. Most (all?) vb2 drivers leave enforcing the maximum to
> >> vb2, they only care about the minimum, so they will automatically use the new
> >> maximum.
> >>
> >>>
> >>> Btw, wouldn't this change break for DaVinci:
> >>> 	include/media/davinci/vpfe_capture.h:   u8 *fbuffers[VIDEO_MAX_FRAME];
> >>
> >> That driver still uses vb1, so this patch won't break it.
> > 
> > No, it doesn't. Check the DaVinci Kconfig/Makefile. I merged a patchset
> > that converted it to VB2.
> > 
> > Regards,
> > Mauro
> > 
> >>
> >> Regards,
> >>
> >> 	Hans
> >>
> >>>
> >>> Regards,
> >>> Mauro
> >>>
> >>>>
> >>>> Signed-off-by: Divneil Wadhawan <divneil.wadhawan@st.com>
> >>>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> >>>> ---
> >>>>   drivers/media/pci/saa7134/saa7134-ts.c       | 4 ++--
> >>>>   drivers/media/pci/saa7134/saa7134-vbi.c      | 4 ++--
> >>>>   drivers/media/pci/saa7134/saa7134-video.c    | 2 +-
> >>>>   drivers/media/platform/mem2mem_testdev.c     | 2 +-
> >>>>   drivers/media/platform/ti-vpe/vpe.c          | 2 +-
> >>>>   drivers/media/platform/vivid/vivid-core.h    | 2 +-
> >>>>   drivers/media/platform/vivid/vivid-ctrls.c   | 2 +-
> >>>>   drivers/media/platform/vivid/vivid-vid-cap.c | 2 +-
> >>>>   drivers/media/v4l2-core/videobuf2-core.c     | 8 ++++----
> >>>>   include/media/videobuf2-core.h               | 4 +++-
> >>>>   10 files changed, 17 insertions(+), 15 deletions(-)
> >>>>
> >>>> diff --git a/drivers/media/pci/saa7134/saa7134-ts.c b/drivers/media/pci/saa7134/saa7134-ts.c
> >>>> index bd25323..0d04995 100644
> >>>> --- a/drivers/media/pci/saa7134/saa7134-ts.c
> >>>> +++ b/drivers/media/pci/saa7134/saa7134-ts.c
> >>>> @@ -227,8 +227,8 @@ int saa7134_ts_init1(struct saa7134_dev *dev)
> >>>>   	/* sanitycheck insmod options */
> >>>>   	if (tsbufs < 2)
> >>>>   		tsbufs = 2;
> >>>> -	if (tsbufs > VIDEO_MAX_FRAME)
> >>>> -		tsbufs = VIDEO_MAX_FRAME;
> >>>> +	if (tsbufs > VB2_MAX_FRAME)
> >>>> +		tsbufs = VB2_MAX_FRAME;
> >>>>   	if (ts_nr_packets < 4)
> >>>>   		ts_nr_packets = 4;
> >>>>   	if (ts_nr_packets > 312)
> >>>> diff --git a/drivers/media/pci/saa7134/saa7134-vbi.c b/drivers/media/pci/saa7134/saa7134-vbi.c
> >>>> index 4f0b101..2269837 100644
> >>>> --- a/drivers/media/pci/saa7134/saa7134-vbi.c
> >>>> +++ b/drivers/media/pci/saa7134/saa7134-vbi.c
> >>>> @@ -203,8 +203,8 @@ int saa7134_vbi_init1(struct saa7134_dev *dev)
> >>>>   
> >>>>   	if (vbibufs < 2)
> >>>>   		vbibufs = 2;
> >>>> -	if (vbibufs > VIDEO_MAX_FRAME)
> >>>> -		vbibufs = VIDEO_MAX_FRAME;
> >>>> +	if (vbibufs > VB2_MAX_FRAME)
> >>>> +		vbibufs = VB2_MAX_FRAME;
> >>>>   	return 0;
> >>>>   }
> >>>>   
> >>>> diff --git a/drivers/media/pci/saa7134/saa7134-video.c b/drivers/media/pci/saa7134/saa7134-video.c
> >>>> index fc4a427..c7f39be 100644
> >>>> --- a/drivers/media/pci/saa7134/saa7134-video.c
> >>>> +++ b/drivers/media/pci/saa7134/saa7134-video.c
> >>>> @@ -2030,7 +2030,7 @@ int saa7134_video_init1(struct saa7134_dev *dev)
> >>>>   	int ret;
> >>>>   
> >>>>   	/* sanitycheck insmod options */
> >>>> -	if (gbuffers < 2 || gbuffers > VIDEO_MAX_FRAME)
> >>>> +	if (gbuffers < 2 || gbuffers > VB2_MAX_FRAME)
> >>>>   		gbuffers = 2;
> >>>>   	if (gbufsize > gbufsize_max)
> >>>>   		gbufsize = gbufsize_max;
> >>>> diff --git a/drivers/media/platform/mem2mem_testdev.c b/drivers/media/platform/mem2mem_testdev.c
> >>>> index c1b03cf..e1ff7e0 100644
> >>>> --- a/drivers/media/platform/mem2mem_testdev.c
> >>>> +++ b/drivers/media/platform/mem2mem_testdev.c
> >>>> @@ -55,7 +55,7 @@ MODULE_PARM_DESC(debug, "activates debug info");
> >>>>   #define MEM2MEM_NAME		"m2m-testdev"
> >>>>   
> >>>>   /* Per queue */
> >>>> -#define MEM2MEM_DEF_NUM_BUFS	VIDEO_MAX_FRAME
> >>>> +#define MEM2MEM_DEF_NUM_BUFS	VB2_MAX_FRAME
> >>>>   /* In bytes, per queue */
> >>>>   #define MEM2MEM_VID_MEM_LIMIT	(16 * 1024 * 1024)
> >>>>   
> >>>> diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c
> >>>> index 9a081c2..04e37c0 100644
> >>>> --- a/drivers/media/platform/ti-vpe/vpe.c
> >>>> +++ b/drivers/media/platform/ti-vpe/vpe.c
> >>>> @@ -1971,7 +1971,7 @@ static const struct v4l2_ctrl_config vpe_bufs_per_job = {
> >>>>   	.type = V4L2_CTRL_TYPE_INTEGER,
> >>>>   	.def = VPE_DEF_BUFS_PER_JOB,
> >>>>   	.min = 1,
> >>>> -	.max = VIDEO_MAX_FRAME,
> >>>> +	.max = VB2_MAX_FRAME,
> >>>>   	.step = 1,
> >>>>   };
> >>>>   
> >>>> diff --git a/drivers/media/platform/vivid/vivid-core.h b/drivers/media/platform/vivid/vivid-core.h
> >>>> index 811c286..c0375a1 100644
> >>>> --- a/drivers/media/platform/vivid/vivid-core.h
> >>>> +++ b/drivers/media/platform/vivid/vivid-core.h
> >>>> @@ -346,7 +346,7 @@ struct vivid_dev {
> >>>>   	/* video capture */
> >>>>   	struct tpg_data			tpg;
> >>>>   	unsigned			ms_vid_cap;
> >>>> -	bool				must_blank[VIDEO_MAX_FRAME];
> >>>> +	bool				must_blank[VB2_MAX_FRAME];
> >>>>   
> >>>>   	const struct vivid_fmt		*fmt_cap;
> >>>>   	struct v4l2_fract		timeperframe_vid_cap;
> >>>> diff --git a/drivers/media/platform/vivid/vivid-ctrls.c b/drivers/media/platform/vivid/vivid-ctrls.c
> >>>> index d5cbf00..7162f97 100644
> >>>> --- a/drivers/media/platform/vivid/vivid-ctrls.c
> >>>> +++ b/drivers/media/platform/vivid/vivid-ctrls.c
> >>>> @@ -332,7 +332,7 @@ static int vivid_vid_cap_s_ctrl(struct v4l2_ctrl *ctrl)
> >>>>   		break;
> >>>>   	case VIVID_CID_PERCENTAGE_FILL:
> >>>>   		tpg_s_perc_fill(&dev->tpg, ctrl->val);
> >>>> -		for (i = 0; i < VIDEO_MAX_FRAME; i++)
> >>>> +		for (i = 0; i < VB2_MAX_FRAME; i++)
> >>>>   			dev->must_blank[i] = ctrl->val < 100;
> >>>>   		break;
> >>>>   	case VIVID_CID_INSERT_SAV:
> >>>> diff --git a/drivers/media/platform/vivid/vivid-vid-cap.c b/drivers/media/platform/vivid/vivid-vid-cap.c
> >>>> index 331c544..55e8158 100644
> >>>> --- a/drivers/media/platform/vivid/vivid-vid-cap.c
> >>>> +++ b/drivers/media/platform/vivid/vivid-vid-cap.c
> >>>> @@ -252,7 +252,7 @@ static int vid_cap_start_streaming(struct vb2_queue *vq, unsigned count)
> >>>>   
> >>>>   	dev->vid_cap_seq_count = 0;
> >>>>   	dprintk(dev, 1, "%s\n", __func__);
> >>>> -	for (i = 0; i < VIDEO_MAX_FRAME; i++)
> >>>> +	for (i = 0; i < VB2_MAX_FRAME; i++)
> >>>>   		dev->must_blank[i] = tpg_g_perc_fill(&dev->tpg) < 100;
> >>>>   	if (dev->start_streaming_error) {
> >>>>   		dev->start_streaming_error = false;
> >>>> diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
> >>>> index 15b02f9..60354b4 100644
> >>>> --- a/drivers/media/v4l2-core/videobuf2-core.c
> >>>> +++ b/drivers/media/v4l2-core/videobuf2-core.c
> >>>> @@ -911,7 +911,7 @@ static int __reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
> >>>>   	/*
> >>>>   	 * Make sure the requested values and current defaults are sane.
> >>>>   	 */
> >>>> -	num_buffers = min_t(unsigned int, req->count, VIDEO_MAX_FRAME);
> >>>> +	num_buffers = min_t(unsigned int, req->count, VB2_MAX_FRAME);
> >>>>   	num_buffers = max_t(unsigned int, num_buffers, q->min_buffers_needed);
> >>>>   	memset(q->plane_sizes, 0, sizeof(q->plane_sizes));
> >>>>   	memset(q->alloc_ctx, 0, sizeof(q->alloc_ctx));
> >>>> @@ -1015,7 +1015,7 @@ static int __create_bufs(struct vb2_queue *q, struct v4l2_create_buffers *create
> >>>>   	unsigned int num_planes = 0, num_buffers, allocated_buffers;
> >>>>   	int ret;
> >>>>   
> >>>> -	if (q->num_buffers == VIDEO_MAX_FRAME) {
> >>>> +	if (q->num_buffers == VB2_MAX_FRAME) {
> >>>>   		dprintk(1, "maximum number of buffers already allocated\n");
> >>>>   		return -ENOBUFS;
> >>>>   	}
> >>>> @@ -1026,7 +1026,7 @@ static int __create_bufs(struct vb2_queue *q, struct v4l2_create_buffers *create
> >>>>   		q->memory = create->memory;
> >>>>   	}
> >>>>   
> >>>> -	num_buffers = min(create->count, VIDEO_MAX_FRAME - q->num_buffers);
> >>>> +	num_buffers = min(create->count, VB2_MAX_FRAME - q->num_buffers);
> >>>>   
> >>>>   	/*
> >>>>   	 * Ask the driver, whether the requested number of buffers, planes per
> >>>> @@ -2725,7 +2725,7 @@ struct vb2_fileio_data {
> >>>>   	struct v4l2_requestbuffers req;
> >>>>   	struct v4l2_plane p;
> >>>>   	struct v4l2_buffer b;
> >>>> -	struct vb2_fileio_buf bufs[VIDEO_MAX_FRAME];
> >>>> +	struct vb2_fileio_buf bufs[VB2_MAX_FRAME];
> >>>>   	unsigned int cur_index;
> >>>>   	unsigned int initial_index;
> >>>>   	unsigned int q_count;
> >>>> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> >>>> index a8608ce..66dcc40 100644
> >>>> --- a/include/media/videobuf2-core.h
> >>>> +++ b/include/media/videobuf2-core.h
> >>>> @@ -18,6 +18,8 @@
> >>>>   #include <linux/videodev2.h>
> >>>>   #include <linux/dma-buf.h>
> >>>>   
> >>>> +#define VB2_MAX_FRAME		64
> >>>> +
> >>>>   struct vb2_alloc_ctx;
> >>>>   struct vb2_fileio_data;
> >>>>   struct vb2_threadio_data;
> >>>> @@ -402,7 +404,7 @@ struct vb2_queue {
> >>>>   /* private: internal use only */
> >>>>   	struct mutex			mmap_lock;
> >>>>   	enum v4l2_memory		memory;
> >>>> -	struct vb2_buffer		*bufs[VIDEO_MAX_FRAME];
> >>>> +	struct vb2_buffer		*bufs[VB2_MAX_FRAME];
> >>>>   	unsigned int			num_buffers;
> >>>>   
> >>>>   	struct list_head		queued_list;
> >>
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-media" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] vb2: replace VIDEO_MAX_FRAME with VB2_MAX_FRAME

[Hans Verkuil]

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

Re: [PATCH] vb2: replace VIDEO_MAX_FRAME with VB2_MAX_FRAME

[Mauro Carvalho Chehab]

Em Wed, 29 Oct 2014 11:01:24 +0100
Hans Verkuil <hverkuil@xs4all.nl> escreveu:

> 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?

Yes.

> I am a bit afraid that that might break applications (especially if there
> are any that use bits in a 32-bit unsigned variable).

What 32-bits have to do with that? This is just the maximum number of
buffers, and not the number of bits.

> 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.

It is not driver dependent. It basically depends on the streaming logic.
Both VB and VB2 are free to set whatever size it is needed. They can
even change the logic to use a linked list, to avoid pre-allocating
anything.

Ok, there's actually a hardware limit, with is the maximum amount of
memory that could be used for DMA on a given hardware/architecture.

The 32 limit was just a random number that was chosen. Actually,
with V4L1 API, one struct were relying on it:

^1da177e4c3f4 (Linus Torvalds        2005-04-16 15:20:36 -0700 264) #define VIDEO_MAX_FRAME             32
^1da177e4c3f4 (Linus Torvalds        2005-04-16 15:20:36 -0700 265) 
^1da177e4c3f4 (Linus Torvalds        2005-04-16 15:20:36 -0700 266) struct video_mbuf
^1da177e4c3f4 (Linus Torvalds        2005-04-16 15:20:36 -0700 267) {
^1da177e4c3f4 (Linus Torvalds        2005-04-16 15:20:36 -0700 268)     int     size;           /* Total memory to map */
^1da177e4c3f4 (Linus Torvalds        2005-04-16 15:20:36 -0700 269)     int     frames;         /* Frames */
^1da177e4c3f4 (Linus Torvalds        2005-04-16 15:20:36 -0700 270)     int     offsets[VIDEO_MAX_FRAME];
^1da177e4c3f4 (Linus Torvalds        2005-04-16 15:20:36 -0700 271) };

But, for V4L2, this is just a number that can be used by apps when
then want to get the biggest number of buffers that the Kernel
supports.

> Basically I would like to deprecate VIDEO_MAX_FRAME.

As usual, this requires to first fix all applications that use it,
give a few years for them to stop using it. Then, it can be removed.

If you want to do so, you should start with v4l-utils:

$ git grep VIDEO_MAX_FRAME
contrib/freebsd/include/linux/videodev2.h:#define VIDEO_MAX_FRAME               32
include/linux/videodev2.h:#define VIDEO_MAX_FRAME               32
utils/v4l2-compliance/v4l-helpers.h:    __u32 mem_offsets[VIDEO_MAX_FRAME][VIDEO_MAX_PLANES];
utils/v4l2-compliance/v4l-helpers.h:    void *mmappings[VIDEO_MAX_FRAME][VIDEO_MAX_PLANES];
utils/v4l2-compliance/v4l-helpers.h:    unsigned long userptrs[VIDEO_MAX_FRAME][VIDEO_MAX_PLANES];
utils/v4l2-compliance/v4l-helpers.h:    int fds[VIDEO_MAX_FRAME][VIDEO_MAX_PLANES];
utils/v4l2-compliance/v4l-helpers.h:    for (i = 0; i < VIDEO_MAX_FRAME; i++)
utils/v4l2-compliance/v4l2-test-buffers.cpp:    fail_on_test(g_index() >= VIDEO_MAX_FRAME);
utils/v4l2-compliance/v4l2-test-buffers.cpp:                    buf.s_index(buf.g_index() + VIDEO_MAX_FRAME);
utils/v4l2-compliance/v4l2-test-buffers.cpp:                    buf.s_index(buf.g_index() - VIDEO_MAX_FRAME);
utils/v4l2-compliance/v4l2-test-formats.cpp:            fail_on_test(cap->readbuffers > VIDEO_MAX_FRAME);
utils/v4l2-compliance/v4l2-test-formats.cpp:            fail_on_test(out->writebuffers > VIDEO_MAX_FRAME);
utils/v4l2-ctl/v4l2-ctl-streaming.cpp:  for (i = 0; i < VIDEO_MAX_FRAME; i++) {
utils/v4l2-ctl/v4l2-ctl-streaming.cpp:          for (int i = 0; i < VIDEO_MAX_FRAME; i++)
utils/v4l2-ctl/v4l2-ctl-streaming.cpp:          for (int i = 0; i < VIDEO_MAX_FRAME; i++)
utils/v4l2-ctl/v4l2-ctl-streaming.cpp:  struct v4l2_plane planes[VIDEO_MAX_FRAME][VIDEO_MAX_PLANES];
utils/v4l2-ctl/v4l2-ctl-streaming.cpp:  void *bufs[VIDEO_MAX_FRAME][VIDEO_MAX_PLANES];
utils/v4l2-ctl/v4l2-ctl-streaming.cpp:  int fds[VIDEO_MAX_FRAME][VIDEO_MAX_PLANES];

Regards,
Mauro

Re: [PATCH] vb2: replace VIDEO_MAX_FRAME with VB2_MAX_FRAME

[Laurent Pinchart]

Hi Mauro,

On Wednesday 29 October 2014 10:40:33 Mauro Carvalho Chehab wrote:
> Em Wed, 29 Oct 2014 11:01:24 +0100 Hans Verkuil escreveu:
> > On 10/29/14 10:13, Mauro Carvalho Chehab wrote:
> > > Em Wed, 29 Oct 2014 09:59:56 +0100 Hans Verkuil escreveu:
> > >> On 10/29/14 09:29, Mauro Carvalho Chehab wrote:
> > >>> Em Wed, 29 Oct 2014 08:29:08 +0100 Hans Verkuil escreveu:
> > >>>> On 10/28/2014 07:26 PM, Mauro Carvalho Chehab wrote:
> > >>>>> Em Fri, 10 Oct 2014 10:04:58 +0200 Hans Verkuil 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?
> 
> Yes.
> 
> > I am a bit afraid that that might break applications (especially if there
> > are any that use bits in a 32-bit unsigned variable).
> 
> What 32-bits have to do with that? This is just the maximum number of
> buffers, and not the number of bits.

Applications might use a bitmask to track buffers.

> > 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.
> 
> It is not driver dependent. It basically depends on the streaming logic.
> Both VB and VB2 are free to set whatever size it is needed. They can
> even change the logic to use a linked list, to avoid pre-allocating
> anything.
> 
> Ok, there's actually a hardware limit, with is the maximum amount of
> memory that could be used for DMA on a given hardware/architecture.
> 
> The 32 limit was just a random number that was chosen.

So, can't we just mark VIDEO_MAX_FRAME as deprecated ? We can't remove it as 
applications might depend on it, but it's pretty useless otherwise.

> Actually, with V4L1 API, one struct were relying on it:
> 
> ^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700 264) #define
> VIDEO_MAX_FRAME             32
> ^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700 265)
> ^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700 266) struct
> video_mbuf
> ^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700 267) {
> ^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700 268)     int    
> size;          /* Total memory to map */
> ^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700 269)     int    
> frames;         /* Frames */
> ^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700 270)     int    
> offsets[VIDEO_MAX_FRAME];
> ^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700 271) };
> 
> But, for V4L2, this is just a number that can be used by apps when
> then want to get the biggest number of buffers that the Kernel
> supports.
> 
> > Basically I would like to deprecate VIDEO_MAX_FRAME.
> 
> As usual, this requires to first fix all applications that use it,
> give a few years for them to stop using it. Then, it can be removed.
> 
> If you want to do so, you should start with v4l-utils:
> 
> $ git grep VIDEO_MAX_FRAME
> contrib/freebsd/include/linux/videodev2.h:#define VIDEO_MAX_FRAME           
>    32 include/linux/videodev2.h:#define VIDEO_MAX_FRAME               32
> utils/v4l2-compliance/v4l-helpers.h:    __u32
> mem_offsets[VIDEO_MAX_FRAME][VIDEO_MAX_PLANES];
> utils/v4l2-compliance/v4l-helpers.h:    void
> *mmappings[VIDEO_MAX_FRAME][VIDEO_MAX_PLANES];
> utils/v4l2-compliance/v4l-helpers.h:    unsigned long
> userptrs[VIDEO_MAX_FRAME][VIDEO_MAX_PLANES];
> utils/v4l2-compliance/v4l-helpers.h:    int
> fds[VIDEO_MAX_FRAME][VIDEO_MAX_PLANES];
> utils/v4l2-compliance/v4l-helpers.h:    for (i = 0; i < VIDEO_MAX_FRAME;
> i++) utils/v4l2-compliance/v4l2-test-buffers.cpp:    fail_on_test(g_index()
> >= VIDEO_MAX_FRAME); utils/v4l2-compliance/v4l2-test-buffers.cpp:          
>          buf.s_index(buf.g_index() + VIDEO_MAX_FRAME);
> utils/v4l2-compliance/v4l2-test-buffers.cpp:                   
> buf.s_index(buf.g_index() - VIDEO_MAX_FRAME);
> utils/v4l2-compliance/v4l2-test-formats.cpp:           
> fail_on_test(cap->readbuffers > VIDEO_MAX_FRAME);
> utils/v4l2-compliance/v4l2-test-formats.cpp:           
> fail_on_test(out->writebuffers > VIDEO_MAX_FRAME);
> utils/v4l2-ctl/v4l2-ctl-streaming.cpp:  for (i = 0; i < VIDEO_MAX_FRAME;
> i++) { utils/v4l2-ctl/v4l2-ctl-streaming.cpp:          for (int i = 0; i <
> VIDEO_MAX_FRAME; i++) utils/v4l2-ctl/v4l2-ctl-streaming.cpp:          for
> (int i = 0; i < VIDEO_MAX_FRAME; i++)
> utils/v4l2-ctl/v4l2-ctl-streaming.cpp:  struct v4l2_plane
> planes[VIDEO_MAX_FRAME][VIDEO_MAX_PLANES];
> utils/v4l2-ctl/v4l2-ctl-streaming.cpp:  void
> *bufs[VIDEO_MAX_FRAME][VIDEO_MAX_PLANES];
> utils/v4l2-ctl/v4l2-ctl-streaming.cpp:  int
> fds[VIDEO_MAX_FRAME][VIDEO_MAX_PLANES];

-- 
Regards,

Laurent Pinchart

Re: [PATCH] vb2: replace VIDEO_MAX_FRAME with VB2_MAX_FRAME

[Mauro Carvalho Chehab]

Em Wed, 29 Oct 2014 14:46:55 +0200
Laurent Pinchart <laurent.pinchart@ideasonboard.com> escreveu:

> > > Hmm, so you think VIDEO_MAX_FRAME should just be updated to 64?
> > 
> > Yes.
> > 
> > > I am a bit afraid that that might break applications (especially if there
> > > are any that use bits in a 32-bit unsigned variable).
> > 
> > What 32-bits have to do with that? This is just the maximum number of
> > buffers, and not the number of bits.
> 
> Applications might use a bitmask to track buffers.

True, but then it should be limiting the max buffer to 32, if the 
implementation won't support more than 32 bits at its bitmask
implementation.

Anyway, we need to double check if nothing will break at the open
source apps before being able to change its value.

> 
> > > 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.
> > 
> > It is not driver dependent. It basically depends on the streaming logic.
> > Both VB and VB2 are free to set whatever size it is needed. They can
> > even change the logic to use a linked list, to avoid pre-allocating
> > anything.
> > 
> > Ok, there's actually a hardware limit, with is the maximum amount of
> > memory that could be used for DMA on a given hardware/architecture.
> > 
> > The 32 limit was just a random number that was chosen.
> 
> So, can't we just mark VIDEO_MAX_FRAME as deprecated ? We can't remove it as 
> applications might depend on it, but it's pretty useless otherwise.

As I pointed below, even the applications _we_ wrote at v4l-utils use
it. The good news is that I double-checked xawtv3, xawtv4 and tvtime:
none of them use it. Perhaps we're lucky enough, but I wouldn't count
with that.

Ok, we can always write a note there saying that this is deprecated,
but the same symbol is still used internally on the drivers.

If we're willing to deprecate, we should do something like:

#ifndef __KERNEL__
	/* This define is deprecated because (...) */
	#define VIDEO_MAX_FRAME	32
#endif

And then remove all occurrences of it at Kernelspace.

We should also first fix v4l-utils no not use it, as v4l-utils is
currently the reference code for users. Please notice, however, that
v4l-compliance depends on it. I suspect that it wants/needs to test
the maximum buffer size. What would be a reasonable way to replace
it, and still be able to test the maximum buffer limit?

Regards,
Mauro

Re: [PATCH] vb2: replace VIDEO_MAX_FRAME with VB2_MAX_FRAME

[Laurent Pinchart]

Hi Mauro,

On Wednesday 29 October 2014 11:05:34 Mauro Carvalho Chehab wrote:
> Em Wed, 29 Oct 2014 14:46:55 +0200 Laurent Pinchart escreveu:
> > > > Hmm, so you think VIDEO_MAX_FRAME should just be updated to 64?
> > > 
> > > Yes.
> > > 
> > > > I am a bit afraid that that might break applications (especially if
> > > > there are any that use bits in a 32-bit unsigned variable).
> > > 
> > > What 32-bits have to do with that? This is just the maximum number of
> > > buffers, and not the number of bits.
> > 
> > Applications might use a bitmask to track buffers.
> 
> True, but then it should be limiting the max buffer to 32, if the
> implementation won't support more than 32 bits at its bitmask
> implementation.
> 
> Anyway, we need to double check if nothing will break at the open
> source apps before being able to change its value.

I don't think we should change the value of VIDEO_MAX_FRAME. Applications that 
rely on it will thus allocate a maximum of 32 buffers, nothing should break 
(provided that no driver requires a minimum number of buffers higher than 32).

> > > > 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.
> > > 
> > > It is not driver dependent. It basically depends on the streaming logic.
> > > Both VB and VB2 are free to set whatever size it is needed. They can
> > > even change the logic to use a linked list, to avoid pre-allocating
> > > anything.
> > > 
> > > Ok, there's actually a hardware limit, with is the maximum amount of
> > > memory that could be used for DMA on a given hardware/architecture.
> > > 
> > > The 32 limit was just a random number that was chosen.
> > 
> > So, can't we just mark VIDEO_MAX_FRAME as deprecated ? We can't remove it
> > as applications might depend on it, but it's pretty useless otherwise.
>
> As I pointed below, even the applications _we_ wrote at v4l-utils use
> it. The good news is that I double-checked xawtv3, xawtv4 and tvtime:
> none of them use it. Perhaps we're lucky enough, but I wouldn't count
> with that.
> 
> Ok, we can always write a note there saying that this is deprecated,
> but the same symbol is still used internally on the drivers.
> 
> If we're willing to deprecate, we should do something like:
> 
> #ifndef __KERNEL__
> 	/* This define is deprecated because (...) */
> 	#define VIDEO_MAX_FRAME	32
> #endif
> 
> And then remove all occurrences of it at Kernelspace.

Agreed.

> We should also first fix v4l-utils no not use it, as v4l-utils is currently
> the reference code for users.

That sounds reasonable to me. There's no urgency, as nothing will break if an 
application uses VIDEO_MAX_FRAME set to 32 while VB2 can support 64, but we 
should still remove references to VIDEO_MAX_FRAME from v4l-utils.

> Please notice, however, that v4l-compliance depends on it. I suspect that it
> wants/needs to test the maximum buffer size. What would be a reasonable way
> to replace it, and still be able to test the maximum buffer limit?

I'll let Hans comment on that.

-- 
Regards,

Laurent Pinchart

Re: [PATCH] vb2: replace VIDEO_MAX_FRAME with VB2_MAX_FRAME

[Hans Verkuil]

On 10/29/14 14:17, Laurent Pinchart wrote:
> Hi Mauro,
> 
> On Wednesday 29 October 2014 11:05:34 Mauro Carvalho Chehab wrote:
>> Em Wed, 29 Oct 2014 14:46:55 +0200 Laurent Pinchart escreveu:
>>>>> Hmm, so you think VIDEO_MAX_FRAME should just be updated to 64?
>>>>
>>>> Yes.
>>>>
>>>>> I am a bit afraid that that might break applications (especially if
>>>>> there are any that use bits in a 32-bit unsigned variable).
>>>>
>>>> What 32-bits have to do with that? This is just the maximum number of
>>>> buffers, and not the number of bits.
>>>
>>> Applications might use a bitmask to track buffers.
>>
>> True, but then it should be limiting the max buffer to 32, if the
>> implementation won't support more than 32 bits at its bitmask
>> implementation.
>>
>> Anyway, we need to double check if nothing will break at the open
>> source apps before being able to change its value.
> 
> I don't think we should change the value of VIDEO_MAX_FRAME. Applications that 
> rely on it will thus allocate a maximum of 32 buffers, nothing should break 
> (provided that no driver requires a minimum number of buffers higher than 32).
> 
>>>>> 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.
>>>>
>>>> It is not driver dependent. It basically depends on the streaming logic.
>>>> Both VB and VB2 are free to set whatever size it is needed. They can
>>>> even change the logic to use a linked list, to avoid pre-allocating
>>>> anything.
>>>>
>>>> Ok, there's actually a hardware limit, with is the maximum amount of
>>>> memory that could be used for DMA on a given hardware/architecture.
>>>>
>>>> The 32 limit was just a random number that was chosen.
>>>
>>> So, can't we just mark VIDEO_MAX_FRAME as deprecated ? We can't remove it
>>> as applications might depend on it, but it's pretty useless otherwise.
>>
>> As I pointed below, even the applications _we_ wrote at v4l-utils use
>> it. The good news is that I double-checked xawtv3, xawtv4 and tvtime:
>> none of them use it. Perhaps we're lucky enough, but I wouldn't count
>> with that.
>>
>> Ok, we can always write a note there saying that this is deprecated,
>> but the same symbol is still used internally on the drivers.
>>
>> If we're willing to deprecate, we should do something like:
>>
>> #ifndef __KERNEL__
>> 	/* This define is deprecated because (...) */
>> 	#define VIDEO_MAX_FRAME	32
>> #endif
>>
>> And then remove all occurrences of it at Kernelspace.

No problem.

> 
> Agreed.
> 
>> We should also first fix v4l-utils no not use it, as v4l-utils is currently
>> the reference code for users.
> 
> That sounds reasonable to me. There's no urgency, as nothing will break if an 
> application uses VIDEO_MAX_FRAME set to 32 while VB2 can support 64, but we 
> should still remove references to VIDEO_MAX_FRAME from v4l-utils.
> 
>> Please notice, however, that v4l-compliance depends on it. I suspect that it
>> wants/needs to test the maximum buffer size. What would be a reasonable way
>> to replace it, and still be able to test the maximum buffer limit?
> 
> I'll let Hans comment on that.

None of this is difficult to do. And it would most likely improve the code as well.

OK. How about this:

1) Remove the use of VIDEO_MAX_FRAME in the kernel by introducing VB1_MAX_FRAME
   and VB2_MAX_FRAME defines, initially both are set to 32.
2) Remove the use of VIDEO_MAX_FRAME in v4l-utils.
3) Patch VB2_MAX_FRAME to 64 and update the spec.

Regards,

	Hans

Re: [PATCH] vb2: replace VIDEO_MAX_FRAME with VB2_MAX_FRAME

[Mauro Carvalho Chehab]

Em Wed, 29 Oct 2014 14:53:53 +0100
Hans Verkuil <hverkuil@xs4all.nl> escreveu:

> On 10/29/14 14:17, Laurent Pinchart wrote:
> > Hi Mauro,
> > 
> > On Wednesday 29 October 2014 11:05:34 Mauro Carvalho Chehab wrote:
> >> Em Wed, 29 Oct 2014 14:46:55 +0200 Laurent Pinchart escreveu:
> >>>>> Hmm, so you think VIDEO_MAX_FRAME should just be updated to 64?
> >>>>
> >>>> Yes.
> >>>>
> >>>>> I am a bit afraid that that might break applications (especially if
> >>>>> there are any that use bits in a 32-bit unsigned variable).
> >>>>
> >>>> What 32-bits have to do with that? This is just the maximum number of
> >>>> buffers, and not the number of bits.
> >>>
> >>> Applications might use a bitmask to track buffers.
> >>
> >> True, but then it should be limiting the max buffer to 32, if the
> >> implementation won't support more than 32 bits at its bitmask
> >> implementation.
> >>
> >> Anyway, we need to double check if nothing will break at the open
> >> source apps before being able to change its value.
> > 
> > I don't think we should change the value of VIDEO_MAX_FRAME. Applications that 
> > rely on it will thus allocate a maximum of 32 buffers, nothing should break 
> > (provided that no driver requires a minimum number of buffers higher than 32).
> > 
> >>>>> 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.
> >>>>
> >>>> It is not driver dependent. It basically depends on the streaming logic.
> >>>> Both VB and VB2 are free to set whatever size it is needed. They can
> >>>> even change the logic to use a linked list, to avoid pre-allocating
> >>>> anything.
> >>>>
> >>>> Ok, there's actually a hardware limit, with is the maximum amount of
> >>>> memory that could be used for DMA on a given hardware/architecture.
> >>>>
> >>>> The 32 limit was just a random number that was chosen.
> >>>
> >>> So, can't we just mark VIDEO_MAX_FRAME as deprecated ? We can't remove it
> >>> as applications might depend on it, but it's pretty useless otherwise.
> >>
> >> As I pointed below, even the applications _we_ wrote at v4l-utils use
> >> it. The good news is that I double-checked xawtv3, xawtv4 and tvtime:
> >> none of them use it. Perhaps we're lucky enough, but I wouldn't count
> >> with that.
> >>
> >> Ok, we can always write a note there saying that this is deprecated,
> >> but the same symbol is still used internally on the drivers.
> >>
> >> If we're willing to deprecate, we should do something like:
> >>
> >> #ifndef __KERNEL__
> >> 	/* This define is deprecated because (...) */
> >> 	#define VIDEO_MAX_FRAME	32
> >> #endif
> >>
> >> And then remove all occurrences of it at Kernelspace.
> 
> No problem.
> 
> > 
> > Agreed.
> > 
> >> We should also first fix v4l-utils no not use it, as v4l-utils is currently
> >> the reference code for users.
> > 
> > That sounds reasonable to me. There's no urgency, as nothing will break if an 
> > application uses VIDEO_MAX_FRAME set to 32 while VB2 can support 64, but we 
> > should still remove references to VIDEO_MAX_FRAME from v4l-utils.
> > 
> >> Please notice, however, that v4l-compliance depends on it. I suspect that it
> >> wants/needs to test the maximum buffer size. What would be a reasonable way
> >> to replace it, and still be able to test the maximum buffer limit?
> > 
> > I'll let Hans comment on that.
> 
> None of this is difficult to do. And it would most likely improve the code as well.
> 
> OK. How about this:
> 
> 1) Remove the use of VIDEO_MAX_FRAME in the kernel by introducing VB1_MAX_FRAME
>    and VB2_MAX_FRAME defines, initially both are set to 32.

Why to have two separate macros Kernelside? Just use VB_MAX_FRAME for both.

> 2) Remove the use of VIDEO_MAX_FRAME in v4l-utils.
> 3) Patch VB2_MAX_FRAME to 64 and update the spec.

Steps (2) and (3) sounds ok to me, provided that we keep using the same
macro. If are there any specific issue on some driver that can't support
64 buffers, then we can just add a driver-specific maximum value and use
it instead.

Regards,
Mauro
> 
> Regards,
> 
> 	Hans
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html