[FFmpeg-devel] [PATCH]Simplify targa decoding

[Jean First]

On Tue Jan 17 23:49:18 2012, Carl Eugen Hoyos wrote:
> On Tuesday 17 January 2012 11:34:23 pm Reimar Döffinger wrote:
>> On Tue, Jan 17, 2012 at 11:22:59PM +0100, Carl Eugen Hoyos wrote:
>>> diff --git a/libavcodec/targa.c b/libavcodec/targa.c
>>> index 57a4fee..0210133 100644
>>> --- a/libavcodec/targa.c
>>> +++ b/libavcodec/targa.c
>>> @@ -70,7 +70,7 @@ static int targa_decode_rle(AVCodecContext *avctx,
>>> TargaContext *s, const uint8_ *dst = *src;
>>>                   break;
>>>               case 2:
>>> -                *((uint16_t*)dst) = AV_RL16(src);
>>> +                *((uint16_t*)dst) = *((const uint16_t*)src);
>>>                   break;
>>>               case 3:
>>>                   dst[0] = src[0];
>>> @@ -78,7 +78,7 @@ static int targa_decode_rle(AVCodecContext *avctx,
>>> TargaContext *s, const uint8_ dst[2] = src[2];
>>>                   break;
>>>               case 4:
>>> -                *((uint32_t*)dst) = AV_RL32(src);
>>> +                *((uint32_t*)dst) = *((const uint32_t*)src);
>>
>> When changing it anyway, those might be better to use
>> AV_WN??A and AV_RN??A, it doesn't seem uglier to me than casts
>> and should reliably avoid aliasing issues (even if I can't imagine
>> a compiler ever doing something "wrong" here).
>
> As in attached?
>
>> Making the code using memcpy is an option, too, however I'd expect
>> depending on the content it will make the code anything from a
>> little faster to a whole lot slower.
>
> That was my original idea, but I now prefer the smaller change.

     case 15:
-        avctx->pix_fmt = PIX_FMT_RGB555;
+        avctx->pix_fmt = PIX_FMT_RGB555LE;
         break;
     case 16:
-        avctx->pix_fmt = PIX_FMT_RGB555;
+        avctx->pix_fmt = PIX_FMT_RGB555LE;

nit: these two can be merged.
Jean