On 07/18/2016 02:28 PM, tiffany lin wrote: > Understood now. > > Now I am trying to figure out how to make this function right. > Our encoder only support crop range from (0, 0) to (width, height), so > if s->r.top and s->r.left not 0, I will return -EINVAL. > > > Another thing is that in v4l2-compliance test, it has testLegacyCrop. > It looks like we still need to support > V4L2_SEL_TGT_COMPOSE_DEFAULT: > V4L2_SEL_TGT_COMPOSE_BOUNDS: > V4L2_SEL_TGT_COMPOSE: > to pass v4l2 compliance test, Or it will fail in > fail: v4l2-test-formats.cpp(1318): !doioctl(node, VIDIOC_G_SELECTION, > &sel) > fail: v4l2-test-formats.cpp(1336): testLegacyCrop(node) > test Cropping: FAIL Against which kernel are you testing? In the current media_tree master there is a bug in drivers/media/v4l2-core/v4l2-ioctl.c, v4l_cropcap(): This code: if (WARN_ON(!ops->vidioc_cropcap && !ops->vidioc_cropcap)) should be: if (WARN_ON(!ops->vidioc_cropcap && !ops->vidioc_g_selection)) The fix is waiting for a pull from Linus. Also update to the latest v4l2-compliance: I've made some changes that might affect this. And I added additional checks to verify if all the colorspace-related format fields are properly propagated from the output format to the capture format. Regards, Hans > > I don't understand the following testing code. > > /* > * If either CROPCAP or G_CROP works, then G_SELECTION should > * work as well. > * If neither CROPCAP nor G_CROP work, then G_SELECTION > shouldn't > * work either. > */ > if (!doioctl(node, VIDIOC_CROPCAP, &cap)) { > fail_on_test(doioctl(node, VIDIOC_G_SELECTION, &sel)); > > // Checks for invalid types > if (cap.type == V4L2_BUF_TYPE_VIDEO_CAPTURE) > cap.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE; > else > cap.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE; > fail_on_test(doioctl(node, VIDIOC_CROPCAP, &cap) != > EINVAL); > cap.type = 0xff; > fail_on_test(doioctl(node, VIDIOC_CROPCAP, &cap) != > EINVAL); > } else { > fail_on_test(!doioctl(node, VIDIOC_G_SELECTION, &sel)); > -> fail here > } > > When test OUTPUT queue, it fail because v4l_cropcap will fail when > s.target = V4L2_SEL_TGT_COMPOSE_BOUNDS. > If VIDIOC_CROPCAP ioctl fail, VIDIOC_G_SELECTION should fail. > But VIDIOC_G_SELECTION target on CROP not COMPOSE and it success. > > > best regards, > Tiffany > > > >> Regards, >> >> Hans >> >>> >>> >>> static int v4l_g_crop(const struct v4l2_ioctl_ops *ops, >>> struct file *file, void *fh, void *arg) >>> { >>> struct v4l2_crop *p = arg; >>> struct v4l2_selection s = { >>> .type = p->type, >>> }; >>> int ret; >>> >>> if (ops->vidioc_g_crop) >>> return ops->vidioc_g_crop(file, fh, p); >>> /* simulate capture crop using selection api */ >>> >>> /* crop means compose for output devices */ >>> if (V4L2_TYPE_IS_OUTPUT(p->type)) >>> s.target = V4L2_SEL_TGT_COMPOSE_ACTIVE; >>> else >>> s.target = V4L2_SEL_TGT_CROP_ACTIVE; >>> >>> ret = ops->vidioc_g_selection(file, fh, &s); >>> >>> /* copying results to old structure on success */ >>> if (!ret) >>> p->c = s.r; >>> return ret; >>> } >>> >>> static int v4l_s_crop(const struct v4l2_ioctl_ops *ops, >>> struct file *file, void *fh, void *arg) >>> { >>> struct v4l2_crop *p = arg; >>> struct v4l2_selection s = { >>> .type = p->type, >>> .r = p->c, >>> }; >>> >>> if (ops->vidioc_s_crop) >>> return ops->vidioc_s_crop(file, fh, p); >>> /* simulate capture crop using selection api */ >>> >>> /* crop means compose for output devices */ >>> if (V4L2_TYPE_IS_OUTPUT(p->type)) >>> s.target = V4L2_SEL_TGT_COMPOSE_ACTIVE; >>> else >>> s.target = V4L2_SEL_TGT_CROP_ACTIVE; >>> >>> return ops->vidioc_s_selection(file, fh, &s); >>> } >>> >>> >>> best regards, >>> Tiffany >>> >>> >>> >>> >>>> Regards, >>>> >>>> Hans >>>> >>>>> + >>>>> + return 0; >>>>> +} >>>>> + >>>>> static int vidioc_venc_qbuf(struct file *file, void *priv, >>>>> struct v4l2_buffer *buf) >>>>> { >>>>> @@ -688,6 +759,9 @@ const struct v4l2_ioctl_ops mtk_venc_ioctl_ops = { >>>>> >>>>> .vidioc_create_bufs = v4l2_m2m_ioctl_create_bufs, >>>>> .vidioc_prepare_buf = v4l2_m2m_ioctl_prepare_buf, >>>>> + >>>>> + .vidioc_g_selection = vidioc_venc_g_selection, >>>>> + .vidioc_s_selection = vidioc_venc_s_selection, >>>>> }; >>>>> >>>>> static int vb2ops_venc_queue_setup(struct vb2_queue *vq, >>>>> >>> >>> >>> -- >>> To unsubscribe from this list: send the line "unsubscribe linux-media" in >>> the body of a message to majordomo@xxxxxxxxxxxxxxx >>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>> > > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html