[FFmpeg-devel] [PATCH 3/3] cllc: Implement ARGB support

[Derek Buitenhuis]

On 11/08/2012 8:41 AM, Reimar Döffinger wrote:
> On Fri, Aug 10, 2012 at 01:07:11PM -0400, Derek Buitenhuis wrote:
>> +        pred[0] += code;
>> +        dst[0]   = pred[0];
>> +
>> +        /* Skip the components if they are  entirely transparent */
>> +        if (dst[0]) {
>> +            /* Red */
>> +            UPDATE_CACHE(bits, gb);
>> +            GET_VLC(code, bits, gb, vlc[1].table, 7, 2);
>> +
>> +            pred[1] += code;
>> +            dst[1]   = pred[1];
>> +
>> +            /* Green */
>> +            UPDATE_CACHE(bits, gb);
>> +            GET_VLC(code, bits, gb, vlc[2].table, 7, 2);
>> +
>> +            pred[2] += code;
>> +            dst[2]   = pred[2];
>> +
>> +            /* Blue */
>> +            UPDATE_CACHE(bits, gb);
>> +            GET_VLC(code, bits, gb, vlc[3].table, 7, 2);
>> +
>> +            pred[3] += code;
>> +            dst[3]   = pred[3];
>> +        } else {
>> +            dst[1] = 0;
>> +            dst[2] = 0;
>> +            dst[3] = 0;
> 
> Those writes are not optimal.
> You could use
> AV_COPY32(dst, pred);
> and
> AV_ZERO(dst);
> respectively.
> However I admit the AV_COPY might needlessly make gcc write the data
> out to stack...

This seems to have made it no faster, but require more changes. I elect to
keep it as-is.

> 
>> +    top_left[0]  = dst[0];
>> +    
>> +    /* Only stash components if they are not transparent */
>> +    if (top_left[0]) {
>> +        top_left[1] = dst[1];
>> +        top_left[2] = dst[2];
>> +        top_left[3] = dst[3];
>> +    }
> 
> AV_COPY might make sense here as well:
> if (dst[0]) AV_COPY32(top_left, dst);
> else top_left[0] = 0;

Same.

- Derek