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

[Ramiro Polla]

Michael Niedermayer wrote:
> On Thu, Jul 10, 2008 at 12:00:01AM +0200, Gregory Montoir wrote:
>> 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.
> 
> 
> patch looks ok

Tested with http://samples.mplayerhq.hu/mvi/INTRO.MVI .

Applied demuxer and decoder separately with minor version bumps on each 
library.

Ramiro Polla