[FFmpeg-devel] [PATCH]Simplify targa decoding

Reimar Döffinger Reimar.Doeffinger at gmx.de
Tue Jan 17 23:55:31 CET 2012


On Tue, Jan 17, 2012 at 11:51:42PM +0100, Jean First wrote:
> 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.

Patch seems fine to me.

>     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.

True, though unrelated.
I assume someone tested these are actually correct
and the second one should not be 565?


More information about the ffmpeg-devel mailing list