[Mesa-dev] [PATCH 01/21] mesa/format_unpack: Fix YCBCR unpack

[Brian Paul]

On 06/11/2012 12:59 AM, Pauli Nieminen wrote:
> 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 video requirements.
>
> 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.
>
> 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.
>
> But same time as I was checking call sites I started suspecting that we
> can't ever hit these unpack functions for YCBCR. But I need to study the
> code paths a lot more to be sure if that is true.
>
> 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 at linux.intel.com>

This unpack code isn't used during texture sampling by swrast.  I 
think the only way it would be hit is if one did a glReadPixels from 
ycbcr texture attached to a FBO.  But we should probably disallow that 
(attaching ycbcr textures to FBOs) in the first place.

Have you found another path that hits this code?

The changes look OK in any case.

Reviewed-by: Brian Paul <brianp at vmware.com>

-Brian