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

[Michael Niedermayer]

On Thu, Apr 02, 2009 at 11:18:08AM +0200, Gwenole Beauchesne wrote:
> On Wed, 25 Mar 2009, Gwenole Beauchesne wrote:
>
>> On Sun, 22 Mar 2009, Gwenole Beauchesne wrote:
>>
>>>>> New patch attached to cope with source code split, hwaccel_data_private 
>>>>> infrastructure and VA API headers cleanup.
>>>> New patch attached to cope with "recent" changes.
>>> New patch attached to cope with (applied) common parts. Also add missing 
>>> hunk in allcodecs.c to register this new HW acceleratored codec.
>>
>> Ping.
>> <https://lists.mplayerhq.hu/pipermail/ffmpeg-devel/2009-March/065954.html>
>
> New patch attached to cope with new VA API buffers allocation logic, 
> removal of single-line functions, and a few more cosmetics.
>
> For the record:
>   476 ffmpeg.hwaccel.vaapi.h264.patch
>   495 ffmpeg.hwaccel.vaapi.h264.2.patch
>   438 ffmpeg.hwaccel.vaapi.h264.3.patch
>   466 ffmpeg.hwaccel.vaapi.h264.4.patch
>   367 ffmpeg.hwaccel.vaapi.h264.5.patch


[...]
> +static void vaapi_h264_fill_ReferenceFrames(VAPictureParameterBufferH264 *pic_param, H264Context *h)
> +{
> +    DPB dpb;
> +    unsigned int i, l;
> +
> +    dpb_init(&dpb,
> +             pic_param->ReferenceFrames,
> +             FF_ARRAY_ELEMS(pic_param->ReferenceFrames));
> +
> +    for (l = 0; l < h->list_count; l++) {

please use "list" not l, l is the worst single letter variable because it
looks like 1 in some fonts.


[...]
> +static int vaapi_h264_start_frame(AVCodecContext *avctx, av_unused const uint8_t *buffer, av_unused uint32_t size)
> +{
> +    H264Context * const h = avctx->priv_data;
> +    MpegEncContext * const s = &h->s;
> +    struct vaapi_context * const vactx = avctx->hwaccel_context;
> +    VAPictureParameterBufferH264 *pic_param;
> +    VAIQMatrixBufferH264 *iq_matrix;
> +
> +    dprintf(avctx, "vaapi_h264_start_frame()\n");
> +
> +    vactx->slice_param_size = sizeof(VASliceParameterBufferH264);
> +
> +    /* Fill in VAPictureParameterBufferH264 */
> +    pic_param = ff_vaapi_alloc_picture(vactx, sizeof(VAPictureParameterBufferH264));
> +    if (!pic_param)
> +        return -1;
> +    vaapi_h264_fill_picture(&pic_param->CurrPic, s->current_picture_ptr, s->picture_structure);
> +    vaapi_h264_fill_ReferenceFrames(pic_param, h);
> +    pic_param->picture_width_in_mbs_minus1                      = s->mb_width - 1;
> +    pic_param->picture_height_in_mbs_minus1                     = s->mb_height - 1;

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


[...]

> +    for (l = 0; l < 2; l++) {

list ...


> +        typedef short (*chroma_table_t)[32][2];
> +        unsigned char *p_luma_weight_flag;
> +        unsigned char *p_chroma_weight_flag;
> +        short *p_luma_weight;
> +        short *p_luma_offset;
> +        chroma_table_t p_chroma_weight;
> +        chroma_table_t p_chroma_offset;

> +        const int luma_def = 1 << h->luma_log2_weight_denom;
> +        const int chroma_def = 1 << h->chroma_log2_weight_denom;

vertical align


> +
> +        if (l == 0) {
> +            p_luma_weight_flag   = &slice_param->luma_weight_l0_flag;
> +            p_luma_weight        = slice_param->luma_weight_l0;
> +            p_luma_offset        = slice_param->luma_offset_l0;
> +            p_chroma_weight_flag = &slice_param->chroma_weight_l0_flag;
> +            p_chroma_weight      = &slice_param->chroma_weight_l0;
> +            p_chroma_offset      = &slice_param->chroma_offset_l0;

> +        }
> +        else {

not K&R style


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


> +
> +        *p_luma_weight_flag = h->luma_weight_flag[l];
> +        *p_chroma_weight_flag = h->chroma_weight_flag[l];

align

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

Frequently ignored awnser#1 FFmpeg bugs should be sent to our bugtracker. User
questions about the command line tools should be sent to the ffmpeg-user ML.
And questions about how to use libav* should be sent to the libav-user ML.
-------------- 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/20090416/976feb94/attachment.pgp>