Hi, On 11/03/2016 12:55 PM, Hans Verkuil wrote: > On 03/11/16 11:45, Stanimir Varbanov wrote: >> Hi Hans, >> >> On 09/19/2016 01:04 PM, Hans Verkuil wrote: >>> On 09/07/2016 01:37 PM, Stanimir Varbanov wrote: >>>> This consists of video decoder implementation plus decoder >>>> controls. >>>> >>>> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@xxxxxxxxxx> >>>> --- >>>> drivers/media/platform/qcom/vidc/vdec.c | 1091 >>>> +++++++++++++++++++++++++ >>>> drivers/media/platform/qcom/vidc/vdec.h | 29 + >>>> drivers/media/platform/qcom/vidc/vdec_ctrls.c | 200 +++++ >>>> drivers/media/platform/qcom/vidc/vdec_ctrls.h | 21 + >>>> 4 files changed, 1341 insertions(+) >>>> create mode 100644 drivers/media/platform/qcom/vidc/vdec.c >>>> create mode 100644 drivers/media/platform/qcom/vidc/vdec.h >>>> create mode 100644 drivers/media/platform/qcom/vidc/vdec_ctrls.c >>>> create mode 100644 drivers/media/platform/qcom/vidc/vdec_ctrls.h >>>> >> >> <cut> >> >>>> + >>>> +static int >>>> +vdec_g_selection(struct file *file, void *priv, struct >>>> v4l2_selection *s) >>>> +{ >>>> + struct vidc_inst *inst = to_inst(file); >>>> + >>>> + if (s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) >>>> + return -EINVAL; >>>> + >>>> + switch (s->target) { >>>> + case V4L2_SEL_TGT_CROP_DEFAULT: >>>> + case V4L2_SEL_TGT_CROP_BOUNDS: >>>> + case V4L2_SEL_TGT_CROP: >>>> + case V4L2_SEL_TGT_COMPOSE_DEFAULT: >>>> + case V4L2_SEL_TGT_COMPOSE_BOUNDS: >>>> + case V4L2_SEL_TGT_COMPOSE: >>> >>> This is almost certainly wrong. >>> >>> For capture I would expect that you can do compose, but not crop. >>> >>> This would likely explain that v4l2-compliance thinks that the driver >>> can >>> scale: >>> >>> test Scaling: OK >>> >> >> Maybe I need some help to implement correctly g_selection. >> >> Lets say that the resolution of the compressed stream is 1280x720, and >> that resolution is set with s_fmt(OUTPUT queue), then I calculate the >> output resolution which I will return by g_fmt(CAPTURE queue) and it >> will be 1280x736 (hardware wants height to be multiple of 32 lines). So >> the result will be 16 lines of vertical padding which should be exposed >> to client (think of gstreamer v4l2videodec element) via g_crop >> (g_selection) as 1280x720 because this is the actual image. >> >> So from what I understood while read Selection API, I need to support >> only composing on CAPTURE queue, no scaling and no cropping. >> >> OUTPUT buffer type, data source >> TGT_CROP_BOUNDS = TGT_CROP_DEFAULT = TGT_CROP = 1280x720 >> TGT_COMPOSE_BOUNDS = TGT_COMPOSE_DEFAULT = TGT_COMPOSE = 1280x720 > > The output buffer type doesn't do composition, only crop. So you shouldn't > support the compose targets. OK I tried to return EINVAL for compose targets for output buffer type and the result is : test Cropping: OK test Composing: OK test Scaling: OK Which is odd because now Scaling is supported. > >> >> CAPTURE buffer type, data sink >> TGT_CROP_BOUNDS = TGT_CROP_DEFAULT = TGT_CROP = EINVAL >> TGT_COMPOSE_BOUNDS = 1280x736 >> TGT_COMPOSE_DEFAULT = TGT_COMPOSE = 1280x720 > > This looks good. > >> >> With this logic in g_selection the output of v4l2-compliance test >> application is: >> test Cropping: OK >> test Composing: OK >> test Scaling: OK (Not Supported) >> >> So why v4l2-compliance still thinks that the driver supports Cropping? > > Hmm, v4l2-compliance could be improved. The problem is that it doesn't > show for m2m devices for which side (capture or output) cropping, composing > or scaling is supported. It does test both sides, but you can't tell from > the output. > > It's a bit confusing in this case. OK I will incorporate the above g_selection behavior and send a new version of the patches, I do not waste time on that. The drawback is that this padding will be displayed as green bar on bottom of the displayed image, although the gstreamer master branch now use g_selection so probably it should be fine there. > > Regards, > > Hans > >> >>>> + break; >>>> + default: >>>> + return -EINVAL; >>>> + } >>>> + >>>> + s->r.top = 0; >>>> + s->r.left = 0; >>>> + s->r.width = inst->out_width; >>>> + s->r.height = inst->out_height; >>>> + >>>> + return 0; >>>> +} >> >> <cut> >> -- regards, Stan -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html