casaroli commented on code in PR #18353:
URL: https://github.com/apache/nuttx/pull/18353#discussion_r4392214018
##########
drivers/video/v4l2_m2m.c:
##########
@@ -389,28 +468,60 @@ static int codec_qbuf(FAR struct file *filep,
return -EAGAIN;
}
- memcpy(&container->buf, buf, sizeof(struct v4l2_buffer));
- if (buf->memory == V4L2_MEMORY_MMAP)
+ memset(&format, 0, sizeof(format));
+ if (V4L2_TYPE_IS_OUTPUT(buf->type))
+ {
+ CODEC_OUTPUT_G_FMT(cmng->codec, cfile->priv, &format);
+ }
+ else
{
- /* only use userptr inside the container */
+ offset = CAPTURE_BUF_OFFSET;
+ CODEC_CAPTURE_G_FMT(cmng->codec, cfile->priv, &format);
+ }
- if (V4L2_TYPE_IS_OUTPUT(buf->type))
+ memcpy(&container->buf, buf, sizeof(struct v4l2_buffer));
+
+ if (V4L2_TYPE_IS_MULTIPLANAR(buf->type))
+ {
+ memcpy(container->planes, buf->m.planes,
+ sizeof(struct v4l2_plane) * format.fmt.pix_mp.num_planes);
+ container->buf.m.planes = container->planes;
+ for (i = 0; i < format.fmt.pix_mp.num_planes; i++)
{
- buf_size = CODEC_OUTPUT_G_BUFSIZE(cmng->codec, cfile->priv);
+ switch (container->buf.memory)
+ {
+ case V4L2_MEMORY_MMAP:
+ container->buf.m.planes[i].m.vaddr =
+ (FAR void *)(buf->m.planes[i].m.mem_offset -
+ offset + type_inf->bufheap);
+ break;
+
+ case V4L2_MEMORY_USERPTR:
+ container->buf.m.planes[i].m.vaddr =
+ (FAR void *)buf->m.planes[i].m.userptr;
+ break;
+
+ default:
+ return -EINVAL;
+ }
}
- else
+ }
+ else
+ {
+ switch (buf->memory)
{
- buf_size = CODEC_CAPTURE_G_BUFSIZE(cmng->codec, cfile->priv);
- }
+ case V4L2_MEMORY_MMAP:
+ container->buf.m.vaddr =
+ (FAR void *)(type_inf->bufheap + buf->m.offset - offset);
Review Comment:
`buf->m.offset` comes from the user and is not checked, so the codec can
access any kernel address (also `mem_offset` at line 495).
Fix: return `-EINVAL` unless the offset is inside `bufheap` and `buf->index`
is less than the container count.
##########
drivers/video/v4l2_m2m.c:
##########
@@ -428,9 +539,15 @@ static int codec_qbuf(FAR struct file *filep,
static int codec_dqbuf(FAR struct file *filep,
FAR struct v4l2_buffer *buf)
{
+ FAR struct v4l2_plane *plans = buf->m.planes;
Review Comment:
`buf->m.planes` is read before the `buf == NULL` check.
Fix: read it after the check.
##########
drivers/video/v4l2_m2m.c:
##########
@@ -389,28 +468,60 @@ static int codec_qbuf(FAR struct file *filep,
return -EAGAIN;
}
- memcpy(&container->buf, buf, sizeof(struct v4l2_buffer));
- if (buf->memory == V4L2_MEMORY_MMAP)
+ memset(&format, 0, sizeof(format));
+ if (V4L2_TYPE_IS_OUTPUT(buf->type))
+ {
+ CODEC_OUTPUT_G_FMT(cmng->codec, cfile->priv, &format);
+ }
+ else
{
- /* only use userptr inside the container */
+ offset = CAPTURE_BUF_OFFSET;
+ CODEC_CAPTURE_G_FMT(cmng->codec, cfile->priv, &format);
+ }
- if (V4L2_TYPE_IS_OUTPUT(buf->type))
+ memcpy(&container->buf, buf, sizeof(struct v4l2_buffer));
+
+ if (V4L2_TYPE_IS_MULTIPLANAR(buf->type))
+ {
+ memcpy(container->planes, buf->m.planes,
+ sizeof(struct v4l2_plane) * format.fmt.pix_mp.num_planes);
+ container->buf.m.planes = container->planes;
+ for (i = 0; i < format.fmt.pix_mp.num_planes; i++)
{
- buf_size = CODEC_OUTPUT_G_BUFSIZE(cmng->codec, cfile->priv);
+ switch (container->buf.memory)
+ {
+ case V4L2_MEMORY_MMAP:
+ container->buf.m.planes[i].m.vaddr =
+ (FAR void *)(buf->m.planes[i].m.mem_offset -
+ offset + type_inf->bufheap);
+ break;
+
+ case V4L2_MEMORY_USERPTR:
+ container->buf.m.planes[i].m.vaddr =
+ (FAR void *)buf->m.planes[i].m.userptr;
+ break;
+
+ default:
+ return -EINVAL;
+ }
}
- else
+ }
+ else
+ {
+ switch (buf->memory)
{
- buf_size = CODEC_CAPTURE_G_BUFSIZE(cmng->codec, cfile->priv);
- }
+ case V4L2_MEMORY_MMAP:
+ container->buf.m.vaddr =
+ (FAR void *)(type_inf->bufheap + buf->m.offset - offset);
Review Comment:
The `default: return -EINVAL;` paths do not give back the container taken
before the `switch`, so each bad `VIDIOC_QBUF` loses one.
Fix: check `buf->memory` before `video_framebuff_get_container()`.
##########
drivers/video/v4l2_cap.c:
##########
@@ -1330,6 +1339,11 @@ static size_t get_bufsize(FAR video_format_t *vf)
uint32_t height = vf->height;
size_t ret = width * height;
+ if (vf->sizeimage)
Review Comment:
`sizeimage` from the user is used for every format, so a small value for
YUYV or RGB565 gives buffers smaller than one frame.
Fix: use it only for JPEG and ENTROPY, or take `MAX(vf->sizeimage, computed
size)`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
