[FFmpeg-devel] [PATCH] MVI demuxer / Motion Pixels decoder

[Gregory Montoir]

Michael Niedermayer wrote:
> On Mon, Jul 07, 2008 at 11:30:14PM +0200, Gregory Montoir wrote:
>> Michael Niedermayer wrote:
>>> On Sat, Jul 05, 2008 at 01:10:15AM +0200, Gregory Montoir wrote:
>>>> Michael Niedermayer wrote:
>>>>> On Thu, Jul 03, 2008 at 11:40:16PM +0200, Gregory Montoir wrote:
> [...]
>>> [...]
>>>> +    for (y = 0; y < mp->avctx->height; ++y) {
>>>> +        if (mp->changes_map[y * mp->avctx->width] != 0) {
>>>> +            yp = y;
>>>> +        } else {
>>>> +            if (yp != -1) {
>>>> +                p = mp_get_yuv_from_rgb(mp, 0, yp);
>>>> +                memset(mp->gradient_scale, 1, 
>>>> sizeof(mp->gradient_scale));
>>>> +                yp = -1;
>>>> +            }
>>>> +            p.y += mp_gradient(mp, 0, mp_get_vlc(mp, gb));
>>>> +            if ((y & 3) == 0) {
>>>> +                p.v += mp_gradient(mp, 1, mp_get_vlc(mp, gb));
>>>> +                p.u += mp_gradient(mp, 2, mp_get_vlc(mp, gb));
>>>> +            }
>>>> +            mp->vpt[y] = p;
>>>> +            mp_set_rgb_from_yuv(mp, 0, y, &p);
>>>> +        }
>>>> +    }
>>> for (y = 0; y < mp->avctx->height; ++y) {
>>>     if (mp->changes_map[y * mp->avctx->width]) {
>>>         p = mp_get_yuv_from_rgb(mp, 0, y);
>>>         memset(mp->gradient_scale, 1, sizeof(mp->gradient_scale));
>>>     } else {
>>>         p.y += mp_gradient(mp, 0, mp_get_vlc(mp, gb));
>>>         if ((y & 3) == 0) {
>>>             p.v += mp_gradient(mp, 1, mp_get_vlc(mp, gb));
>>>             p.u += mp_gradient(mp, 2, mp_get_vlc(mp, gb));
>>>         }
>>>         mp->vpt[y] = p;
>>>         mp_set_rgb_from_yuv(mp, 0, y, &p);
>>>     }
>>> }
>> haven't changed for the moment. the idea was avoid the calls to memset() 
>> and yuv_to_rgb() if there were not necessary (changes_map[y] can be != 0 
>> for several consecutives lines and only the previous line is used when 
>> applying the deltas).
> 
> What speed difference does it make?

I also changed mp_decode_line in the same way but indeed, nothing 
significant (hard to benchmark actually, since using on START/STOP_TIMER 
around decode_frame_helper gives me roughly the same number of cycles in 
both cases).

as it simplifies the code, changed.


> [...]
>> +typedef struct HuffCode {
>> +    int delta;
> 
> uint8_t is enough

changed.


>> +    int code;
> 
>> +    int size;
> 
> uint8_t is enough

changed.


> [...]
>> +static av_cold int mp_decode_init(AVCodecContext *avctx)
>> +{
>> +    MotionPixelsContext *mp = avctx->priv_data;
>> +
> 
>> +    if (!(mp_rgb_yuv_table[0].u | mp_rgb_yuv_table[0].v | mp_rgb_yuv_table[0].y)) {
> 
> 2 of these are unneeded

yes, changed.


> [...]
>> +static void mp_get_code(MotionPixelsContext *mp, GetBitContext *gb, int size, int code)
>> +{
>> +    while (get_bits1(gb)) {
>> +        ++size;
> 
>> +        if (size > mp->max_codes_bits)
>> +            break;
>> +        code <<= 1;
>> +        mp_get_code(mp, gb, size, code + 1);
>> +    }
>> +    if (mp->current_codes_count < MAX_HUFF_CODES) {
>> +        mp->codes[mp->current_codes_count  ].code = code;
>> +        mp->codes[mp->current_codes_count++].size = size;
>> +    }
> 
> some error messages would be nice for these error conditions

added, also made the function return directly in this case.


-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: ffmpeg-mvi-7.diff
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080710/cbb49a74/attachment.txt>