mesa/format_unpack: Fix YCBCR unpack

The YCBCR unpack was reading one RGBA value from 4 bytes of YCBCR
texture. But single pixel is only 2 bytes in the format.

After noticing that error Ville told me that second chroma sample has to
be average to match MPEG chroma subsampling. MPEG defines horizontally
alligment for chroma sample to be same as first luma sample.

Looking at places where unpack to rgba is called it is possible to get
odd X coordinate and odd number of bytes. That makes unpack operation
complex problem. But I assume that source allocation is 4 byte aligned
always. That should hold for most of allocations but may fail if malloc
is used for small allocations.

Better solution would be threating YCBCR as compressed format
internally. But I suspect that I would break a lot of assumptions
everywhere with that changes.

Changes to fix unpacking:
* Always start reading from the start of chroma pair.
* Always read to the end of chroma pair
* Unpack two RGB values in single iteration
* Read next chroma pair (if available) to calculate average for second
  sample.
* Refactor shared color space conversion code to separate function.

Signed-off-by: Pauli Nieminen <pauli.nieminen@linux.intel.com>
Reviewed-by: Brian Paul <brianp@vmware.com>

1 files changed, 66 insertions, 36 deletions

diff --git a/src/mesa/main/format_unpack.c b/src/mesa/main/format_unpack.c
index c42bac19c7..feac558d88 100644
--- a/src/mesa/main/format_unpack.c
+++ b/src/mesa/main/format_unpack.c
@@ -468,52 +468,82 @@ unpack_I16(const void *src, GLfloat dst[][4], GLuint n)
 }
 
 static void
