Hi Alex, Thanks for the comments!
On 1/24/19 10:43 AM, Alexandre Courbot wrote: > On Fri, Jan 18, 2019 at 1:21 AM Stanimir Varbanov > <stanimir.varba...@linaro.org> wrote: >> >> In most of the cases the client will know better what could be >> the maximum size for compressed data buffers. Change the driver >> to permit the user to set bigger size for the compressed buffer >> but make reasonable sanitation. >> >> Signed-off-by: Stanimir Varbanov <stanimir.varba...@linaro.org> >> --- >> drivers/media/platform/qcom/venus/vdec.c | 18 +++++++++++++----- >> 1 file changed, 13 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/media/platform/qcom/venus/vdec.c >> b/drivers/media/platform/qcom/venus/vdec.c >> index 282de21cf2e1..7a9370df7515 100644 >> --- a/drivers/media/platform/qcom/venus/vdec.c >> +++ b/drivers/media/platform/qcom/venus/vdec.c >> @@ -142,6 +142,7 @@ vdec_try_fmt_common(struct venus_inst *inst, struct >> v4l2_format *f) >> struct v4l2_pix_format_mplane *pixmp = &f->fmt.pix_mp; >> struct v4l2_plane_pix_format *pfmt = pixmp->plane_fmt; >> const struct venus_format *fmt; >> + u32 szimage; >> >> memset(pfmt[0].reserved, 0, sizeof(pfmt[0].reserved)); >> memset(pixmp->reserved, 0, sizeof(pixmp->reserved)); >> @@ -170,14 +171,18 @@ vdec_try_fmt_common(struct venus_inst *inst, struct >> v4l2_format *f) >> pixmp->num_planes = fmt->num_planes; >> pixmp->flags = 0; >> >> - pfmt[0].sizeimage = venus_helper_get_framesz(pixmp->pixelformat, >> - pixmp->width, >> - pixmp->height); >> + szimage = venus_helper_get_framesz(pixmp->pixelformat, pixmp->width, >> + pixmp->height); >> >> - if (f->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) >> + if (f->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) { >> + pfmt[0].sizeimage = szimage; >> pfmt[0].bytesperline = ALIGN(pixmp->width, 128); >> - else >> + } else { >> + pfmt[0].sizeimage = clamp_t(u32, pfmt[0].sizeimage, 0, >> SZ_4M); >> + if (szimage > pfmt[0].sizeimage) >> + pfmt[0].sizeimage = szimage; > > pfmt[0].sizeimage = max(clamp_t(u32, pfmt[0].sizeimage, 0, SZ_4M), > szimage)? I'm not a big fan to that calling of macro from macro :) What about this: pfmt[0].sizeimage = clamp_t(u32, pfmt[0].sizeimage, 0, SZ_4M); pfmt[0].sizeimage = max(pfmt[0].sizeimage, szimage); > >> pfmt[0].bytesperline = 0; >> + } >> >> return fmt; >> } >> @@ -275,6 +280,7 @@ static int vdec_s_fmt(struct file *file, void *fh, >> struct v4l2_format *f) >> inst->ycbcr_enc = pixmp->ycbcr_enc; >> inst->quantization = pixmp->quantization; >> inst->xfer_func = pixmp->xfer_func; >> + inst->input_buf_size = pixmp->plane_fmt[0].sizeimage; >> } >> >> memset(&format, 0, sizeof(format)); >> @@ -737,6 +743,8 @@ static int vdec_queue_setup(struct vb2_queue *q, >> sizes[0] = venus_helper_get_framesz(inst->fmt_out->pixfmt, >> inst->out_width, >> inst->out_height); >> + if (inst->input_buf_size > sizes[0]) >> + sizes[0] = inst->input_buf_size; > > sizes[0] = max(venus_helper_get_framesz(inst->fmt_out->pixfmt, > inst->out_width, > inst->out_height), > inst->input_buf_size)? I think it'd be more readable that way: sizes[0] = max(sizes[0], inst->input_buf_size); > > > >> inst->input_buf_size = sizes[0]; >> *num_buffers = max(*num_buffers, in_num); >> inst->num_input_bufs = *num_buffers; >> -- >> 2.17.1 >> -- regards, Stan