[FFmpeg-devel] [PATCH] AVHWAccel infrastructure v2 (take 4)

[Michael Niedermayer]

On Mon, Jan 04, 2010 at 04:39:02PM +0100, Gwenole Beauchesne wrote:
> Hi,
>
> On Mon, 4 Jan 2010, Michael Niedermayer wrote:
>
>>> In the new model I wanted to use, hwaccel_context was to be solely
>>> maintained by libavcodec. However, it still needed a way to get some
>>> hwaccel context initialized by the user-application. e.g. a VADisplay 
>>> (void
>>> *), an LPDIRECTXVIDEODECODER, etc.
>>>
>>> I felt interesting to pass this information through hwaccel attrs along
>>> with other int attributes.
>>
>> We have existing means to pass information, that is through AVCodecContext
>> and using AVOption where appropriate
>> As well as using AVFrame instead of AVCodecContext where its per frame
>> data
>
> OK, I nuked the hwaccel attrs and replaced it with 
> AVCodecContext.hwaccel_{id,flags}. hwaccel_flags is handled by AVOption. I 
> left hwaccel_id as is to match existing practise (pix_fmt et al.).
>
> I haven't tested with MPlayer yet but this new approach still works with my 
> modified ffplay.
>
> So, HW accelerator selection works as is:
> - Set AVCodecContext.hwaccel_id to the desired accelerator
> - Set AVCodecContext.hwaccel_flags to HWACCEL_CAP_GET_PIXELS if the user 
> wants AVFrame.data[0-2] populated with YV12 or NV12 pixels
>
> This means we can handle multiple accelerators automatically at once. 
> Anyway, this lived in an ideal world without practical use since the 
> user-application would still have to handle the HW accelerator 
> specifically.
>
> Regards,
> Gwenole.

]...]
> @@ -2497,7 +2498,7 @@ typedef struct AVCodecContext {
>       * provided by the user. In that case, this holds display-dependent
>       * data FFmpeg cannot instantiate itself. Please refer to the
>       * FFmpeg HW accelerator documentation to know how to fill this
> -     * is. e.g. for VA API, this is a struct vaapi_context.
> +     * is. e.g. for VA API, this is a VADisplay.
>       * - encoding: unused
>       * - decoding: Set by user
>       */

hunk ok probably

[...]
> +    /**
> +     * Called in codec initialization.
> +     *
> +     * This is used to initialize the AVCodecContext.hwaccel_context
> +     * hardware accelerator context.
> +     *
> +     * @param avctx the codec context
> +     * @return zero if successful, a negative value otherwise
> +     */
> +    int (*init)(AVCodecContext *avctx);
> +
> +    /**
> +     * Called in codec finalization.
> +     *
> +     * This is used to clean-up any data from the hardware accelerator
> +     * context. Should this function be implemented, it shall reset
> +     * AVCodecContext.hwaccel_context to NULL.
> +     *
> +     * @param avctx the codec context
> +     * @return zero if successful, a negative value otherwise
> +     */
> +    int (*close)(AVCodecContext *avctx);
> +
> +    /**
> +     * Called at the beginning of each frame to allocate a HW surface.
> +     *
> +     * The returned surface will be stored in Picture.data[3].
> +     *
> +     * @param avctx the codec context
> +     * @param surface the pointer to the HW accelerator surface
> +     * @return zero if successful, a negative value otherwise
> +     */
> +    int (*alloc_surface)(AVCodecContext *avctx, uint8_t **surface);
> +
> +    /**
> +     * Called to free the HW surface that was allocated with alloc_surface().
> +     *
> +     * @param avctx the codec context
> +     * @param surface the HW accelerator surface
> +     * @return zero if successful, a negative value otherwise
> +     */
> +    void (*free_surface)(AVCodecContext *avctx, uint8_t *surface);
>  } AVHWAccel;

iam not in favor of these. What are these good for?
Why are they needed?


>  
>  /**

> diff --git a/libavcodec/hwaccel.h b/libavcodec/hwaccel.h
> new file mode 100644
> index 0000000..d40186d
> --- /dev/null
> +++ b/libavcodec/hwaccel.h
> @@ -0,0 +1,48 @@
> +/*
> + * Hardware Acceleration API
> + *
> + * Copyright (C) 2008-2009 Splitted-Desktop Systems
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#ifndef AVCODEC_HWACCEL_H
> +#define AVCODEC_HWACCEL_H
> +
> +/** Identifies the hardware accelerator */
> +enum HWAccelID {
> +    HWACCEL_ID_NONE,
> +    HWACCEL_ID_VAAPI,           ///< Video Acceleration (VA) API
> +    HWACCEL_ID_NB
> +};

this needs a any or auto for autodetection (which should be 0)
also HWACCEL_ID_NB needs a comment that it is not part of the public
API/ABI or we can not add another hwaccel
except that iam ok with this & adding hwaccel_id to AVCodecContext if people
from the player side have no objections


> +
> +/** Identifies the hardware accelerator entry-point */
> +enum HWAccelEntrypoint {
> +    HWACCEL_ENTRYPOINT_VLD = 1, ///< Variable-length decoding
> +    HWACCEL_ENTRYPOINT_IDCT,    ///< Inverse discrete cosine transform
> +    HWACCEL_ENTRYPOINT_MOCO,    ///< Motion compensation
> +    HWACCEL_ENTRYPOINT_NB
> +};

as said previously this is ambigous

HWACCEL_ENTRYPOINT_VLD could mean the hw does just VLD and leaves IDCT/MC
to software

i think it would be better to completely list the steps that are
accelerated, maybe even as flags like:
HWACCEL_BITSTREAM   1
HWACCEL_IDCT        2
HWACCEL_MC          4
HWACCEL_LOOPFILTR   8


> +
> +/**
> + * The hardware accelerator can fetch the pixels from the decoded
> + * frames. If the user requested it, avcodec_default_get_buffer()
> + * will then allocate the data buffers.
> + */
> +#define HWACCEL_CAP_GET_PIXELS 0x00000001

might need a AV prefix

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

Why not whip the teacher when the pupil misbehaves? -- Diogenes of Sinope
-------------- 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/20100222/03cf53d2/attachment.pgp>