+YCBR_to_RGB(GLfloat dst[4], GLubyte y, GLubyte cb, GLubyte cr)
+{
+   GLfloat r = 1.164F * (y - 16) + 1.596F * (cr - 128);
+   GLfloat g = 1.164F * (y - 16) - 0.813F * (cr - 128) - 0.391F * (cb - 128);
+   GLfloat b = 1.164F * (y - 16) + 2.018F * (cb - 128);
+   r *= (1.0F / 255.0F);
+   g *= (1.0F / 255.0F);
+   b *= (1.0F / 255.0F);
+   dst[RCOMP] = CLAMP(r, 0.0F, 1.0F);
+   dst[GCOMP] = CLAMP(g, 0.0F, 1.0F);
+   dst[BCOMP] = CLAMP(b, 0.0F, 1.0F);
+   dst[ACOMP] = 1.0F;
+}
+
+static void
 unpack_YCBCR(const void *src, GLfloat dst[][4], GLuint n)
 {
-   GLuint i;
-   for (i = 0; i < n; i++) {
-      const GLushort *src0 = ((const GLushort *) src) + i * 2; /* even */
+   GLuint i, ediff;
+   /* Align the start of reading to chroma pair */
+   uintptr_t sdiff = (uintptr_t)src % 4;
+   src = (const char *)src + sdiff;
+   /* Add extra length for the padding */
+   sdiff /= 2;
+   n += sdiff;
+   /* Align the end to the chroma pair */
+   ediff = n % 2;
+   n += ediff;
+
+   for (i = 0; i < n; i+=2) {
+      const GLushort *src0 = ((const GLushort *) src) + i; /* even */
       const GLushort *src1 = src0 + 1;         /* odd */
-      const GLubyte y0 = (*src0 >> 8) & 0xff;  /* luminance */
-      const GLubyte cb = *src0 & 0xff;         /* chroma U */
-      const GLubyte y1 = (*src1 >> 8) & 0xff;  /* luminance */
-      const GLubyte cr = *src1 & 0xff;         /* chroma V */
-      const GLubyte y = (i & 1) ? y1 : y0;     /* choose even/odd luminance */
-      GLfloat r = 1.164F * (y - 16) + 1.596F * (cr - 128);
-      GLfloat g = 1.164F * (y - 16) - 0.813F * (cr - 128) - 0.391F * (cb - 128);
-      GLfloat b = 1.164F * (y - 16) + 2.018F * (cb - 128);
-      r *= (1.0F / 255.0F);
-      g *= (1.0F / 255.0F);
-      b *= (1.0F / 255.0F);
-      dst[i][RCOMP] = CLAMP(r, 0.0F, 1.0F);
-      dst[i][GCOMP] = CLAMP(g, 0.0F, 1.0F);
-      dst[i][BCOMP] = CLAMP(b, 0.0F, 1.0F);
-      dst[i][ACOMP] = 1.0F;
+      /* Unpack next chroma pair for average if we know there is valid data */
+      const GLushort *src2 = i + 2 < n ? src0 + 2 : src0;
+      const GLushort *src3 = i + 2 < n ? src0 + 3 : src1;
+      const GLubyte y0  = (*src0 >> 8) & 0xff;  /* luminance */
+      const GLshort cb0 = *src0 & 0xff;         /* chroma U */
+      const GLshort cb1 = *src2 & 0xff;         /* chroma U */
+      const GLubyte y1  = (*src1 >> 8) & 0xff;  /* luminance */
+      const GLshort cr0 = *src1 & 0xff;         /* chroma V */
+      const GLshort cr1 = *src3 & 0xff;         /* chroma V */
+      if (i >= sdiff)
+         YCBR_to_RGB(dst[i], y0, cb0, cr0);
+      if (i < n - ediff)
+         YCBR_to_RGB(dst[i+1], y1, (cb0 + cb1)/2, (cr0 + cr1)/2);
    }
 }
 
 static void
 unpack_YCBCR_REV(const void *src, GLfloat dst[][4], GLuint n)
 {
-   GLuint i;
-   for (i = 0; i < n; i++) {
-      const GLushort *src0 = ((const GLushort *) src) + i * 2; /* even */
+   GLuint i, ediff;
+   /* Align the start of reading to chroma pair */
+   uintptr_t sdiff = (uintptr_t)src % 4;
+   src = (const char *)src + sdiff;
+   /* Add extra length for the padding */
+   n+=sdiff/2;
+   /* Align the end to the chroma pair */
+   ediff = n % 2;
+   n += ediff;
+
+   for (i = 0; i < n; i+=2) {
+      const GLushort *src0 = ((const GLushort *) src) + i; /* even */
       const GLushort *src1 = src0 + 1;         /* odd */
-      const GLubyte y0 = *src0 & 0xff;         /* luminance */
-      const GLubyte cr = (*src0 >> 8) & 0xff;  /* chroma V */
-      const GLubyte y1 = *src1 & 0xff;         /* luminance */
-      const GLubyte cb = (*src1 >> 8) & 0xff;  /* chroma U */
-      const GLubyte y = (i & 1) ? y1 : y0;     /* choose even/odd luminance */
-      GLfloat r = 1.164F * (y - 16) + 1.596F * (cr - 128);
-      GLfloat g = 1.164F * (y - 16) - 0.813F * (cr - 128) - 0.391F * (cb - 128);
-      GLfloat b = 1.164F * (y - 16) + 2.018F * (cb - 128);
-      r *= (1.0F / 255.0F);
-      g *= (1.0F / 255.0F);
-      b *= (1.0F / 255.0F);
-      dst[i][RCOMP] = CLAMP(r, 0.0F, 1.0F);
-      dst[i][GCOMP] = CLAMP(g, 0.0F, 1.0F);
-      dst[i][BCOMP] = CLAMP(b, 0.0F, 1.0F);
-      dst[i][ACOMP] = 1.0F;
+      /* Unpack next chroma pair for average if we know there is valid data */
+      const GLushort *src2 = i + 2 < n ? src0 + 2 : src0;
+      const GLushort *src3 = i + 2 < n ? src0 + 3 : src1;
+      const GLubyte y0  = *src0 & 0xff;         /* luminance */
+      const GLshort cr0 = (*src0 >> 8) & 0xff;  /* chroma V */
+      const GLshort cr1 = (*src2 >> 8) & 0xff;  /* chroma V */
+      const GLubyte y1  = *src1 & 0xff;         /* luminance */
+      const GLshort cb0 = (*src1 >> 8) & 0xff;  /* chroma U */
+      const GLshort cb1 = (*src3 >> 8) & 0xff;  /* chroma U */
+      if (i >= sdiff)
+         YCBR_to_RGB(dst[i], y0, cb0, cr0);
+      if (i < n - ediff)
+         YCBR_to_RGB(dst[i+1], y1, (cb0 + cb1)/2, (cr0 + cr1)/2);
    }
 }