xiaoxiang781216 commented on code in PR #7687: URL: https://github.com/apache/nuttx/pull/7687#discussion_r1040341497
########## drivers/video/video.c: ########## @@ -134,6 +137,9 @@ struct video_mng_s uint8_t open_num; video_type_inf_t video_inf; video_type_inf_t still_inf; + FAR void *vmem_heap; /* for V4L2_MEMORY_MMAP buffers */ Review Comment: vmem_heap need move to video_type_inf_s ########## drivers/video/video.c: ########## @@ -134,6 +137,9 @@ struct video_mng_s uint8_t open_num; video_type_inf_t video_inf; video_type_inf_t still_inf; + FAR void *vmem_heap; /* for V4L2_MEMORY_MMAP buffers */ + uint32_t buf_size; + uint16_t buf_count; Review Comment: remove, since we can get from video_framebuff_t::container_size ########## drivers/video/video.c: ########## @@ -1172,6 +1184,16 @@ static int video_reqbufs(FAR struct video_mng_s *vmng, } else { + if (reqbufs->memory == V4L2_MEMORY_MMAP) Review Comment: remove the check since the config should work for non-map mode too. ########## drivers/video/video.c: ########## @@ -1172,6 +1184,16 @@ static int video_reqbufs(FAR struct video_mng_s *vmng, } else { + if (reqbufs->memory == V4L2_MEMORY_MMAP) + { + vmng->buf_size = + video_fmt_buf_size(&type_inf->fmt[VIDEO_FMT_MAIN]); + if (reqbufs->count * vmng->buf_size > MAX_VIDEO_HEAP_SIZE) Review Comment: Let's add new config VIDEO_REQBUFS_COUNT_MAX to limit the count directly ########## drivers/video/video.c: ########## @@ -1180,9 +1202,51 @@ static int video_reqbufs(FAR struct video_mng_s *vmng, leave_critical_section(flags); + if ((ret == OK) && (reqbufs->memory == V4L2_MEMORY_MMAP)) + { + vmng->buf_count = reqbufs->count; + if (vmng->vmem_heap != NULL) + { + kmm_free(vmng->vmem_heap); Review Comment: let's use kmm_realloc instead kmm_free/kmm_malloc ########## drivers/video/video.c: ########## @@ -1180,9 +1202,51 @@ static int video_reqbufs(FAR struct video_mng_s *vmng, leave_critical_section(flags); + if ((ret == OK) && (reqbufs->memory == V4L2_MEMORY_MMAP)) + { + vmng->buf_count = reqbufs->count; + if (vmng->vmem_heap != NULL) + { + kmm_free(vmng->vmem_heap); + } + + vmng->vmem_heap = kmm_memalign(32, vmng->buf_count * vmng->buf_size); + if (vmng->vmem_heap == NULL) + { + ret = -ENOMEM; + } + } + return ret; } +static int video_querybuf(FAR struct video_mng_s *vmng, + FAR struct v4l2_buffer *buf) +{ + FAR video_type_inf_t *type_inf; + + if ((vmng == NULL) || (buf == NULL) || (buf->memory != V4L2_MEMORY_MMAP)) Review Comment: ```suggestion if (vmng == NULL || buf == NULL || buf->memory != V4L2_MEMORY_MMAP) ``` ########## drivers/video/video.c: ########## @@ -1180,9 +1202,51 @@ static int video_reqbufs(FAR struct video_mng_s *vmng, leave_critical_section(flags); + if ((ret == OK) && (reqbufs->memory == V4L2_MEMORY_MMAP)) Review Comment: let's move into else clause and before line 1197 ########## drivers/video/video.c: ########## @@ -978,6 +985,11 @@ static void cleanup_resources(FAR video_mng_t *vmng) cleanup_streamresources(&vmng->video_inf); cleanup_streamresources(&vmng->still_inf); cleanup_scenes_parameter(); + if (vmng->vmem_heap != NULL) Review Comment: move to cleanup_streamresources ########## drivers/video/video.c: ########## @@ -134,6 +137,9 @@ struct video_mng_s uint8_t open_num; video_type_inf_t video_inf; video_type_inf_t still_inf; + FAR void *vmem_heap; /* for V4L2_MEMORY_MMAP buffers */ + uint32_t buf_size; Review Comment: remove, let's save to v4l2_buffer::length or compute every time -- 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: commits-unsubscr...@nuttx.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org