[FFmpeg-devel] [PATCH][VAAPI][6/6] Add H.264 bitstream decoding (take 6)

[Michael Niedermayer]

On Thu, Apr 16, 2009 at 11:30:27PM +0200, Gwenole Beauchesne wrote:
> Le 16 avr. 09 ? 20:00, Michael Niedermayer a ?crit :
>
>>> +    pic_param->bit_depth_luma_minus8                            = 
>>> h->sps.bit_depth_luma   >= 8 ? h->sps.bit_depth_luma   - 8 : 0;
>>> +    pic_param->bit_depth_chroma_minus8                          = 
>>> h->sps.bit_depth_chroma >= 8 ? h->sps.bit_depth_chroma - 8 : 0;
>>
>> it cant be smaller 8 (not counting overflows on broken streams)
>
> Mmmm, I think I had a case where it was needed, I will check again with my 
> samples tomorrow.
>
>>> +            p_luma_weight_flag   = &slice_param->luma_weight_l1_flag;
>>> +            p_luma_weight        = slice_param->luma_weight_l1;
>>> +            p_luma_offset        = slice_param->luma_offset_l1;
>>> +            p_chroma_weight_flag = &slice_param->chroma_weight_l1_flag;
>>> +            p_chroma_weight      = &slice_param->chroma_weight_l1;
>>> +            p_chroma_offset      = &slice_param->chroma_offset_l1;
>>> +        }
>>
>> could be simplified with a macro
>
> Like in the new attachment?
[...]

> +static void dpb_add(DPB *dpb, Picture *ff_pic)
> +{
> +    unsigned int i;
> +
> +    for (i = 0; i < dpb->size; i++) {
> +        VAPictureH264 * const pic = &dpb->pics[i];
> +        if (pic->picture_id == ff_vaapi_get_surface(ff_pic)) {
> +            VAPictureH264 new_pic;
> +            vaapi_h264_fill_picture(&new_pic, ff_pic, 0);
> +            if ((new_pic.flags ^ pic->flags) & (VA_PICTURE_H264_TOP_FIELD | VA_PICTURE_H264_BOTTOM_FIELD)) {
> +                /* Merge second field */
> +                if (new_pic.flags & VA_PICTURE_H264_TOP_FIELD) {
> +                    pic->flags              |= VA_PICTURE_H264_TOP_FIELD;
> +                    pic->TopFieldOrderCnt    = new_pic.TopFieldOrderCnt;
> +                }
> +                if (new_pic.flags & VA_PICTURE_H264_BOTTOM_FIELD) {
> +                    pic->flags              |= VA_PICTURE_H264_BOTTOM_FIELD;
> +                    pic->BottomFieldOrderCnt = new_pic.BottomFieldOrderCnt;
> +                }
> +            }
> +            return;
> +        }
> +    }
> +
> +    assert(dpb->size < dpb->max_size);
> +    vaapi_h264_fill_picture(&dpb->pics[dpb->size++], ff_pic, 0);

i think this assert should be a hard check as its failure looks
bad



[...]
> +        typedef short (*chroma_table_t)[32][2];
> +        unsigned char *p_luma_weight_flag;
> +        unsigned char *p_chroma_weight_flag;
[...]
> +
> +#define init_reflist(n) do {                                            \
> +        p_luma_weight_flag   = &slice_param->luma_weight_l##n##_flag;   \
[...]
> +        p_chroma_weight_flag = &slice_param->chroma_weight_l##n##_flag; \
[...]
> +    } while (0)
> +
> +        if (list == 0)
> +            init_reflist(0);
> +        else
> +            init_reflist(1);
> +
> +        *p_luma_weight_flag   = h->luma_weight_flag[list];
> +        *p_chroma_weight_flag = h->chroma_weight_flag[list];

and whats the sense of this double indirection?!

besides i prefer macros to use all uppercase names, not that thats really
important


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

Avoid a single point of failure, be that a person or equipment.
-------------- 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/20090417/3d2c848c/attachment.pgp>