media: venus: add support for selection rectangles
Subsystems (4)
PATCH v2 1/1 media: venus: add support for selection rectangles
Handles target type crop by setting the new active rectangle to hardware. The new rectangle should be within YUV size.
--- drivers/media/platform/qcom/venus/venc.c | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-)
drivers/media/platform/qcom/venus/venc.c +22 -4
@@ -478,16 +478,34 @@ static int venc_g_fmt(struct file *file, void *fh, struct v4l2_format *f) venc_s_selection(struct file *file, void *fh, struct v4l2_selection *s) { struct venus_inst *inst = to_inst(file); +int ret; +u32 buftype;if (s->type !=V4L2_BUF_TYPE_VIDEO_OUTPUT )return -EINVAL ;switch (s->target) {case V4L2_SEL_TGT_CROP : -if (s->r.width != inst->out_width || - s->r.height != inst->out_height || - s->r.top !=0 || s->r.left !=0 ) -return -EINVAL ; +if (s->r.left !=0 ) { + s->r.width += s->r.left; + s->r.left =0 ; + } + +if (s->r.top !=0 ) { + s->r.height += s->r.top; + s->r.top =0 ; + } + +if (s->r.width > inst->width) + s->r.width = inst->width; +else + inst->width = s->r.width; + +if (s->r.height > inst->height) + s->r.height = inst->height; +else + inst->height = s->r.height; +break ;default :return -EINVAL ;
Signature · 1 line
1.9.1
Hi Malathi,
Quoted 50 lines · Malathi Gottam
Handles target type crop by setting the new active rectangle to hardware. The new rectangle should be within YUV size.
Signed-off-by Malathi Gottam <[email protected]>--- drivers/media/platform/qcom/venus/venc.c | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-)drivers/media/platform/qcom/venus/venc.c +22
-4@@ -478,16 +478,34 @@ static int venc_g_fmt(struct file *file, void *fh, struct v4l2_format *f) venc_s_selection(struct file *file, void *fh, struct v4l2_selection *s) { struct venus_inst *inst = to_inst(file); +int ret; +u32 buftype;if (s->type !=V4L2_BUF_TYPE_VIDEO_OUTPUT )return -EINVAL ;switch (s->target) {case V4L2_SEL_TGT_CROP : -if (s->r.width != inst->out_width || - s->r.height != inst->out_height || - s->r.top !=0 || s->r.left !=0 ) -return -EINVAL ; +if (s->r.left !=0 ) { + s->r.width += s->r.left; + s->r.left =0 ; + } + +if (s->r.top !=0 ) { + s->r.height += s->r.top; + s->r.top =0 ; + } + +if (s->r.width > inst->width) + s->r.width = inst->width; +else + inst->width = s->r.width; + +if (s->r.height > inst->height) + s->r.height = inst->height; +else + inst->height = s->r.height; +
Quoted 1 line
From semantic point of view, it looks fine, but where is the rectangle
actually set to the hardware?
Best regards,
Tomasz
Quoted 59 lines · Tomasz Figa
Hi Malathi,
Quoted 42 lines · Malathi Gottam
Handles target type crop by setting the new active rectangle to hardware. The new rectangle should be within YUV size.
Signed-off-by Malathi Gottam <[email protected]>--- drivers/media/platform/qcom/venus/venc.c | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-)drivers/media/platform/qcom/venus/venc.c +12
-4@@ -478,16 +478,34 @@ static int venc_g_fmt(struct file *file, void *fh, struct v4l2_format *f)venc_s_selection (struct file *file,void *fh,struct v4l2_selection *s) {struct venus_inst *inst =to_inst (file); +int ret; +u32 buftype;if (s->type !=V4L2_BUF_TYPE_VIDEO_OUTPUT )return -EINVAL ;switch (s->target) {case V4L2_SEL_TGT_CROP : -if (s->r.width != inst->out_width || - s->r.height != inst->out_height || - s->r.top !=0 || s->r.left !=0 ) -return -EINVAL ; +if (s->r.left !=0 ) { + s->r.width += s->r.left; + s->r.left =0 ; + } + +if (s->r.top !=0 ) { + s->r.height += s->r.top; + s->r.top =0 ; + } +drivers/media/platform/qcom/venus/venc.c static int venc_g_fmt(struct file *file, void · line 510
Quoted 10 lines
drivers/media/platform/qcom/venus/venc.c +10
-0+if (s->r.width > inst->width) + s->r.width = inst->width; +else + inst->width = s->r.width; + +if (s->r.height > inst->height) + s->r.height = inst->height; +else + inst->height = s->r.height; +From semantic point of view, it looks fine, but where is the rectangle actually set to the hardware?
Quoted 2 lines
Best regards,
Tomasz
As this set selection call occurs before the hfi session initialization, for now we are holding these values in driver.
As this call is followed by VIDIOC_REQBUFS(), as a part of this we have venc_init_session
static int venc_init_session(struct venus_inst *inst)
{
int ret;
ret = hfi_session_init(inst, inst->fmt_cap->pixfmt);
if (ret)
return ret;
ret = venus_helper_set_input_resolution(inst, inst->width,
inst->height);
if (ret)
goto deinit;
ret = venus_helper_set_output_resolution(inst, inst->width,
inst->height,
HFI_BUFFER_OUTPUT);
if (ret)
goto deinit;
ret = venus_helper_set_color_format(inst, inst->fmt_out->pixfmt);
if (ret)
goto deinit;
ret = venc_set_properties(inst);From here we set these values to hardware.
Hi Malathi,
Quoted 73 lines · On Fri, Nov 9, 2018 at 6:20 PM <[email protected]> wrote:
Quoted 61 lines · Tomasz Figa
Hi Malathi,
Quoted 43 lines · Malathi Gottam
Handles target type crop by setting the new active rectangle to hardware. The new rectangle should be within YUV size.
Signed-off-by Malathi Gottam <[email protected]>--- drivers/media/platform/qcom/venus/venc.c | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-)drivers/media/platform/qcom/venus/venc.c +12
-4@@ -478,16 +478,34 @@ static int venc_g_fmt(struct file *file, void *fh, struct v4l2_format *f)venc_s_selection (struct file *file,void *fh,struct v4l2_selection *s) {struct venus_inst *inst =to_inst (file); +int ret; +u32 buftype;if (s->type !=V4L2_BUF_TYPE_VIDEO_OUTPUT )return -EINVAL ;switch (s->target) {case V4L2_SEL_TGT_CROP : -if (s->r.width != inst->out_width || - s->r.height != inst->out_height || - s->r.top !=0 || s->r.left !=0 ) -return -EINVAL ; +if (s->r.left !=0 ) { + s->r.width += s->r.left; + s->r.left =0 ; + } + +if (s->r.top !=0 ) { + s->r.height += s->r.top; + s->r.top =0 ; + } +drivers/media/platform/qcom/venus/venc.c static int venc_g_fmt(struct file *file, void · line 510
Quoted 10 lines
drivers/media/platform/qcom/venus/venc.c +10
-0+if (s->r.width > inst->width) + s->r.width = inst->width; +else + inst->width = s->r.width; + +if (s->r.height > inst->height) + s->r.height = inst->height; +else + inst->height = s->r.height; +From semantic point of view, it looks fine, but where is the rectangle actually set to the hardware?
Quoted 2 lines
Best regards,
TomaszAs this set selection call occurs before the hfi session initialization, for now we are holding these values in driver.
As this call is followed by
VIDIOC_REQBUFS(), as a part of this we havevenc_init_sessionstatic int
venc_init_session(struct venus_inst *inst)
Quoted 15 lines
{
int ret;
ret = hfi_session_init(inst, inst->fmt_cap->pixfmt);
if (ret)
return ret;
ret = venus_helper_set_input_resolution(inst, inst->width,
inst->height);
if (ret)
goto deinit;
ret = venus_helper_set_output_resolution(inst, inst->width,
inst->height,
HFI_BUFFER_OUTPUT);Something sounds not right here. Shouldn't one of the width/height be the OUPUT format and the other the selection CROP target rectangle?
Quoted 11 lines
if (ret) goto deinit; ret = venus_helper_set_color_format(inst, inst->fmt_out->pixfmt); if (ret) goto deinit; ret = venc_set_properties(inst);From here we set these values to hardware.
Okay, thanks for the explanation. In this case, we must return -EBUSY if selection is attempted to be set after the session is initialized.
Best regards,
Tomasz
Quoted 20 lines · Tomasz Figa
Hi Malathi,
Quoted 16 lines · On Fri, Nov 9, 2018 at 6:20 PM <[email protected]> wrote:
Quoted 14 lines · Tomasz Figa
Hi Malathi,
Quoted 10 lines · Malathi Gottam
Handles target type crop by setting the new active rectangle to hardware. The new rectangle should be within YUV size.
Signed-off-by Malathi Gottam <[email protected]>--- drivers/media/platform/qcom/venus/venc.c | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-)
Quoted 13 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
drivers/media/platform/qcom/venus/venc.c +0
-0Quoted 8 lines
Quoted 8 lines
drivers/media/platform/qcom/venus/venc.c static int venc_g_fmt(struct file *file, void · line 484
Quoted 8 lines
drivers/media/platform/qcom/venus/venc.c +2
-0@@ -478,16 +478,34 @@ static int venc_g_fmt(struct file *file, void *fh, struct v4l2_format *f)venc_s_selection (struct file *file,void *fh,struct v4l2_selection *s) {struct venus_inst *inst =to_inst (file); +int ret; +u32 buftype;
Looks like these two variables are not used anymore?
Quoted 56 lines
Quoted 56 lines
Quoted 34 lines
Quoted 20 lines
drivers/media/platform/qcom/venus/venc.c +10
-4if (s->type !=V4L2_BUF_TYPE_VIDEO_OUTPUT )return -EINVAL ;switch (s->target) {case V4L2_SEL_TGT_CROP : -if (s->r.width != inst->out_width || - s->r.height != inst->out_height || - s->r.top !=0 || s->r.left !=0 ) -return -EINVAL ; +if (s->r.left !=0 ) { + s->r.width += s->r.left; + s->r.left =0 ; + } + +if (s->r.top !=0 ) { + s->r.height += s->r.top; + s->r.top =0 ; + } +drivers/media/platform/qcom/venus/venc.c static int venc_g_fmt(struct file *file, void *fh, struct v4l2_format *f) · line 508
Quoted 10 lines
drivers/media/platform/qcom/venus/venc.c +10
-0+if (s->r.width > inst->width) + s->r.width = inst->width; +else + inst->width = s->r.width; + +if (s->r.height > inst->height) + s->r.height = inst->height; +else + inst->height = s->r.height; +From semantic point of view, it looks fine, but where is the rectangle actually set to the hardware?
Quoted 2 lines
Best regards,
TomaszAs this set selection call occurs before the hfi session initialization, for now we are holding these values in driver.
As this call is followed by
VIDIOC_REQBUFS(), as a part of this we havevenc_init_sessionstatic int
venc_init_session(struct venus_inst *inst){ int ret; ret = hfi_session_init(inst, inst->fmt_cap->pixfmt); if (ret) return ret; ret = venus_helper_set_input_resolution(inst, inst->width, inst->height); if (ret) goto deinit;
Quoted 6 lines
Quoted 3 lines
ret = venus_helper_set_output_resolution(inst, inst->width, inst->height, HFI_BUFFER_OUTPUT);Something sounds not right here. Shouldn't one of the width/height be the OUPUT format and the other the selection CROP target rectangle?
Yeah, I don't see where the stride of the input (from HFI perspective) buffer is set, so unless I missed something that would not capture the correct part of the buffer.
Also doesn't Venus support non-zero left and top parameters for the selection rectangle?