[FFmpeg-devel] [PATCH][RFC] Lagarith Decoder.

Michael Niedermayer michaelni
Wed Aug 12 14:12:55 CEST 2009


On Mon, Aug 10, 2009 at 11:42:19PM -0600, Nathan Caldwell wrote:
> On Sat, Aug 8, 2009 at 6:32 AM, Michael Niedermayer<michaelni at gmx.at> wrote:
[...]
> >> +    low_scaled = l->low / range_scaled;
> >
> > this possibly could be done with a LUT (of course that only makes
> > sense if it is faster
> 
> Since the denominator is the upper two bytes of range, that makes a
> fairly large LUT (16k * 6bytes).
> 

> >> +    if(n == 0) return;
> >
> > this is never true
> 
> It's actually true quite a bit. there are quite a few zero runs that
> trigger the escape code, but are a run of 0. In that case 0 gets
> passed as the number of bytes.

i mixed it up with "step" while checking, still the check does not belong in
there, its more efficient to check in the caller because alot more can be
skiped if its 0 also some calls do not look like they can pass 0


> 
> >> +static inline int lag_predict(uint8_t *p_src, int i_stride, int i_step)
> >> +{
> >> +    int T  = p_src[-i_stride];
> >> +    int L  = p_src[-i_step];
> >> +    int TL = p_src[-i_stride - i_step];
> >> +
> >> +    return mid_pred( T, L, L + T - TL);
> >> +}
> >
> > thats a duplicate
> 
> How so?
> There are a few other decoders that have very similar functions in them.

yes that way


> 
> >> +/* Fast round up to least power of 2 >= to x */
> >> +static inline uint32_t clp2(uint32_t x)
> >> +{
> >> +    x--;
> >> +    x |= (x >> 1);
> >> +    x |= (x >> 2);
> >> +    x |= (x >> 4);
> >> +    x |= (x >> 8);
> >> +    x |= (x >> 16);
> >> +    return x+1;
> >> +}
> >
> > is 1<<av_log2(x) faster?
> 
> Might be, but it gives different results, so it's a moot point.

2<<av_log2(x-1)
or whatever


[...]
> +typedef struct lag_rac {
> +    AVCodecContext *avctx;
> +    unsigned low;
> +    unsigned range;
> +    unsigned scale;
> +    unsigned hash_shift;
> +
> +    uint8_t *bytestream_start;
> +    uint8_t *bytestream;
> +    uint8_t *bytestream_end;
> +
> +    int prob[257];
> +    int range_hash[256];
> +} lag_rac;

these could benefit from some documentation


[...]
> +typedef struct LagarithContext {
> +    AVCodecContext *avctx;
> +    AVFrame picture;

> +    int zeros;
> +    int zeros_rem;

missing documentation


> +} LagarithContext;
> +
> +static av_cold int lag_decode_init(AVCodecContext * avctx)
> +{
> +    LagarithContext *l = avctx->priv_data;
> +

> +    avctx->pix_fmt = PIX_FMT_NONE;

is that not default?


[...]
> +static void lag_pred_line(LagarithContext * l, uint8_t * buf,
> +                          int width, int stride, int step, int mode)
> +{

mode really is the line number, thus its not an ideal variable name


[...]
> +
> +static int lag_decode_line(LagarithContext * l, lag_rac * rac,
> +                           uint8_t * dst, int width, int stride,
> +                           int step, int esc_count)
> +{
> +    int i = 0;
> +    int ret = 0;
> +
> +    /* Output any zeros remaining from the previous run */
> +    if (l->zeros_rem) {

> +        int count = l->zeros_rem;
> +        if (l->zeros_rem > width)
> +            count = width;

count= FFMIN(l->zeros_rem, width);


[...]
> +static int lag_decode_arith_plane(LagarithContext * l, uint8_t * dst,
> +                                  int width, int height, int stride,
> +                                  int step, const uint8_t * src,
> +                                  int src_size)
> +{
> +    int esc_count = src[0];
> +    int i = 0;
> +    int read = 0;
> +    uint32_t offset = 1;
> +    uint32_t length;
> +    GetBitContext gb;
> +    lag_rac rac;
> +
> +    rac.avctx = l->avctx;
> +
> +    if (esc_count < 4) {

> +        if (esc_count != 0) {

the != 0 is superflous


[...]
> +enum LagarithFrameType {
> +    FRAME_RAW           = 1,    /* Uncompressed */
> +    FRAME_U_RGB24       = 2,    /* Unaligned RGB24 */
> +    FRAME_ARITH_YUY2    = 3,    /* Arith coded YUY2 */
> +    FRAME_ARITH_RGB24   = 4,    /* Arith coded RGB24 */
> +    FRAME_SOLID_GRAY    = 5,    /* Solid grayscale color frame */
> +    FRAME_SOLID_COLOR   = 6,    /* Solid non-grayscale color frame */
> +    FRAME_OLD_ARITH_RGB = 7,    /* Obsolete arithmetic coded RGB keyframe (Maintained for backwards compatibility) */
> +    FRAME_ARITH_RGBA    = 8,    /* Arithmetic coded RGBA */
> +    FRAME_SOLID_RGBA    = 9,    /* Solid RGBA color frame */
> +    FRAME_ARITH_YV12    = 10,   /* Arithmetic coded YV12 */
> +    FRAME_REDUCED_RES   = 11,   /* Reduced resolution frame */
> +};

not doxygen compatible


> +
> +
> +static const uint8_t run_table[]={0,2,4,6,8,10,12,14,16,18,20,22,24,26,28,30,32,
> +	34,36,38,40,42,44,46,48,50,52,54,56,58,60,62,64,66,68,70,72,74,76,78,80,82,84,
> +	86,88,90,92,94,96,98,100,102,104,106,108,110,112,114,116,118,120,122,124,126,
> +	128,130,132,134,136,138,140,142,144,146,148,150,152,154,156,158,160,162,164,
> +	166,168,170,172,174,176,178,180,182,184,186,188,190,192,194,196,198,200,202,
> +	204,206,208,210,212,214,216,218,220,222,224,226,228,230,232,234,236,238,240,
> +	242,244,246,248,250,252,254,255,253,251,249,247,245,243,241,239,237,235,233,
> +	231,229,227,225,223,221,219,217,215,213,211,209,207,205,203,201,199,197,195,
> +	193,191,189,187,185,183,181,179,177,175,173,171,169,167,165,163,161,159,157,
> +	155,153,151,149,147,145,143,141,139,137,135,133,131,129,127,125,123,121,119,
> +	117,115,113,111,109,107,105,103,101,99,97,95,93,91,89,87,85,83,81,79,77,75,73,
> +	71,69,67,65,63,61,59,57,55,53,51,49,47,45,43,41,39,37,35,33,31,29,27,25,23,21,
> +	19,17,15,13,11,9,7,5,3,1};

tabs are forbidden in ffmpeg svn

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

No human being will ever know the Truth, for even if they happen to say it
by chance, they would not even known they had done so. -- Xenophanes
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090812/09961d77/attachment.pgp>



More information about the ffmpeg-devel mailing list