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

[Nathan Caldwell]

On Sat, Aug 8, 2009 at 6:32 AM, Michael Niedermayer<michaelni at gmx.at> wrote:
> On Thu, Aug 06, 2009 at 03:06:32PM -0600, 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.
> [...]
>> +
>> +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;
>
> the coder could be put in a seperate file and patch

Done. Just curious though, why?

[...]
> trailing whitespace

Yep, sorry about that. Forgot to fix that before I sent in the patch.

>> +    av_log_set_level(AV_LOG_DEBUG);
>
> that doesnt belong here

Was for debugging, already removed.

>> +        l->low |= 0xff & (l->bytestream[0] << 7 | l->bytestream[1] >> 1);
>
> isnt it possible to refill with normal aligned bytes?

Appears so. Hadn't tried it, just copied the code David posted.

>> +    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.

>> +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.

>> +/* 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.


Thanks everyone for the responses. I think I took care of most of the concerns.
Let me know if there is anything else, so I can fix it up before
getting started on RGB.


--
-Nathan Caldwell
-------------- next part --------------
A non-text attachment was scrubbed...
Name: lagarith-rangecoder-1.patch
Type: text/x-patch
Size: 4592 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090810/03ce295a/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: lagarith-decoder-2.patch
Type: text/x-patch
Size: 19533 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090810/03ce295a/attachment-0001.bin>