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

[Vitor Sessak]

Nathan Caldwell wrote:
> Here's my first attempt at a Lagarith decoder. At the moment it only
> handles YV12 content, I do plan on adding in the other modes (RGB24,
> YUY2, and RGBA). I just wanted some input on things that need changed
> before I get too far along.

Just a few minor things:

> +    uint8_t *bytestream;
> +    uint8_t *bytestream_end;
> +
> +    int prob[257];
> +    int range_hash[256];
> +} lag_rac;
> +
> +/**
> +* local variable storage
> +*/
> +typedef struct LagarithContext{

This comment is redundant.

> +/**
> +* initializes decoder
> +* @param avctx codec context
> +* @return 0 on success or negative if fails
> +*/
> +static av_cold int lag_decode_init(AVCodecContext *avctx)

So is this

> +{
> +    LagarithContext *p_ctx = avctx->priv_data;
> +    
> +    avctx->pix_fmt= PIX_FMT_NONE;
> +    
> +    p_ctx->avctx = avctx;
> +    

> +    av_log_set_level(AV_LOG_DEBUG);

I don't it is up to the decoder to set that.

> +static void lag_rac_init(lag_rac *l, GetBitContext *gb, int length)
> +{
> +    int i,j;
> +
> +    /* According to reference decoder "1st byte is garbage" */ 
> +//    skip_bits(gb, 8);

The commented code line together with the comment is pretty confusing. 
Is the first byte ignored or not?

> +/* Fast round up to least power of 2 >= to x */
> +static inline uint32_t clp2(uint32_t x)
> +/**
> +* closes decoder
> +* @param avctx codec context
> +* @return 0 on success or negative if fails
> +*/
> +static av_cold int lag_decode_end(AVCodecContext *avctx)
> +{
> +    
> +    return 0;
> +}

You can just put a NULL pointer in the AVCodec context, no need to 
create an empty function.

-Vitor