[FFmpeg-devel] [PATCH][VAAPI][3/6] Add MPEG-2 bitstream decoding (take 5)

Michael Niedermayer michaelni
Sun Mar 22 04:05:12 CET 2009


On Sun, Mar 22, 2009 at 02:44:35AM +0100, Gwenole Beauchesne wrote:
> On Wed, 18 Mar 2009, Gwenole Beauchesne wrote:
>
>> On Wed, 11 Mar 2009, Gwenole Beauchesne wrote:
>>
>>> On Fri, 6 Mar 2009, Gwenole Beauchesne wrote:
>>>> On Wed, 4 Mar 2009, Aurelien Jacobs wrote:
>>>>>> +#if CONFIG_MPEG2_VAAPI_HWACCEL
>>>>>> +AVHWAccel mpeg2_vaapi_hwaccel = {
>>>>>> +    .name           = "mpeg2_vaapi",
>>>>>> +    .type           = CODEC_TYPE_VIDEO,
>>>>>> +    .id             = CODEC_ID_MPEG2VIDEO,
>>>>>> +    .pix_fmt        = PIX_FMT_VAAPI_VLD,
>>>>>> +    .capabilities   = 0,
>>>>>> +    .start_frame    = vaapi_mpeg2_start_frame,
>>>>>> +    .end_frame      = ff_vaapi_common_end_frame,
>>>>>> +    .decode_slice   = vaapi_mpeg2_decode_slice,
>>>>>> +    .priv_data_size = sizeof(struct vaapi_render_state_private),
>>>>>> +    .close          = ff_vaapi_destroy_picture,
>>>>>> +};
>>>>>> +#endif
>>>>> This file won't ever be compiled without CONFIG_MPEG2_VAAPI_HWACCEL
>>>>> so the #if is useless.
>>>>> I guess those remarks also apply to every codec specific vaapi_*.c 
>>>>> files.
>>>> vaapi_{mpeg4,vc1}.c contain several hwaccels, but I will drop it for
>>>> vaapi_{mpeg2,h264}.c.
>>> New patch attached.
>>
>> Ping. New patch attached to cope with recent changes of common parts + mix 
>> decoders and HW accelerators in Makefile per Diego's advise.
>
> New patch attached to cope with recent changes in (committed) common parts. 
> Also add missing hunk for allcodecs.c to register the new HW accelerator.

[...]
> +/** Reconstruct bitstream f_code */
> +static inline int mpeg2_get_f_code(MpegEncContext *s)
> +{
> +    return ((s->mpeg_f_code[0][0] << 12) |
> +            (s->mpeg_f_code[0][1] << 8) |
> +            (s->mpeg_f_code[1][0] << 4) | s->mpeg_f_code[1][1]);

dont you think this is terribly formated?
i mean id either place them in a aligned 2x2 block or in 4 lines ...



> +}
> +
> +/** Is the current field the first field for field picture? */
> +static inline int mpeg2_get_is_first_field(MpegEncContext *s)
> +{
> +    return s->first_field || s->picture_structure == PICT_FRAME;
> +}

picture_structure == PICT_FRAME are not field pics


