[FFmpeg-devel] [RFC] mixed-endian get_bits

[Michael Niedermayer]

On Thu, Sep 30, 2010 at 09:16:43PM +0200, Reimar D?ffinger wrote:
> Hello,
> this patch would, when ALT_BITSTREAM_READER is defined, provide an
> alternative get_bits_le that behaves as the get_bits when
> ALT_BITSTREAM_READER_LE is defined.
> This is a hack that is due to the fact the GSM 06.10 and the MS
> variant use different bitstream layouts.
> The best idea I had without bloating neither code nor binary size
> involes using this as in attached patch.
> There are a lot of other ways to do this of course (manually parsing
> the bitstream like other GSM decoders do certainly is not one I'd
> consider good though).

>  get_bits.h        |   28 ++++++++++++++++++++++------
>  gsmdec.c          |   49 +++++++++++++++++++++++--------------------------
>  gsmdec_template.c |   21 +++++++++++++++++++++
>  3 files changed, 66 insertions(+), 32 deletions(-)
> 50ea69dac6afac341f0cb7506f253f5ec54c548c  gsm.diff
> Index: libavcodec/get_bits.h
> ===================================================================
> --- libavcodec/get_bits.h	(revision 25271)
> +++ libavcodec/get_bits.h	(working copy)
> @@ -137,12 +137,14 @@
>  #   define CLOSE_READER(name, gb)\
>          (gb)->index= name##_index;\
>  
> -# ifdef ALT_BITSTREAM_READER_LE
> -#   define UPDATE_CACHE(name, gb)\
> +#   define UPDATE_CACHE_LE(name, gb)\
>          name##_cache= AV_RL32( ((const uint8_t *)(gb)->buffer)+(name##_index>>3) ) >> (name##_index&0x07);\
>  
> -#   define SKIP_CACHE(name, gb, num)\
> +#   define SKIP_CACHE_LE(name, gb, num)\
>          name##_cache >>= (num);
> +# ifdef ALT_BITSTREAM_READER_LE
> +#   define UPDATE_CACHE(name, gb) UPDATE_CACHE_LE(name, gb)
> +#   define SKIP_CACHE(name, gb, num) SKIP_CACHE_LE(name, gb, num)
>  # else
>  #   define UPDATE_CACHE(name, gb)\
>          name##_cache= AV_RB32( ((const uint8_t *)(gb)->buffer)+(name##_index>>3) ) << (name##_index&0x07);\
> @@ -164,12 +166,14 @@
>  #   define LAST_SKIP_BITS(name, gb, num) SKIP_COUNTER(name, gb, num)
>  #   define LAST_SKIP_CACHE(name, gb, num) ;
>  
> -# ifdef ALT_BITSTREAM_READER_LE
> -#   define SHOW_UBITS(name, gb, num)\
> +#   define SHOW_UBITS_LE(name, gb, num)\
>          zero_extend(name##_cache, num)
>  
> -#   define SHOW_SBITS(name, gb, num)\
> +#   define SHOW_SBITS_LE(name, gb, num)\
>          sign_extend(name##_cache, num)
> +# ifdef ALT_BITSTREAM_READER_LE
> +#   define SHOW_UBITS(name, gb, num) SHOW_UBITS_LE(name, gb, num)
> +#   define SHOW_SBITS(name, gb, num) SHOW_SBITS_LE(name, gb, num)
>  # else
>  #   define SHOW_UBITS(name, gb, num)\
>          NEG_USR32(name##_cache, num)
> @@ -355,6 +359,18 @@
>      return tmp;
>  }
>  
> +#ifdef ALT_BITSTREAM_READER
> +static inline unsigned int get_bits_le(GetBitContext *s, int n){
> +    register int tmp;
> +    OPEN_READER(re, s)
> +    UPDATE_CACHE_LE(re, s)
> +    tmp= SHOW_UBITS_LE(re, s, n);
> +    LAST_SKIP_BITS(re, s, n)
> +    CLOSE_READER(re, s)
> +    return tmp;
> +}
> +#endif
> +
>  /**
>   * reads 1-17 bits.
>   * Note, the alt bitstream reader can read up to 25 bits, but the libmpeg2 reader can't

This patch entangles 2 bitstream readers and breaks the others (due to no
longer implementing the whole API)

if you want to allow get_bits() being #included 2 times ones for a LE
and once for a BE reader (they can use different structs for the context too
your patch assumes they are equal which is not reasonable)
then i dont mind though it seems like huge work for a problem that has a simple
solution, You compile the whole codec twice and factor out what is common
in a 3rd file
the whole codec after strip with full optimizations is just 8kb object file


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Asymptotically faster algorithms should always be preferred if you have
asymptotical amounts of data
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20101001/8e3a54bb/attachment.pgp>