[FFmpeg-devel] [PATCH] Fix unaligned access in lcl decoder

[Reimar Döffinger]

On Wed, May 27, 2009 at 02:31:56PM -0400, Jeff Downs wrote:
> On Wed, 27 May 2009, Michael Niedermayer wrote:
> 
> > On Wed, May 27, 2009 at 12:22:19PM -0400, Jeff Downs wrote:
> > > Attached fixes by using memcpy instead of integer copies on platforms 
> > > without fast unaligned access.
> > 
> > AV_R/WN32() might be more appropriate than the ifdeffery
> > 
> 
> Much cleaner looking, yes. Attached.
> 
> Probably a slight speed hit on platforms where AV_R/WN32 are non-optimized 
> though due to all the shifts. I don't have a platform where that is 
> realistically the case to test it on, but if I force that implementation 
> it is slower than memcpy (2% with the test sample on sparc).
> 
> 	-Jeff
> Index: libavcodec/lcldec.c
> ===================================================================
> --- libavcodec/lcldec.c	(revision 18893)
> +++ libavcodec/lcldec.c	(working copy)
> @@ -129,7 +129,7 @@
>          if ((mask & (1 << (--maskbit))) == 0) {
>              if (destptr + 4 > destptr_end)
>                  break;
> -            *(int*)destptr = *(int*)srcptr;
> +            AV_WN32(destptr, AV_RN32(srcptr));
>              srclen -= 4;
>              destptr += 4;
>              srcptr += 4;

Did you check the compiler does not make a mess of it, due to multiple evaluation of AV_RN?
I'd feel mor comfortable with a temporary variable in there.
Well, actually I wonder, is there any reason why these need to be macros instead of
static inline functions?