> +
> +static int vaapi_mpeg2_start_frame(AVCodecContext *avctx, av_unused const uint8_t *buffer, av_unused uint32_t size)
> +{
> +    struct MpegEncContext * const s = avctx->priv_data;
> +    struct vaapi_context * const vactx = avctx->hwaccel_context;
> +    VAPictureParameterBufferMPEG2 *pic_param;
> +    VAIQMatrixBufferMPEG2 *iq_matrix;
> +    uint8_t m[64];
> +    int i;
> +
> +    dprintf(avctx, "vaapi_mpeg2_start_frame()\n");
> +
> +    vactx->pic_param_size   = sizeof(VAPictureParameterBufferMPEG2);
> +    vactx->iq_matrix_size   = sizeof(VAIQMatrixBufferMPEG2);
> +    vactx->slice_param_size = sizeof(VASliceParameterBufferMPEG2);
> +
> +    /* Fill in VAPictureParameterBufferMPEG2 */
> +    pic_param = &vactx->pic_param.mpeg2;
> +    pic_param->horizontal_size                                  = s->width;
> +    pic_param->vertical_size                                    = s->height;
> +    pic_param->forward_reference_picture                        = 0xffffffff;
> +    pic_param->backward_reference_picture                       = 0xffffffff;
> +    pic_param->picture_coding_type                              = s->pict_type;
> +    pic_param->f_code                                           = mpeg2_get_f_code(s);
> +    pic_param->picture_coding_extension.value                   = 0; /* reset all bits */
> +    pic_param->picture_coding_extension.bits.intra_dc_precision = s->intra_dc_precision;
> +    pic_param->picture_coding_extension.bits.picture_structure  = s->picture_structure;
> +    pic_param->picture_coding_extension.bits.top_field_first    = s->top_field_first;
> +    pic_param->picture_coding_extension.bits.frame_pred_frame_dct = s->frame_pred_frame_dct;
> +    pic_param->picture_coding_extension.bits.concealment_motion_vectors = s->concealment_motion_vectors;
> +    pic_param->picture_coding_extension.bits.q_scale_type       = s->q_scale_type;
> +    pic_param->picture_coding_extension.bits.intra_vlc_format   = s->intra_vlc_format;
> +    pic_param->picture_coding_extension.bits.alternate_scan     = s->alternate_scan;
> +    pic_param->picture_coding_extension.bits.repeat_first_field = s->repeat_first_field;
> +    pic_param->picture_coding_extension.bits.progressive_frame  = s->progressive_frame;
> +    pic_param->picture_coding_extension.bits.is_first_field     = mpeg2_get_is_first_field(s);
> +
> +    switch (s->pict_type) {
> +    case FF_I_TYPE:
> +        break; // no prediction from other frames
> +    case FF_B_TYPE:
> +        pic_param->backward_reference_picture = ff_vaapi_get_surface(&s->next_picture);
> +        // fall-through
> +    case FF_P_TYPE:
> +        pic_param->forward_reference_picture = ff_vaapi_get_surface(&s->last_picture);
> +        break;

> +    default:
> +        assert(0);
> +        return -1;

this is ugly and wrong
pict_type can at least be that dc only mpeg1 thing too IIRC


> +    }
> +
> +    /* Fill in VAIQMatrixBufferMPEG2 */
> +    iq_matrix = &vactx->iq_matrix.mpeg2;
> +    iq_matrix->load_intra_quantiser_matrix              = 1;
> +    iq_matrix->load_non_intra_quantiser_matrix          = 1;
> +    iq_matrix->load_chroma_intra_quantiser_matrix       = 1;
> +    iq_matrix->load_chroma_non_intra_quantiser_matrix   = 1;
> +    vactx->iq_matrix_present                            = 1;
> +

> +    for (i = 0; i < 64; i++)
> +        m[i] = s->dsp.idct_permutation[ff_zigzag_direct[i]];
> +
> +    for (i = 0; i < 64; i++) {
> +        int n = m[i];
> +        iq_matrix->intra_quantiser_matrix[i]            = s->intra_matrix[n];
> +        iq_matrix->non_intra_quantiser_matrix[i]        = s->inter_matrix[n];
> +        iq_matrix->chroma_intra_quantiser_matrix[i]     = s->chroma_intra_matrix[n];
> +        iq_matrix->chroma_non_intra_quantiser_matrix[i] = s->chroma_inter_matrix[n];
> +    }
> +    return 0;

the intermediate m array seems useless

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

Those who are too smart to engage in politics are punished by being
governed by those who are dumber. -- Plato 
-------------- 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/20090322/6a632ba5/attachment.pgp>



More information about the ffmpeg-devel mailing list