RE: Support on discontinuous planer buffer and stride
Hi,
Forgot to mention that this may need a new memory type as well.
Murali Karicheri
Software Design Engineer
Texas Instruments Inc.
Germantown, MD 20874
phone: 301-407-9583
email: m-kariche...@ti.com
>-Original Message-
>From: linux-media-ow...@vger.kernel.org [mailto:linux-media-
>ow...@vger.kernel.org] On Behalf Of Karicheri, Muralidharan
>Sent: Thursday, October 15, 2009 10:00 AM
>To: Hans Verkuil; Jun Nie
>Cc: g.liakhovet...@gmx.de; linux-media
>Subject: RE: Support on discontinuous planer buffer and stride
>
>Hans,
>
>>
>>Well, it is definitely not possible to do it in this manner since changing
>>the size of struct v4l2_buffer will break the API. Furthermore, something
>>like
>>this will only work if the DMA engine can handle strides. Is that the case
>>for your hardware? I don't think you mentioned what the hardware is you
>use.
>>
>In fact I was planning to write an RFC for this as well. DM365 Resizer
>allows setting separate buffer address for Y and C plane (For _NV12 pixel
>format) as well as line offsets. Similarly on the display side, VPBE
>provides separate registers for configuring this. Currently we have
>proprietary IOCTL to configure the C-Plane buffer address and is not the
>right way to do it. For planar pixel format like NV12, NV16 etc, where the
>hardware is capable of configuring different address for individual plane,
>current v4l2 API has limitations. So I suggest that Jun Nie work on a RFC
>&implementation that allows application to set buffer addresses for
>individual planes of planar pixel formats. Something like below for userptr
>case (I feel only userptr case to be supported in this case)...
>
>+ struct v4l2_userptr_planar {
>+ /* Number of planes in the pixel format. 2 or 3,
>+ * 2 - for Y & CbCr, 3 for Y, Cb, & Cr
>+ */
>+ __u32 num_planes;
>+ /* Y or R */
>+ unsigned long userptr_yr;
>+ /* Cb or G */
>+ unsigned long userptr_cbg;
>+ /* Cr or B */
>+ unsigned long userptr_crb;
>+ };
>
>struct v4l2_buffer {
> __u32 index;
> enum v4l2_buf_type type;
> __u32 bytesused;
> __u32 flags;
> enum v4l2_field field;
> struct timeval timestamp;
> struct v4l2_timecodetimecode;
> __u32 sequence;
>
> /* memory location */
> enum v4l2_memorymemory;
> union {
> __u32 offset;
> unsigned long userptr;
>+ struct v4l2_userptr_planar userptr_p;
> } m;
> __u32 length;
> __u32 input;
> __u32 reserved;
>};
>
>-Murali
>>Regards,
>>
>> Hans
>>
>>--
>>Hans Verkuil - video4linux developer - sponsored by TANDBERG Telecom
>>¢éì¹» ®&Þ~º&¶¬–+-±éݶ¥Šw®žË›±ÊâmébžìfyØšŠ{ayº ʇڙë,j
>¢f£¢·hš‹àz¹ ®w¥¢¸
>>
>>¢·¦j:+v‰¨ŠwèjØm¶Ÿÿ¾
>«‘êçzZ+ƒùšŽŠÝ¢j"�ú!¶i
>--
>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: Support on discontinuous planer buffer and stride
Hans,
>
>Well, it is definitely not possible to do it in this manner since changing
>the size of struct v4l2_buffer will break the API. Furthermore, something
>like
>this will only work if the DMA engine can handle strides. Is that the case
>for your hardware? I don't think you mentioned what the hardware is you use.
>
In fact I was planning to write an RFC for this as well. DM365 Resizer allows
setting separate buffer address for Y and C plane (For _NV12 pixel format) as
well as line offsets. Similarly on the display side, VPBE provides separate
registers for configuring this. Currently we have proprietary IOCTL to
configure the C-Plane buffer address and is not the right way to do it. For
planar pixel format like NV12, NV16 etc, where the hardware is capable of
configuring different address for individual plane, current v4l2 API has
limitations. So I suggest that Jun Nie work on a RFC &implementation that
allows application to set buffer addresses for individual planes of planar
pixel formats. Something like below for userptr case (I feel only userptr case
to be supported in this case)...
+ struct v4l2_userptr_planar {
+ /* Number of planes in the pixel format. 2 or 3,
+* 2 - for Y & CbCr, 3 for Y, Cb, & Cr
+*/
+ __u32 num_planes;
+ /* Y or R */
+ unsigned long userptr_yr;
+ /* Cb or G */
+ unsigned long userptr_cbg;
+ /* Cr or B */
+ unsigned long userptr_crb;
+ };
struct v4l2_buffer {
__u32 index;
enum v4l2_buf_type type;
__u32 bytesused;
__u32 flags;
enum v4l2_field field;
struct timeval timestamp;
struct v4l2_timecodetimecode;
__u32 sequence;
/* memory location */
enum v4l2_memorymemory;
union {
__u32 offset;
unsigned long userptr;
+ struct v4l2_userptr_planar userptr_p;
} m;
__u32 length;
__u32 input;
__u32 reserved;
};
-Murali
>Regards,
>
> Hans
>
>--
>Hans Verkuil - video4linux developer - sponsored by TANDBERG Telecom
>¢éì¹» ®&Þ~º&¶¬–+-±éݶ¥Šw®žË›±ÊâmébžìfyØšŠ{ayº ʇڙë,j
¢f£¢·hš‹àz¹ ®w¥¢¸
>
>¢·¦j:+v‰¨ŠwèjØm¶Ÿÿ¾
«‘êçzZ+ƒùšŽŠÝ¢j"�ú!¶i
--
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: Support on discontinuous planer buffer and stride
2009/10/10 Hans Verkuil :
> On Saturday 10 October 2009 03:34:27 Jun Nie wrote:
>> 2009/10/9 Hans Verkuil :
>> > On Friday 09 October 2009 07:07:32 Jun Nie wrote:
>> >> 2009/9/23 Jun Nie :
>> >> > Hi,
>> >> > I re-send this email for the last one is rejected by system. I am
>> >> > sorry if you guys received both.
>> >> >
>> >> > I am optimizing video playback with overlay with V4L2 driver. The
>> >> > video content is a sub-region of codec output. Thus a memory copy is
>> >> > necessary.
>> >> > Is there plan to support for stride and discrete YUV planer in
>> >> > kernel? Such as below changes can help much for my usage case.
>> >> >
>> >> > --- a/include/linux/videodev2.h
>> >> > +++ b/include/linux/videodev2.h
>> >> > @@ -529,7 +529,20 @@ struct v4l2_buffer {
>> >> > __u32 offset;
>> >> > unsigned long userptr;
>> >> > } m;
>> >> > + /* UV/GB location is valid only in planer case */
>> >> > + union {
>> >> > + __u32 offset_ug;
>> >> > + unsigned long userptr_ug;
>> >> > + } m_ug;
>> >> > + union {
>> >> > + __u32 offset_vb;
>> >> > + unsigned long userptr_vb;
>> >> > + } m_vb;
>> >> > __u32 length;
>> >> > + /* stride of YUV or RGB */
>> >> > + __u32 stride_yr;
>> >> > + __u32 stride_ug;
>> >> > + __u32 stride_vb;
>> >> > __u32 input;
>> >> > __u32 reserved;
>> >> > };
>> >> >
>> >> > If such change is acceptable for everyone, I may help on the
>> >> > implementation.
>> >> > Any comments are welcome.
>> >> >
>> >> > Jun
>> >> >
>> >>
>> >> Hi, Hans/Guennadi
>> >>
>> >> What do you think of supporting this feature?
>> >>
>> >> Jun
>> >>
>> >>
>> >
>> > Well, it is definitely not possible to do it in this manner since changing
>> > the size of struct v4l2_buffer will break the API. Furthermore, something
>> > like
>> > this will only work if the DMA engine can handle strides. Is that the case
>> > for your hardware? I don't think you mentioned what the hardware is you
>> > use.
>> >
>> > Regards,
>> >
>> > Hans
>> >
>> > --
>> > Hans Verkuil - video4linux developer - sponsored by TANDBERG Telecom
>> >
>>
>> Hi, Hans
>>
>> Thanks for your comments. My hardware, PXA168/910 support Y/U/V,
>> three DMA addresses. Y and UV stride can be different with width, such
>> as Y_stride == 336, UV_stride == 168 while width == 320. LCD
>> controller will handle dropping 20 bytes for every line before new
>> line scanning.
>>
>> Application should query driver for the sub-region capability
>> before use it. Driver that do not support this feature will return
>> false by default, application should copy the sub-region to v4l2
>> buffer in this case.
>>
>> The v4l2_buffer size change does impact IOCTL entry index value,
>> application re-compilation is needed, but no code change needed. It is
>> a balance between exploration of hardware capability and existing
>> application binary compatibility. I am new to this community. Was
>> there similar problems in community?
>
> It is not allowed to break application binary compatibility. That's just a
> fact of life.
>
> If you use the PXA hardware, does that mean that you use the pxa_camera
> driver? (Just making sure we talk about the same thing :-) )
>
> Anyway, what you are really trying to do is to crop before sending over the
> picture.
>
> The normal sequence for output devices is that with S_FMT you specify the
> size of the input frame and with S_CROP you can specify the target 'window'
> for that frame. Usually the target window is the same size as the input frame,
> but it can be a different size as well and in that case the hardware will
> have to scale the input frame to the size of the target window.
>
> Note that for output windows the S_CROP command is sort of the inverse
> operation of what S_CROP does for capturing. If we would redo the API I'm sure
> we would implement this differently since it is pretty confusing. But so be
> it.
>
> What you want to do here is to crop the input frame. So we need either some
> new S_PRECROP ioctl or perhaps some new v4l2_buf_type value and use that in
> combination with S_CROP. It should definitely be the responsibility of the
> driver to figure out the strides as that very much depends on the chosen pixel
> format. It's bad API design to put such hardware assumptions in the API.
>
> If we want to keep the symmetry between capture and output streams, then
> S_POSTCROP might be the better name. I.e. for capture streams S_CROP
> determines the source rectangle, S_FMT the output resolution of that rectangle
> and S_POSTCROP the area within that output rectangle that we actually want to
> use. All this reverses for an output stream, so S_POSTCROP would refer to the
Re: Support on discontinuous planer buffer and stride
On Friday 09 October 2009 07:07:32 Jun Nie wrote:
> 2009/9/23 Jun Nie :
> > Hi,
> > I re-send this email for the last one is rejected by system. I am
> > sorry if you guys received both.
> >
> > I am optimizing video playback with overlay with V4L2 driver. The
> > video content is a sub-region of codec output. Thus a memory copy is
> > necessary.
> > Is there plan to support for stride and discrete YUV planer in
> > kernel? Such as below changes can help much for my usage case.
> >
> > --- a/include/linux/videodev2.h
> > +++ b/include/linux/videodev2.h
> > @@ -529,7 +529,20 @@ struct v4l2_buffer {
> > __u32 offset;
> > unsigned long userptr;
> > } m;
> > + /* UV/GB location is valid only in planer case */
> > + union {
> > + __u32 offset_ug;
> > + unsigned long userptr_ug;
> > + } m_ug;
> > + union {
> > + __u32 offset_vb;
> > + unsigned long userptr_vb;
> > + } m_vb;
> > __u32 length;
> > + /* stride of YUV or RGB */
> > + __u32 stride_yr;
> > + __u32 stride_ug;
> > + __u32 stride_vb;
> > __u32 input;
> > __u32 reserved;
> > };
> >
> > If such change is acceptable for everyone, I may help on the
> > implementation.
> > Any comments are welcome.
> >
> > Jun
> >
>
> Hi, Hans/Guennadi
>
> What do you think of supporting this feature?
>
> Jun
>
>
Well, it is definitely not possible to do it in this manner since changing
the size of struct v4l2_buffer will break the API. Furthermore, something like
this will only work if the DMA engine can handle strides. Is that the case
for your hardware? I don't think you mentioned what the hardware is you use.
Regards,
Hans
--
Hans Verkuil - video4linux developer - sponsored by TANDBERG Telecom
Re: Support on discontinuous planer buffer and stride
2009/9/23 Jun Nie :
> Hi,
> I re-send this email for the last one is rejected by system. I am
> sorry if you guys received both.
>
> I am optimizing video playback with overlay with V4L2 driver. The
> video content is a sub-region of codec output. Thus a memory copy is
> necessary.
> Is there plan to support for stride and discrete YUV planer in
> kernel? Such as below changes can help much for my usage case.
>
> --- a/include/linux/videodev2.h
> +++ b/include/linux/videodev2.h
> @@ -529,7 +529,20 @@ struct v4l2_buffer {
> __u32 offset;
> unsigned long userptr;
> } m;
> + /* UV/GB location is valid only in planer case */
> + union {
> + __u32 offset_ug;
> + unsigned long userptr_ug;
> + } m_ug;
> + union {
> + __u32 offset_vb;
> + unsigned long userptr_vb;
> + } m_vb;
> __u32 length;
> + /* stride of YUV or RGB */
> + __u32 stride_yr;
> + __u32 stride_ug;
> + __u32 stride_vb;
> __u32 input;
> __u32 reserved;
> };
>
> If such change is acceptable for everyone, I may help on the
> implementation.
> Any comments are welcome.
>
> Jun
>
Hi, Hans/Guennadi
What do you think of supporting this feature?
Jun
