| Message ID | 20190117162008.25217-10-stanimir.varbanov@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Venus stateful Codec API | expand |
On Fri, Jan 18, 2019 at 1:21 AM Stanimir Varbanov <stanimir.varbanov@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.varbanov@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)? > 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)? > inst->input_buf_size = sizes[0]; > *num_buffers = max(*num_buffers, in_num); > inst->num_input_bufs = *num_buffers; > -- > 2.17.1 >
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.varbanov@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.varbanov@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 >>
On Thu, Jan 24, 2019 at 7:05 PM Stanimir Varbanov <stanimir.varbanov@linaro.org> wrote: > > 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.varbanov@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.varbanov@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); Looks fine, I just wanted to make sure that we use the more expressive "max" instead of an if statement. > > > > >> 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); Ditto.
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].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; inst->input_buf_size = sizes[0]; *num_buffers = max(*num_buffers, in_num); inst->num_input_bufs = *num_buffers;
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.varbanov@linaro.org> --- drivers/media/platform/qcom/venus/vdec.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-)