| Message ID | 20171129132522.9140-2-stanimir.varbanov@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 11/29/17 14:25, Stanimir Varbanov wrote: > This change fixes the calculation of B-frames and GOP size by > adopt v4l2 controls with the firmware interface expectations. > > Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org> > --- > drivers/media/platform/qcom/venus/venc.c | 15 ++++--- > drivers/media/platform/qcom/venus/venc_ctrls.c | 60 +++++++++++++++++++++++++- > 2 files changed, 69 insertions(+), 6 deletions(-) > > diff --git a/drivers/media/platform/qcom/venus/venc.c b/drivers/media/platform/qcom/venus/venc.c > index 36d31540c59d..e3a10a852cad 100644 > --- a/drivers/media/platform/qcom/venus/venc.c > +++ b/drivers/media/platform/qcom/venus/venc.c > @@ -685,8 +685,13 @@ static int venc_set_properties(struct venus_inst *inst) > return ret; > } > > + /* IDR periodicity, n: > + * n = 0 - only the first I-frame is IDR frame > + * n = 1 - all I-frames will be IDR frames > + * n > 1 - every n-th I-frame will be IDR frame > + */ > ptype = HFI_PROPERTY_CONFIG_VENC_IDR_PERIOD; > - idrp.idr_period = ctr->gop_size; > + idrp.idr_period = 0; > ret = hfi_session_set_property(inst, ptype, &idrp); > if (ret) > return ret; > @@ -700,10 +705,6 @@ static int venc_set_properties(struct venus_inst *inst) > return ret; > } > > - /* intra_period = pframes + bframes + 1 */ > - if (!ctr->num_p_frames) > - ctr->num_p_frames = 2 * 15 - 1, > - > ptype = HFI_PROPERTY_CONFIG_VENC_INTRA_PERIOD; > intra_period.pframes = ctr->num_p_frames; > intra_period.bframes = ctr->num_b_frames; > @@ -800,6 +801,10 @@ static int venc_init_session(struct venus_inst *inst) > if (ret) > goto deinit; > > + ret = venc_set_properties(inst); > + if (ret) > + goto deinit; > + > return 0; > deinit: > hfi_session_deinit(inst); > diff --git a/drivers/media/platform/qcom/venus/venc_ctrls.c b/drivers/media/platform/qcom/venus/venc_ctrls.c > index ab0fe51ff0f7..53130d4774d3 100644 > --- a/drivers/media/platform/qcom/venus/venc_ctrls.c > +++ b/drivers/media/platform/qcom/venus/venc_ctrls.c > @@ -30,10 +30,58 @@ > #define AT_SLICE_BOUNDARY \ > V4L2_MPEG_VIDEO_H264_LOOP_FILTER_MODE_DISABLED_AT_SLICE_BOUNDARY > > +static int venc_calc_bpframes(u32 gop_size, u32 conseq_b, u32 *bf, u32 *pf) > +{ > + uint32_t half = (gop_size - 1) >> 1; > + uint32_t b, p, ratio; > + bool found = false; > + > + if (!gop_size) > + return -EINVAL; > + > + *bf = *pf = 0; > + > + if (!conseq_b) { > + *pf = gop_size - 1; > + return 0; > + } > + > + b = p = half; > + > + for (; b <= gop_size - 1; b++, p--) { > + if (b % p) > + continue; > + > + ratio = b; > + do_div(ratio, p); Huh? Why use do_div when both arguments are uint32_t? This gives a pile of compile warnings: In file included from /home/hans/work/build/media-git/arch/arm/include/asm/div64.h:127:0, from /home/hans/work/build/media-git/include/linux/kernel.h:173, from /home/hans/work/build/media-git/include/linux/list.h:9, from /home/hans/work/build/media-git/include/media/v4l2-ctrls.h:20, from /home/hans/work/build/media-git/drivers/media/platform/qcom/venus/venc_ctrls.c:16: /home/hans/work/build/media-git/drivers/media/platform/qcom/venus/venc_ctrls.c: In function 'venc_calc_bpframes': /home/hans/work/build/media-git/include/asm-generic/div64.h:222:28: warning: comparison of distinct pointer types lacks a cast (void)(((typeof((n)) *)0) == ((uint64_t *)0)); \ ^ /home/hans/work/build/media-git/drivers/media/platform/qcom/venus/venc_ctrls.c:56:3: note: in expansion of macro 'do_div' do_div(ratio, p); ^~~~~~ In file included from /home/hans/work/build/media-git/include/linux/kernel.h:10:0, from /home/hans/work/build/media-git/include/linux/list.h:9, from /home/hans/work/build/media-git/include/media/v4l2-ctrls.h:20, from /home/hans/work/build/media-git/drivers/media/platform/qcom/venus/venc_ctrls.c:16: /home/hans/work/build/media-git/include/asm-generic/div64.h:235:25: warning: right shift count >= width of type [-Wshift-count-overflow] } else if (likely(((n) >> 32) == 0)) { \ ^ /home/hans/work/build/media-git/include/linux/compiler.h:76:40: note: in definition of macro 'likely' # define likely(x) __builtin_expect(!!(x), 1) ^ /home/hans/work/build/media-git/drivers/media/platform/qcom/venus/venc_ctrls.c:56:3: note: in expansion of macro 'do_div' do_div(ratio, p); ^~~~~~ In file included from /home/hans/work/build/media-git/arch/arm/include/asm/div64.h:127:0, from /home/hans/work/build/media-git/include/linux/kernel.h:173, from /home/hans/work/build/media-git/include/linux/list.h:9, from /home/hans/work/build/media-git/include/media/v4l2-ctrls.h:20, from /home/hans/work/build/media-git/drivers/media/platform/qcom/venus/venc_ctrls.c:16: /home/hans/work/build/media-git/include/asm-generic/div64.h:239:22: error: passing argument 1 of '__div64_32' from incompatible pointer type [-Werror=incompatible-pointer-types] __rem = __div64_32(&(n), __base); \ ^ /home/hans/work/build/media-git/drivers/media/platform/qcom/venus/venc_ctrls.c:56:3: note: in expansion of macro 'do_div' do_div(ratio, p); ^~~~~~ In file included from /home/hans/work/build/media-git/include/linux/kernel.h:173:0, from /home/hans/work/build/media-git/include/linux/list.h:9, from /home/hans/work/build/media-git/include/media/v4l2-ctrls.h:20, from /home/hans/work/build/media-git/drivers/media/platform/qcom/venus/venc_ctrls.c:16: /home/hans/work/build/media-git/arch/arm/include/asm/div64.h:33:24: note: expected 'uint64_t * {aka long long unsigned int *}' but argument is of type 'uint32_t * {aka unsigned int *}' static inline uint32_t __div64_32(uint64_t *n, uint32_t base) ^~~~~~~~~~ Please post a v2. Regards, Hans > + > + if (ratio == conseq_b) { > + found = true; > + break; > + } > + > + if (ratio > conseq_b) > + break; > + } > + > + if (found == false) > + return -EINVAL; > + > + if (b + p + 1 != gop_size) > + return -EINVAL; > + > + *bf = b; > + *pf = p; > + > + return 0; > +} > + > static int venc_op_s_ctrl(struct v4l2_ctrl *ctrl) > { > struct venus_inst *inst = ctrl_to_inst(ctrl); > struct venc_controls *ctr = &inst->controls.enc; > + u32 bframes; > + int ret; > > switch (ctrl->id) { > case V4L2_CID_MPEG_VIDEO_BITRATE_MODE: > @@ -102,6 +150,11 @@ static int venc_op_s_ctrl(struct v4l2_ctrl *ctrl) > case V4L2_CID_MPEG_VIDEO_CYCLIC_INTRA_REFRESH_MB: > break; > case V4L2_CID_MPEG_VIDEO_GOP_SIZE: > + ret = venc_calc_bpframes(ctrl->val, ctr->num_b_frames, &bframes, > + &ctr->num_p_frames); > + if (ret) > + return ret; > + > ctr->gop_size = ctrl->val; > break; > case V4L2_CID_MPEG_VIDEO_H264_I_PERIOD: > @@ -114,7 +167,12 @@ static int venc_op_s_ctrl(struct v4l2_ctrl *ctrl) > ctr->vp8_max_qp = ctrl->val; > break; > case V4L2_CID_MPEG_VIDEO_B_FRAMES: > - ctr->num_b_frames = ctrl->val; > + ret = venc_calc_bpframes(ctr->gop_size, ctrl->val, &bframes, > + &ctr->num_p_frames); > + if (ret) > + return ret; > + > + ctr->num_b_frames = bframes; > break; > default: > return -EINVAL; >
diff --git a/drivers/media/platform/qcom/venus/venc.c b/drivers/media/platform/qcom/venus/venc.c index 36d31540c59d..e3a10a852cad 100644 --- a/drivers/media/platform/qcom/venus/venc.c +++ b/drivers/media/platform/qcom/venus/venc.c @@ -685,8 +685,13 @@ static int venc_set_properties(struct venus_inst *inst) return ret; } + /* IDR periodicity, n: + * n = 0 - only the first I-frame is IDR frame + * n = 1 - all I-frames will be IDR frames + * n > 1 - every n-th I-frame will be IDR frame + */ ptype = HFI_PROPERTY_CONFIG_VENC_IDR_PERIOD; - idrp.idr_period = ctr->gop_size; + idrp.idr_period = 0; ret = hfi_session_set_property(inst, ptype, &idrp); if (ret) return ret; @@ -700,10 +705,6 @@ static int venc_set_properties(struct venus_inst *inst) return ret; } - /* intra_period = pframes + bframes + 1 */ - if (!ctr->num_p_frames) - ctr->num_p_frames = 2 * 15 - 1, - ptype = HFI_PROPERTY_CONFIG_VENC_INTRA_PERIOD; intra_period.pframes = ctr->num_p_frames; intra_period.bframes = ctr->num_b_frames; @@ -800,6 +801,10 @@ static int venc_init_session(struct venus_inst *inst) if (ret) goto deinit; + ret = venc_set_properties(inst); + if (ret) + goto deinit; + return 0; deinit: hfi_session_deinit(inst); diff --git a/drivers/media/platform/qcom/venus/venc_ctrls.c b/drivers/media/platform/qcom/venus/venc_ctrls.c index ab0fe51ff0f7..53130d4774d3 100644 --- a/drivers/media/platform/qcom/venus/venc_ctrls.c +++ b/drivers/media/platform/qcom/venus/venc_ctrls.c @@ -30,10 +30,58 @@ #define AT_SLICE_BOUNDARY \ V4L2_MPEG_VIDEO_H264_LOOP_FILTER_MODE_DISABLED_AT_SLICE_BOUNDARY +static int venc_calc_bpframes(u32 gop_size, u32 conseq_b, u32 *bf, u32 *pf) +{ + uint32_t half = (gop_size - 1) >> 1; + uint32_t b, p, ratio; + bool found = false; + + if (!gop_size) + return -EINVAL; + + *bf = *pf = 0; + + if (!conseq_b) { + *pf = gop_size - 1; + return 0; + } + + b = p = half; + + for (; b <= gop_size - 1; b++, p--) { + if (b % p) + continue; + + ratio = b; + do_div(ratio, p); + + if (ratio == conseq_b) { + found = true; + break; + } + + if (ratio > conseq_b) + break; + } + + if (found == false) + return -EINVAL; + + if (b + p + 1 != gop_size) + return -EINVAL; + + *bf = b; + *pf = p; + + return 0; +} + static int venc_op_s_ctrl(struct v4l2_ctrl *ctrl) { struct venus_inst *inst = ctrl_to_inst(ctrl); struct venc_controls *ctr = &inst->controls.enc; + u32 bframes; + int ret; switch (ctrl->id) { case V4L2_CID_MPEG_VIDEO_BITRATE_MODE: @@ -102,6 +150,11 @@ static int venc_op_s_ctrl(struct v4l2_ctrl *ctrl) case V4L2_CID_MPEG_VIDEO_CYCLIC_INTRA_REFRESH_MB: break; case V4L2_CID_MPEG_VIDEO_GOP_SIZE: + ret = venc_calc_bpframes(ctrl->val, ctr->num_b_frames, &bframes, + &ctr->num_p_frames); + if (ret) + return ret; + ctr->gop_size = ctrl->val; break; case V4L2_CID_MPEG_VIDEO_H264_I_PERIOD: @@ -114,7 +167,12 @@ static int venc_op_s_ctrl(struct v4l2_ctrl *ctrl) ctr->vp8_max_qp = ctrl->val; break; case V4L2_CID_MPEG_VIDEO_B_FRAMES: - ctr->num_b_frames = ctrl->val; + ret = venc_calc_bpframes(ctr->gop_size, ctrl->val, &bframes, + &ctr->num_p_frames); + if (ret) + return ret; + + ctr->num_b_frames = bframes; break; default: return -EINVAL;
This change fixes the calculation of B-frames and GOP size by adopt v4l2 controls with the firmware interface expectations. Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org> --- drivers/media/platform/qcom/venus/venc.c | 15 ++++--- drivers/media/platform/qcom/venus/venc_ctrls.c | 60 +++++++++++++++++++++++++- 2 files changed, 69 insertions(+), 6 deletions(-)