Skip to contentSkip to conversation

v4l2-ioctl: don't set PRIV_MAGIC unconditionally in g_fmt()

PATCH Superseded Hans Verkuil 2 participants 3 messages Jul 20, 2014 – Jul 21, 2014 Media (V4L/DVB)

v1 of 4 Superseded Latest is v4 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 v4l2-ioctl: don't set PRIV_MAGIC unconditionally in g_fmt()

Regression fix:

V4L2_PIX_FMT_PRIV_MAGIC should only be set for the VIDEO_CAPTURE and VIDEO_OUTPUT buffer types, and not for any others. In the case of the win format this overwrites a pointer value that is passed in from userspace.

Since it is already set for the VIDEO_CAPTURE and VIDEO_OUTPUT cases anyway this line can just be dropped.

---
 drivers/media/v4l2-core/v4l2-ioctl.c | 2 --
 1 file changed, 2 deletions(-)
drivers/media/v4l2-core/v4l2-ioctl.c +0 -2
index e620387..c11a13d 100644
@@ -1143,8 +1143,6 @@ static int v4l_g_fmt(const struct v4l2_ioctl_ops *ops,
 	bool is_tx = vfd->vfl_dir != VFL_DIR_RX;
 	int ret;
 
-	p->fmt.pix.priv = V4L2_PIX_FMT_PRIV_MAGIC;
-
 	/*
 	 * fmt can't be cleared for these overlay types due to the 'clips'
 	 * 'clipcount' and 'bitmap' pointers in struct v4l2_window.
Signature · 1 line
2.0.0
#1 lore raw

Hi Hans,

Thank you for the patch.

Quoted 9 lines · On Sunday 20 July 2014 15:10:03 Hans Verkuil wrote:

Regression fix:

V4L2_PIX_FMT_PRIV_MAGIC should only be set for the VIDEO_CAPTURE and VIDEO_OUTPUT buffer types, and not for any others. In the case of the win format this overwrites a pointer value that is passed in from userspace.

Since it is already set for the VIDEO_CAPTURE and VIDEO_OUTPUT cases anyway this line can just be dropped.

It's set after calling the vidioc_g_fmt_vid_cap or vidioc_g_fmt_vid_out operation, which means that driver will not see the flag being set. Couldn't that be an issue ?

Quoted 18 lines
---
 drivers/media/v4l2-core/v4l2-ioctl.c | 2 --
 1 file changed, 2 deletions(-)
(quoted diff) +0 -0

b/drivers/media/v4l2-core/v4l2-ioctl.c index e620387..c11a13d 100644

drivers/media/v4l2-core/v4l2-ioctl.c +0 -2
@@ -1143,8 +1143,6 @@ static int v4l_g_fmt(const struct v4l2_ioctl_ops *ops,
 bool is_tx = vfd->vfl_dir != VFL_DIR_RX;
 	int ret;
 
-	p->fmt.pix.priv = V4L2_PIX_FMT_PRIV_MAGIC;
-
 	/*
 	 * fmt can't be cleared for these overlay types due to the 'clips'
 	 * 'clipcount' and 'bitmap' pointers in struct v4l2_window.
Signature · 3 lines
Regards,

Laurent Pinchart
#2 lore raw
Quoted 12 lines · Laurent Pinchart

Hi Hans,

Thank you for the patch.

Quoted 7 lines · On Sunday 20 July 2014 15:10:03 Hans Verkuil wrote:

Regression fix:

V4L2_PIX_FMT_PRIV_MAGIC should only be set for the VIDEO_CAPTURE and VIDEO_OUTPUT buffer types, and not for any others. In the case of the win format this overwrites a pointer value that is passed in from userspace.

Quoted 6 lines
Quoted 2 lines

Since it is already set for the VIDEO_CAPTURE and VIDEO_OUTPUT cases anyway this line can just be dropped.

It's set after calling the vidioc_g_fmt_vid_cap or vidioc_g_fmt_vid_out operation, which means that driver will not see the flag being set. Couldn't that be an issue ?

While I don't think it is necessary as such, it is better for consistency. I'll post a new patch.

Regards,

Hans

Quoted 20 lines
Quoted 19 lines
---
 drivers/media/v4l2-core/v4l2-ioctl.c | 2 --
 1 file changed, 2 deletions(-)
(quoted diff) +0 -0

b/drivers/media/v4l2-core/v4l2-ioctl.c index e620387..c11a13d 100644

drivers/media/v4l2-core/v4l2-ioctl.c +0 -2
@@ -1143,8 +1143,6 @@ static int v4l_g_fmt(const struct v4l2_ioctl_ops *ops,
 bool is_tx = vfd->vfl_dir != VFL_DIR_RX;
 	int ret;
 
-	p->fmt.pix.priv = V4L2_PIX_FMT_PRIV_MAGIC;
-
 	/*
 	 * fmt can't be cleared for these overlay types due to the 'clips'
 	 * 'clipcount' and 'bitmap' pointers in struct v4l2_window.