[FFmpeg-devel] [PATCH] AVHWAccel infrastructure v2 (take 6) [ping^3]

Michael Niedermayer michaelni
Sat Oct 30 02:01:24 CEST 2010


On Fri, Oct 29, 2010 at 02:52:41PM +0200, Gwenole Beauchesne wrote:
> Hi,
>
> Here is a new patch that applies to current SVN tree with some further  
> cosmetics (comments) added.
>
> Regards,
> Gwenole.
>
> On Mon, 20 Sep 2010, Gwenole Beauchesne wrote:
>
>> It seems I didn't reply to this one or the message got lost because I 
>> had a v5 lying around for some time. Here is a v6 anyway, that 
>> intersects with the tidsp needs.
>>
>> On Mon, 22 Feb 2010, Michael Niedermayer wrote:
>>
>>> 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?
>>
>> This is used to allocate the HW surfaces (VdpSurface, VASurface, etc.) 
>> from within FFmpeg. i.e. the AVFrame.data[3] part, hence the uint8_t * 
>> instead of the void *.
>
> Those functions are also used to help user implement  
> AVCodecContext.get_buffer() and release_buffer() if he wants to do so  
> manually.
>
> e.g. by default, AVCodecContext.get_buffer() will point to  
> avcodec_default_get_buffer(). If a user overrides that function, he will  
> lose automatic data[3] handling. So, two possible cases:
> 1) He really knows how to do so, so he allocates the HW surface himself
> 2) He can delegate that to FFmpeg that will know how to do that just fine 
> too. e.g. calling the right vaCreateSurfaces(), VdpVideoSurfaceCreate(),  
> etc.
>
>>>> +/** 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)
>>
>> I finally forgot about autodetection because it's too complex to handle 
>> at the FFmpeg level and delegated that to the player level. In 
>> particular, HWAccel context may depend on upper level info, like an X 
>> display...
>>
>>>> +
>>>> +/** 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
>>
>> Actually, I didn't find the exact meaning. But from a pipeline point of 
>> view, this would mean XXX and above. i.e. VLD and above, motion 
>> compensation and above. Flagging those would lead to more player work 
>> (well, just OR'ing more flags).
>>
>> I tried to express that in the comment.
>>
>>>> +/**
>>>> + * 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
>>
>> OK.
>
> Regards,
> Gwenole.

>  Makefile    |    2 -
>  avcodec.h   |   87 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
>  hwaccel.h   |   55 +++++++++++++++++++++++++++++++++++++
>  internal.h  |   14 +++++++++
>  mpegvideo.c |    4 ++
>  options.c   |    2 +
>  utils.c     |   52 +++++++++++++++++++++++++++++++++++
>  7 files changed, 213 insertions(+), 3 deletions(-)
> e890e0f1046043c9a28d3a64d9380146b0316139  ffmpeg.hwaccel.v2.8.patch
> commit 8277742b00c2dbe41fd3af7f25c1c903f4477b43
> Author: Gwenole Beauchesne <gbeauchesne at splitted-desktop.com>
> Date:   Fri Oct 29 06:16:55 2010 +0200
> 
>     Update AVHWAccel infrastructure to v2.
> 
> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> index 385ae02..1b64768 100644
> --- a/libavcodec/Makefile
> +++ b/libavcodec/Makefile
> @@ -3,7 +3,7 @@ include $(SUBDIR)../config.mak
>  NAME = avcodec
>  FFLIBS = avutil avcore
>  
> -HEADERS = avcodec.h avfft.h dxva2.h opt.h vaapi.h vdpau.h xvmc.h
> +HEADERS = avcodec.h avfft.h dxva2.h hwaccel.h opt.h vaapi.h vdpau.h xvmc.h
>  
>  OBJS = allcodecs.o                                                      \
>         audioconvert.o                                                   \
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index 705259e..137f485 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -29,6 +29,7 @@
>  #include <errno.h>
>  #include "libavutil/avutil.h"
>  #include "libavutil/cpu.h"
> +#include "libavcodec/hwaccel.h"
>  
>  #define LIBAVCODEC_VERSION_MAJOR 52
>  #define LIBAVCODEC_VERSION_MINOR 93
> @@ -2607,7 +2608,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
>       */

> @@ -2753,6 +2754,27 @@ typedef struct AVCodecContext {
>       * - decoding: unused
>       */
>      int slices;
> +
> +    /**
> +     * User-requested hardware accelerator. See HWACCEL_ID_xxx.
> +     * - encoding: unused
> +     * - decoding: Set by user
> +     */
> +    enum HWAccelID hwaccel_id;

could you remind me why this is needed compared to the apparently more flexible
system with pixel formats?


> +
> +    /**
> +     * Hardware accelerator configuration flags. See See AV_HWACCEL_CAP_xxx.
> +     * - encoding: unused
> +     * - decoding: Set by user
> +     */
> +    unsigned int hwaccel_flags;

> +
> +    /**
> +     * Hardware accelerator private context.
> +     * - encoding: unused
> +     * - decoding: Set by libavcodec
> +     */
> +    void *hwaccel_context_private;

This looks like it can be merged with hwaccel_context


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

Breaking DRM is a little like attempting to break through a door even
though the window is wide open and the only thing in the house is a bunch
of things you dont want and which you would get tomorrow for free anyway
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20101030/8cca0e8e/attachment.pgp>



More information about the ffmpeg-devel mailing list