Re: [PATCH 2/3] [media] coda: fix coda_g_selection
Am Montag, den 28.07.2014, 00:22 +0200 schrieb Hans Verkuil: > On 07/27/2014 11:32 PM, Nicolas Dufresne wrote: > > Le dimanche 27 juillet 2014 à 20:21 +0200, Hans Verkuil a écrit : > >> If cropcap returns -EINVAL then that means that the current input or > >> output does > >> not support cropping (for input) or composing (for output). In that case > >> the > >> pixel aspect ratio is undefined and you have no way to get hold of that > >> information, > >> which is a bug in the V4L2 API. > >> > >> In the case of an m2m device you can safely assume that whatever the pixel > >> aspect > >> is of the image you give to the m2m device, it will still be the same pixel > >> aspect when you get it back. In fact, I would say that if an m2m device > >> returns > >> cropcap information, then the pixel aspect ratio information is most > >> likely not > >> applicable to the device and will typically be 1:1. > >> > >> Pixel aspect ratio is only relevant if the video comes in or goes out to a > >> physical > >> interface (sensor, video receiver/transmitter). > > > > So far "not applicable" has been interpreted as not implemented / > > ENOTTY. Can't CODA just do that and we can close this subject ? > > Yes, that might be the best solution. Just call v4l2_disable_ioctl for > CROPCAP and > G/S_CROP, since none of them apply to the coda driver. Sounds like a plan, I'll do that. thanks Philipp -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] [media] coda: fix coda_g_selection
On 07/27/2014 11:32 PM, Nicolas Dufresne wrote: > Le dimanche 27 juillet 2014 à 20:21 +0200, Hans Verkuil a écrit : >> If cropcap returns -EINVAL then that means that the current input or >> output does >> not support cropping (for input) or composing (for output). In that case the >> pixel aspect ratio is undefined and you have no way to get hold of that >> information, >> which is a bug in the V4L2 API. >> >> In the case of an m2m device you can safely assume that whatever the pixel >> aspect >> is of the image you give to the m2m device, it will still be the same pixel >> aspect when you get it back. In fact, I would say that if an m2m device >> returns >> cropcap information, then the pixel aspect ratio information is most likely >> not >> applicable to the device and will typically be 1:1. >> >> Pixel aspect ratio is only relevant if the video comes in or goes out to a >> physical >> interface (sensor, video receiver/transmitter). > > So far "not applicable" has been interpreted as not implemented / > ENOTTY. Can't CODA just do that and we can close this subject ? Yes, that might be the best solution. Just call v4l2_disable_ioctl for CROPCAP and G/S_CROP, since none of them apply to the coda driver. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] [media] coda: fix coda_g_selection
Le dimanche 27 juillet 2014 à 20:21 +0200, Hans Verkuil a écrit : > If cropcap returns -EINVAL then that means that the current input or > output does > not support cropping (for input) or composing (for output). In that case the > pixel aspect ratio is undefined and you have no way to get hold of that > information, > which is a bug in the V4L2 API. > > In the case of an m2m device you can safely assume that whatever the pixel > aspect > is of the image you give to the m2m device, it will still be the same pixel > aspect when you get it back. In fact, I would say that if an m2m device > returns > cropcap information, then the pixel aspect ratio information is most likely > not > applicable to the device and will typically be 1:1. > > Pixel aspect ratio is only relevant if the video comes in or goes out to a > physical > interface (sensor, video receiver/transmitter). So far "not applicable" has been interpreted as not implemented / ENOTTY. Can't CODA just do that and we can close this subject ? Nicolas -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] [media] coda: fix coda_g_selection
On 07/27/2014 06:53 PM, Nicolas Dufresne wrote: > > Le Samedi 26 Juillet 2014 12:37 EDT, Philipp Zabel > a écrit: > >> I have tried the GStreamer v4l2videodec element with the coda driver and >> noticed that GStreamer calls VIDIOC_CROPCAP to obtain the pixel aspect >> ratio. This always fails with -EINVAL because of this issue. Currently >> GStreamer >> throws a warning if the return value is an error other than -ENOTTY. > > But for now, this seems like a fair thing to do. We currently assume that if > your > driver is not implementing CROPCAP, then pixel aspect ratio at output will be > unchanged at capture. If there is an error though, it's not good sign, and we > report > it. If that is wrong, let us know why and how to detect your driver error > isn't a an error. If cropcap returns -EINVAL then that means that the current input or output does not support cropping (for input) or composing (for output). In that case the pixel aspect ratio is undefined and you have no way to get hold of that information, which is a bug in the V4L2 API. In the case of an m2m device you can safely assume that whatever the pixel aspect is of the image you give to the m2m device, it will still be the same pixel aspect when you get it back. In fact, I would say that if an m2m device returns cropcap information, then the pixel aspect ratio information is most likely not applicable to the device and will typically be 1:1. Pixel aspect ratio is only relevant if the video comes in or goes out to a physical interface (sensor, video receiver/transmitter). Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] [media] coda: fix coda_g_selection
Le Samedi 26 Juillet 2014 12:37 EDT, Philipp Zabel a écrit: > I have tried the GStreamer v4l2videodec element with the coda driver and > noticed that GStreamer calls VIDIOC_CROPCAP to obtain the pixel aspect > ratio. This always fails with -EINVAL because of this issue. Currently > GStreamer > throws a warning if the return value is an error other than -ENOTTY. But for now, this seems like a fair thing to do. We currently assume that if your driver is not implementing CROPCAP, then pixel aspect ratio at output will be unchanged at capture. If there is an error though, it's not good sign, and we report it. If that is wrong, let us know why and how to detect your driver error isn't a an error. cheers, Nicolas -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] [media] coda: fix coda_g_selection
On 07/26/2014 06:37 PM, Philipp Zabel wrote: > On Sat, Jul 26, 2014 at 5:12 PM, Hans Verkuil wrote: >> On 07/26/2014 04:34 PM, Philipp Zabel wrote: >>> Crop targets are valid on the capture side and compose targets are valid >>> on the output side, not the other way around. >> >> Are you sure about this? Usually for m2m devices the capture side supports >> compose (i.e. the result of the m2m operation can be composed into the >> capture >> buffer) and the output side supports crop (i.e. the m2m operates on the >> cropped >> part of the output buffer instead of on the full buffer), like the coda >> driver >> does today. > > You are right, I haven't thought this through. Please ignore this patch. > >> As a result of that the old G/S_CROP API cannot be used with most m2m devices >> since it does the opposite operation, which does not apply to m2m devices. > > I have tried the GStreamer v4l2videodec element with the coda driver and > noticed that GStreamer calls VIDIOC_CROPCAP to obtain the pixel aspect > ratio. This always fails with -EINVAL because of this issue. Currently > GStreamer > throws a warning if the return value is an error other than -ENOTTY. I never ever liked it that pixelaspect was part of cropcap since it is really unrelated to cropping. Now that the compound control support is in it might be time to create a pair of read-only controls that report the pixelaspect ratio, one for video capture, one for video output. That would be a much cleaner solution. Regards, Hans > > regards > Philipp > -- > To unsubscribe from this list: send the line "unsubscribe linux-media" in > the body of a message to majord...@vger.kernel.org > 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] [media] coda: fix coda_g_selection
On Sat, Jul 26, 2014 at 5:12 PM, Hans Verkuil wrote: > On 07/26/2014 04:34 PM, Philipp Zabel wrote: >> Crop targets are valid on the capture side and compose targets are valid >> on the output side, not the other way around. > > Are you sure about this? Usually for m2m devices the capture side supports > compose (i.e. the result of the m2m operation can be composed into the capture > buffer) and the output side supports crop (i.e. the m2m operates on the > cropped > part of the output buffer instead of on the full buffer), like the coda driver > does today. You are right, I haven't thought this through. Please ignore this patch. > As a result of that the old G/S_CROP API cannot be used with most m2m devices > since it does the opposite operation, which does not apply to m2m devices. I have tried the GStreamer v4l2videodec element with the coda driver and noticed that GStreamer calls VIDIOC_CROPCAP to obtain the pixel aspect ratio. This always fails with -EINVAL because of this issue. Currently GStreamer throws a warning if the return value is an error other than -ENOTTY. regards Philipp -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] [media] coda: fix coda_g_selection
On 07/26/2014 04:34 PM, Philipp Zabel wrote: > Crop targets are valid on the capture side and compose targets are valid > on the output side, not the other way around. Are you sure about this? Usually for m2m devices the capture side supports compose (i.e. the result of the m2m operation can be composed into the capture buffer) and the output side supports crop (i.e. the m2m operates on the cropped part of the output buffer instead of on the full buffer), like the coda driver does today. As a result of that the old G/S_CROP API cannot be used with most m2m devices since it does the opposite operation, which does not apply to m2m devices. Regards, Hans > > Signed-off-by: Philipp Zabel > --- > drivers/media/platform/coda/coda-common.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/platform/coda/coda-common.c > b/drivers/media/platform/coda/coda-common.c > index 95d0b04..b542340 100644 > --- a/drivers/media/platform/coda/coda-common.c > +++ b/drivers/media/platform/coda/coda-common.c > @@ -600,7 +600,7 @@ static int coda_g_selection(struct file *file, void *fh, > rsel = &r; > /* fallthrough */ > case V4L2_SEL_TGT_CROP: > - if (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT) > + if (s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE) > return -EINVAL; > break; > case V4L2_SEL_TGT_COMPOSE_BOUNDS: > @@ -609,7 +609,7 @@ static int coda_g_selection(struct file *file, void *fh, > /* fallthrough */ > case V4L2_SEL_TGT_COMPOSE: > case V4L2_SEL_TGT_COMPOSE_DEFAULT: > - if (s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE) > + if (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT) > return -EINVAL; > break; > default: > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
