[Ffmpeg-devel] [PATCH] AV_R{L,B}{8,16,32}

[Michael Niedermayer]

Hi

On Wed, Jan 17, 2007 at 05:02:18PM +0100, Alex Beregszaszi wrote:
> Hi,
> 
> As discussed in a previous thread (FFmpeg development discussions and
> patches <ffmpeg-devel at mplayerhq.hu>) we might change LE_16/BE_16 to
> include an AV_ prefix.
> 
> Here is the monster patch. Its only cosmetics.

looks ok with the exception below assuming the rest is just cosmetics 
(i am too lazy to actually read the whole)


> 
> The second patch introduces the write methods of the same.

looks ok though it should have been in a seperate mail (simplifies
managing patches on mailinglists)


[...]
>  /* endian macros */
> -#if !defined(BE_16) || !defined(BE_32) || !defined(LE_16) || !defined(LE_32)
> -#define BE_16(x)  ((((uint8_t*)(x))[0] << 8) | ((uint8_t*)(x))[1])
> -#define BE_32(x)  ((((uint8_t*)(x))[0] << 24) | \
> +#if !defined(AV_RB16) || !defined(AV_RB32) || !defined(AV_RL16) || !defined(AV_RL32)

iam agreeing with mans that this #if check does more harm then good


[...]
> Index: qt-faststart.c
> ===================================================================
> --- qt-faststart.c	(revision 7567)
> +++ qt-faststart.c	(working copy)
> @@ -29,8 +29,8 @@
>  #define ftello(x)      ftello64(x)
>  #endif
>  
> -#define BE_16(x) ((((uint8_t*)(x))[0] << 8) | ((uint8_t*)(x))[1])
> -#define BE_32(x) ((((uint8_t*)(x))[0] << 24) | \
> +#define AV_RB16(x) ((((uint8_t*)(x))[0] << 8) | ((uint8_t*)(x))[1])
> +#define AV_RB32(x) ((((uint8_t*)(x))[0] << 24) | \
>                    (((uint8_t*)(x))[1] << 16) | \
>                    (((uint8_t*)(x))[2] << 8) | \
>                     ((uint8_t*)(x))[3])

this looks like code duplication (though thats a seperate issue and has
no effect on this patch, it can be fixed after applying it)

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Opposition brings concord. Out of discord comes the fairest harmony.
-- Heraclitus
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070118/a126e8e7/attachment.pgp>