[Mesa-dev] [PATCH 2/2] egl/android: Use gralloc::lock_ycbcr for resolving YUV formats
Emil Velikov
emil.l.velikov at gmail.com
Wed Nov 9 20:14:07 UTC 2016
On 9 November 2016 at 08:33, Tomasz Figa <tfiga at chromium.org> wrote:
> There is an interface that can be used to query YUV buffers for their
> internal format. Specifically, if gralloc:lock_ycbcr() is given no SW
> usage flags, it's supposed to return plane offsets instead of pointers.
> Let's use this interface to implement support for YUV formats in Android
> EGL backend.
>
gralloc:lock_ycbcr seems to be missing from gbm_gralloc.
Seems trivial to implement - one can even apply the drm_gralloc commit
a43d9523c050b3ddb40190bb8de7920b7521b6ca directly ;-)
> Signed-off-by: Tomasz Figa <tfiga at chromium.org>
> ---
> src/egl/drivers/dri2/platform_android.c | 146 ++++++++++++++++++++++++++------
> 1 file changed, 120 insertions(+), 26 deletions(-)
>
> diff --git a/src/egl/drivers/dri2/platform_android.c b/src/egl/drivers/dri2/platform_android.c
> index 0b59724..696a22f 100644
> --- a/src/egl/drivers/dri2/platform_android.c
> +++ b/src/egl/drivers/dri2/platform_android.c
> @@ -43,6 +43,49 @@
>
> #define ALIGN(val, align) (((val) + (align) - 1) & ~((align) - 1))
>
> +struct droid_yuv_format {
> + /* Lookup keys */
> + int native;
> + int is_ycrcb;
> + int chroma_step;
> +
> + /* Result */
> + int fourcc;
> +};
> +
> +static const struct droid_yuv_format droid_yuv_formats[] = {
> + { HAL_PIXEL_FORMAT_YCbCr_420_888, 0, 2, __DRI_IMAGE_FOURCC_NV12 },
> + { HAL_PIXEL_FORMAT_YCbCr_420_888, 0, 1, __DRI_IMAGE_FOURCC_YUV420 },
> + { HAL_PIXEL_FORMAT_YCbCr_420_888, 1, 1, __DRI_IMAGE_FOURCC_YVU420 },
> + { HAL_PIXEL_FORMAT_YV12, 1, 1, __DRI_IMAGE_FOURCC_YVU420 },
> +};
> +
This table here feels a bit magical. Please add a comment explaining things.
> +static int
> +get_fourcc_yuv(int native, int is_ycrcb, int chroma_step)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(droid_yuv_formats); ++i)
> + if (droid_yuv_formats[i].native == native &&
> + droid_yuv_formats[i].is_ycrcb == is_ycrcb &&
> + droid_yuv_formats[i].chroma_step == chroma_step)
> + return droid_yuv_formats[i].fourcc;
> +
> + return 0;
get_fourcc() uses -1 for error/not-found. Can we have both functions
behave identically - don't mind each way.
> +}
> +
> +static int
We haven't used it too often - yet "bool" would be nice.
> +is_yuv(int native)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(droid_yuv_formats); ++i)
> + if (droid_yuv_formats[i].native == native)
> + return 1;
> +
> + return 0;
> +}
> +
> static int
> get_format_bpp(int native)
> {
> @@ -57,9 +100,6 @@ get_format_bpp(int native)
> case HAL_PIXEL_FORMAT_RGB_565:
> bpp = 2;
> break;
> - case HAL_PIXEL_FORMAT_YV12:
> - bpp = 1;
> - break;
> default:
> bpp = 0;
> break;
> @@ -76,7 +116,6 @@ static int get_fourcc(int native)
> case HAL_PIXEL_FORMAT_BGRA_8888: return __DRI_IMAGE_FOURCC_ARGB8888;
> case HAL_PIXEL_FORMAT_RGBA_8888: return __DRI_IMAGE_FOURCC_ABGR8888;
> case HAL_PIXEL_FORMAT_RGBX_8888: return __DRI_IMAGE_FOURCC_XBGR8888;
> - case HAL_PIXEL_FORMAT_YV12: return __DRI_IMAGE_FOURCC_YVU420;
> default:
> _eglLog(_EGL_WARNING, "unsupported native buffer format 0x%x", native);
> }
> @@ -479,35 +518,68 @@ droid_swap_buffers(_EGLDriver *drv, _EGLDisplay *disp, _EGLSurface *draw)
> }
>
> static _EGLImage *
> -droid_create_image_from_prime_fd(_EGLDisplay *disp, _EGLContext *ctx,
> - struct ANativeWindowBuffer *buf, int fd)
> +droid_create_image_from_prime_fd_yuv(_EGLDisplay *disp, _EGLContext *ctx,
> + struct ANativeWindowBuffer *buf, int fd)
> {
> - unsigned int offsets[3] = { 0, 0, 0 };
> - unsigned int pitches[3] = { 0, 0, 0 };
> + struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp);
> + struct android_ycbcr ycbcr;
> + size_t offsets[3];
> + size_t pitches[3];
> + int is_ycrcb;
> + int fourcc;
> + int ret;
>
> - const int fourcc = get_fourcc(buf->format);
> - if (fourcc == -1) {
> - _eglError(EGL_BAD_PARAMETER, "eglCreateEGLImageKHR");
> + if (!dri2_dpy->gralloc->lock_ycbcr) {
> + _eglLog(_EGL_WARNING, "Gralloc does not support lock_ycbcr");
> return NULL;
> }
>
> - pitches[0] = buf->stride * get_format_bpp(buf->format);
> - if (pitches[0] == 0) {
> - _eglError(EGL_BAD_PARAMETER, "eglCreateEGLImageKHR");
> + memset(&ycbcr, 0, sizeof(ycbcr));
> + ret = dri2_dpy->gralloc->lock_ycbcr(dri2_dpy->gralloc, buf->handle,
> + 0, 0, 0, 0, 0, &ycbcr);
> + if (ret) {
> + _eglLog(_EGL_WARNING, "gralloc->lock_ycbcr failed: %d", ret);
> return NULL;
> }
> + dri2_dpy->gralloc->unlock(dri2_dpy->gralloc, buf->handle);
> +
> + offsets[0] = (size_t)ycbcr.y;
> + is_ycrcb = (size_t)ycbcr.cb < (size_t)ycbcr.cr;
> + if (is_ycrcb) {
> + offsets[1] = (size_t)ycbcr.cr;
> + offsets[2] = (size_t)ycbcr.cb;
> + } else {
> + offsets[1] = (size_t)ycbcr.cb;
> + offsets[2] = (size_t)ycbcr.cr;
> + }
>
> - switch (buf->format) {
> - case HAL_PIXEL_FORMAT_YV12:
> - /* Y plane is assumed to be at offset 0. */
> - /* Cr plane is located after Y plane */
> - offsets[1] = offsets[0] + pitches[0] * buf->height;
> - pitches[1] = ALIGN(pitches[0] / 2, 16);
> - /* Cb plane is located after Cr plane */
> - offsets[2] = offsets[1] + pitches[1] * buf->height / 2;
> - pitches[2] = pitches[1];
> + pitches[0] = ycbcr.ystride;
> + pitches[1] = pitches[2] = ycbcr.cstride;
>
Going through the system/graphics.h description
@cstride is the stride of the chroma planes
I'd imagine that one should interpret it as:
@cstride is a) the total stride of the chroma planes when
interleaved or b) the individual stride of either chroma plane when
planar.
Doesn't Android support planar YUV 4:2:2/YUV 4:1:1 formats ? If it
only does HAL_PIXEL_FORMAT_YV12 where the Cr Cb strides are the same
that would explain "either chroma plane" above.
Can we add a comment about what cstride is and how it works in this case ?
Mildly related: while skimming through hardware/gralloc.h I've noticed
a duplicate hardware/hardware.h include ;-)
Considering I understood things correctly - with the above trivial
suggestions the patch is
Reviewed-by: Emil Velikov <emil.velikov at collabora.com>
Emil
More information about the mesa-dev
mailing list