[Mesa-dev] [PATCH 1/8] __DRIimage: version 5, add new formats and createSubImage

[nobled]

On Sat, Jul 7, 2012 at 1:40 AM, Kristian Høgsberg <hoegsberg at gmail.com> wrote:
> On Fri, Jul 06, 2012 at 02:10:57PM -0700, Eric Anholt wrote:
>> Kristian Høgsberg <krh at bitplanet.net> writes:
>>
>> > The additions in version 5 enables creating EGLImages for different planes
>> > of a YUV buffer.  createImageFromName is still used to create the containing
>> > __DRIimage, and createSubImage can then be used no that __DRIimage to create
>> > __DRIimages that correspond to the y, u, and v planes (__DRI_IMAGE_FORMAT_R8)
>> > or the uv planes (__DRI_IMAGE_FORMAT_RG88) for formats such as NV12 where
>> > the u and v components are interleaved.  Packed formats such as YUYV etc
>> > doesn't require any special treatment, we just sample those as a regular
>> > ARGB texture.
>>
>> This patch series is making more sense to me than the last few yuv
>> patchsets I've seen.
>
> Thanks for the feedback.  I wanted to mention that while it would be a
> lot easier to use different bo's for the different planes, we can't do
> that.  The decoder takes a base address for the destination buffer and
> then offsets relative to that to identify the various planes.  So it
> has to be all within the same bo and the patch series has to touch the
> miptree setup.
>
> Also, after I sent out the series I started working on YUYV support
> (which is what the SNB+ sprites take), and the sub-image feature turns
> out to be useful there as well.  We can create an RG88 texture for
> sampling the Y-component and then an RGBA8888 texture for sampling the
> U and V channels, which means that we can texture from a YUYV buffer
> with bilinear filtering.
>
>> > Signed-off-by: Kristian Høgsberg <krh at bitplanet.net>
>> > ---
>> >  include/GL/internal/dri_interface.h |   16 ++++++++++++++++
>> >  1 file changed, 16 insertions(+)
>> >
>> > diff --git a/include/GL/internal/dri_interface.h b/include/GL/internal/dri_interface.h
>> > index e37917e..5dfe15b 100644
>> > --- a/include/GL/internal/dri_interface.h
>> > +++ b/include/GL/internal/dri_interface.h
>> > @@ -907,6 +907,10 @@ struct __DRIdri2ExtensionRec {
>> >  #define __DRI_IMAGE_FORMAT_ARGB8888     0x1003
>> >  #define __DRI_IMAGE_FORMAT_ABGR8888     0x1004
>> >  #define __DRI_IMAGE_FORMAT_XBGR8888     0x1005
>> > +#define __DRI_IMAGE_FORMAT_R8           0x1006 /* Since version 5 */
>> > +#define __DRI_IMAGE_FORMAT_RG88         0x1007
>> > +#define __DRI_IMAGE_FORMAT_YUV420       0x1008
>> > +#define __DRI_IMAGE_FORMAT_NV12         0x1009
>>
>> Could we get just a couple notes on the yuv420/nv12 formats here?  Also,
>> can one of these end up as an OES_EGL_image that someone might try to
>> bind as a texture?  What should happen, if so?  (If it's possible, the
>> MESA_FORMAT_NONE in the next patch would result in a bunch of
>> _mesa_problem()s and failure)
>
> The __DRIimage we create with either of the YUV formats is never
> exposed as an EGLImage.  We hold on to it in the wl_buffer object and
> use it to create subimages of one of the types that we can actually
> texture from and those get exposed as EGLImages.  I had
> __DRI_IMAGE_FORMAT_NONE instead of the YUV formats, I think that may
> be clearer.
>
>> >  #define __DRI_IMAGE_USE_SHARE              0x0001
>> >  #define __DRI_IMAGE_USE_SCANOUT            0x0002
>> > @@ -963,6 +967,18 @@ struct __DRIimageExtensionRec {
>> >      * \since 4
>> >      */
>> >     int (*write)(__DRIimage *image, const void *buf, size_t count);
>> > +
>> > +   /**
>> > +    * Create an image out of a sub-region of a parent image.  This
>> > +    * entry point lets us create individual __DRIimages for different
>> > +    * planes in a planar buffer (typically yuv), for example.
>> > +    *
>> > +    * \since 5
>> > +    */
>> > +    __DRIimage *(*createSubImage)(__DRIimage *image,
>> > +                                  int width, int height, int format,
>> > +                                  int offset, int pitch,
>> > +                                  void *loaderPrivate);
>> >  };
>>
>> Some notes about lifetime would be good here.  In the implementation you
>> have, the subimage gets separated from the parent, so changing the
>> storage of the parent (if that's a thing that's possible -- I don't
>> understand egl image lifetimes) wouldn't be tracked in the child.  You
>> could free the parent safely, as well.
>
> The lifetime rules are pretty simple; each subimage is an independent
> __DRIimage and its lifetime is not tied to the that of the parent.
> I'll put that in the docs.
>
>> If the subimage can get bound as a render target, it won't be reliably
>> synchronized with texturing from another subimage that overlaps it, and
>> that seems like something to be noted.
>
> I didn't intend for the subimages to end up as render targets, but
> there's nothing in the API that prevents that.  I guess the semantics
> I have in mind are much like the image_external restrictions, where
> you can't write to the EGLImage in any way (no glTexImage2D or
> glTexSubImage2D, no binding as rendertarget, no rendering to the image
> in OpenVG etc).  I can add language to the wayland GLES2 extension
> that exposes the subimages to say that any such usage will result in
> undefined behavior.
Wouldn't it be even better to specify that INVALID_OPERATION is given
if you try to pass this kind of EGLImage to
glEGLImageTargetRenderbufferStorageOES(), and that you can only pass
it to glEGLImageTargetTexture2DOES() with a texture target of
GL_TEXTURE_EXTERNAL_OES?

>
>> Are there supposed to be any restrictions on the layout of
>> width/height/offset/pitch related to the parent?  For tiled buffers,
>> there would be.  Is this totally driver-dependent?
>
> It is all driver-dependent.  The subimage offsets are expected to be
> picked by a driver on the client side that understands the
> restrictions there may be, such that a properly aligned bo will result
> in properly aligned subimages.  The parents width/height/offset/stride
> doesn't really matter, though, we're mostly using the parent as a
> reference to the underlying bo.  For yuv formats, the different planes
> have different width, height and stride.  They share the same bo,
> which means that they are all tiled the same way, of course.
>
> Kristian
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev