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

[Michael Niedermayer]

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

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

I have never wished to cater to the crowd; for what I know they do not
approve, and what they approve I do not know. -- Epicurus
-------------- 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/20080710/5af03655/attachment.pgp>