[FFmpeg-devel] [PATCH][7/8] Add VA API accelerated H.264 decoding (take 4)

[Gwenole Beauchesne]

On Mon, 9 Feb 2009, Michael Niedermayer wrote:

> ehm this is close but not exactly what i had in mind
> i more had in mind:
>
> typedef struct AVHWAccel {
>    /** (copied from AVCodec)
>     * Name of the codec implementation.
>     * The name is globally unique among encoders and among decoders (but an
>     * encoder and a decoder can share the same name).
>     * This is the primary way to find a codec from the user perspective.
>     */
>    const char *name;
>    enum CodecType type;
>    enum CodecID id;
>    int (*start_frame)(AVCodecContext *avctx);
>    int (*start_slice)(AVCodecContext *avctx, const uint8_t *buffer, unsigned size);
>    int (*decode_slice)(AVCodecContext *avctx);
>    int (*end_slice)(AVCodecContext *avctx);
>    int (*end_frame)(AVCodecContext *avctx);
>    int capabilities;
>    const enum PixelFormat *pix_fmts;       ///< array of supported pixel formats, or NULL if unknown, array is terminated by -1
>    /** (copied from AVCodec)
>     * Descriptive name for the codec, meant to be more human readable than \p name.
>     * You \e should use the NULL_IF_CONFIG_SMALL() macro to define it.
>     */
>    const char *long_name;
>
>    struct AVHWAccel *next;
> } AVHWAccel;
>
> av_register_hw_accel(struct AVHWAccel *hw_accel);
>
> and adding a AVHWAccel *hw_accel field to AVCodecContext
> and adding to codecs
> if(ctx->hw_accel)
>    ctx->hw_accel->start_frame(ctx);
>
> anyway above is just a rough draft ...

I don't see fit for name and long_name.

However, an alternative is to have an AVHWAccel with init(), uninit() 
member functions, that's the accelerator class + an AVHWAccelCodec which 
is more consistent with existing AVCodec and that would have the 
start_frame(), start_slice() et al. instead.

Then, avctx could have

- hwaccel_id: the unique identifier for the accelerator, including 
entry-point (VLD, iDCT). This is set by the user (decoder).

e.g.

enum HWAccelID {
     HWACCEL_ID_VAAPI        = 0x56410000, /* 'VA'00 */
     HWACCEL_ID_VAAPI_VLD    = (HWACCEL_ID_VAAPI|HWACCEL_ENTRY_ID_VLD),  ///< VA API acceleration (variable-length decoding)
     HWACCEL_ID_VAAPI_IDCT   = (HWACCEL_ID_VAAPI|HWACCEL_ENTRY_ID_IDCT), ///< VA API acceleration (inverse DCT)
     HWACCEL_ID_VAAPI_MOCO   = (HWACCEL_ID_VAAPI|HWACCEL_ENTRY_ID_MOCO), ///< VA API acceleration (motion compensation)
};

- hwaccel: the AVHWAccel. That one can have:
name, long_name, HWAccelID, init(), close()
This is read-only from a user POV, i.e. only set by libavcodec

- hwaccel_codec: the proposed AVHWAccelCodec minus name, long_name.
This is read-only from a user POV, i.e. only set by libavcodec

Then, avcodec_open() will find a matching accelerator for the specified 
codec and hwaccel_id set beforehand, if acceleration is requested. That 
is, for the MPlayer implementation, this tends to avoid duplication of the 
vcodecs for vaapi, vdpau et al. i.e. the ultimate goal being a simple call 
as mplayer -vo vaapi|vdpau|whatevermerged -va accelerator (va: video 
accelerator), thus a one-liner patch to existing vcodec entries (the 
"out" pixfmt or some other improvement).

>> But there is a problem with this approach. Sometimes, the size of the
>> slice can't be known beforehand. e.g. MPEG-4. Unless you accept to
>> duplicate code to move to the next resync marker or end of buffer, I don't
>> see how to elegantly handle this code.
>
>> Probably make start_slice() take a buffer base + offset into this buffer
>> (for internal optimisation => use a single whole buffer to emit) and have
>> end_slice() take the buffer_end (or size wrt. the previous call to
>> start_slice())?
>
> possible
> its you who knows better what integrates best with the V* APIs

Looking at the code again, the following is more suitable:
start_slice(avctx, buffer, offset) => buffer could be the raw frame buffer
decode_slice(avctx)
end_slice(avctx, buffer_end)

That could work that way.