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@xxxxxxxxxx> 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@xxxxxxxxxx>
>> ---
>> 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
