[FFmpeg-devel] [PATCH] HWAccel infrastructure (take 5)

[Michael Niedermayer]

On Fri, Feb 20, 2009 at 09:51:37AM +0100, Gwenole Beauchesne wrote:
> Hi,
>
> On Thu, 19 Feb 2009, Michael Niedermayer wrote:
>
>>> +AVHWAccel *ff_query_hwaccel_codec(AVCodecContext *avctx, enum CodecID 
>>> codec_id)
>>> +{
>>> +    AVHWAccel *hwaccel;
>>> +    AVHWAccel *hwaccels[PIX_FMT_NB] = { NULL, };
>>> +    enum PixelFormat pix_fmts[PIX_FMT_NB + 1];
>>> +    int pix_fmt, n_pix_fmts = 0;
>>> +
>>
>>> +    /* The user shall override ::get_format() with something sensible 
>>> enough */
>>> +    if (!avctx->get_format || avctx->get_format == 
>>> avcodec_default_get_format)
>>> +        return NULL;
>>
>> avctx->get_format being NULL seems invalid, so no need to check for it
>> also i suspect it would be cleaner to make avcodec_default_get_format()
>> skip hw-accel formats.
>
> Implemented with an ff_is_hwaccel_pix_fmt() function. That's intentionally 
> internal because users are expected to handle the HW accelerated pix_fmt 
> specifically anyway. IMO, this is only useful for lavc.
>
> New patch attached with the following changes:
> * Filter out HW accelerated formats in avcodec_default_get_format().
> * Add ff_is_hwaccel_pix_fmt(() to detect hardware accelerated pix_fmts.
> * Cosmetics and documentation improvements.

[...]

> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index 62e4e47..129e15d 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -2315,6 +2315,13 @@ typedef struct AVCodecContext {
>       * - decoding: unused.
>       */
>      float rc_min_vbv_overflow_use;
> +
> +    /**
> +     * Hardware accelerator in use
> +     * - encoding: unused.
> +     * - decoding: Set by libavcodec
> +     */
> +    struct AVHWAccel *hwaccel;
>  } AVCodecContext;
>  
>  /**

hunk ok


[...]
> +    /**
> +     * Called at the beginning of each frame or field picture.
> +     *
> +     * Meaningful frame information (codec specific) is guaranteed to
> +     * be parsed at this point. This function is mandatory.
> +     *
> +     * Note that \p buf can be NULL along with \p buf_size set to 0.
> +     * Otherwise, this means the whole frame is available at this point.
> +     *
> +     * @param avctx the codec context
> +     * @param buf the frame data buffer base
> +     * @param buf_size the size of the frame in bytes
> +     * @return zero if successful, a negative value otherwise
> +     */
> +    int (*start_frame)(AVCodecContext *avctx, const uint8_t *buf, uint32_t buf_size);

dont you want the current AVFrame as well? just a question ...


[...]
> @@ -3166,4 +3254,16 @@ int av_parse_video_frame_rate(AVRational *frame_rate, const char *str);
>  #define AVERROR_NOENT       AVERROR(ENOENT)  /**< No such file or directory. */
>  #define AVERROR_PATCHWELCOME    -MKTAG('P','A','W','E') /**< Not yet implemented in FFmpeg. Patches welcome. */
>  
> +/**
> + * Registers the hardware accelerator \p hwaccel.
> + */
> +void av_register_hwaccel(AVHWAccel *hwaccel);
> +
> +/**
> + * If hwaccel is NULL, returns the first registered hardware accelerator,
> + * if hwaccel is non-NULL, returns the next registered hardware accelerator
> + * after hwaccel, or NULL if hwaccel is the last one.
> + */
> +AVHWAccel *av_hwaccel_next(AVHWAccel *hwaccel);
> +
>  #endif /* AVCODEC_AVCODEC_H */
> diff --git a/libavcodec/error_resilience.c b/libavcodec/error_resilience.c
> index a196326..c21bc70 100644
> --- a/libavcodec/error_resilience.c
> +++ b/libavcodec/error_resilience.c
> @@ -680,6 +680,7 @@ void ff_er_frame_end(MpegEncContext *s){
>      Picture *pic= s->current_picture_ptr;
>  
>      if(!s->error_recognition || s->error_count==0 || s->avctx->lowres ||
> +       s->avctx->hwaccel ||
>         s->avctx->codec->capabilities&CODEC_CAP_HWACCEL_VDPAU ||
>         s->error_count==3*s->mb_width*(s->avctx->skip_top + s->avctx->skip_bottom)) return;
>  
> diff --git a/libavcodec/h263dec.c b/libavcodec/h263dec.c
> index 4cd2ce8..1eb7f9d 100644
> --- a/libavcodec/h263dec.c
> +++ b/libavcodec/h263dec.c
> @@ -25,6 +25,7 @@
>   * H.263 decoder.
>   */
>  
> +#include "internal.h"
>  #include "avcodec.h"
>  #include "dsputil.h"
>  #include "mpegvideo.h"

3 hunks ok


[...]
> diff --git a/libavcodec/imgconvert.c b/libavcodec/imgconvert.c
> index abc253e..edcc6d9 100644
> --- a/libavcodec/imgconvert.c
> +++ b/libavcodec/imgconvert.c
> @@ -57,6 +57,7 @@ typedef struct PixFmtInfo {
>      uint8_t color_type;      /**< color type (see FF_COLOR_xxx constants) */
>      uint8_t pixel_type;      /**< pixel storage type (see FF_PIXEL_xxx constants) */
>      uint8_t is_alpha : 1;    /**< true if alpha can be specified */
> +    uint8_t is_hwaccel : 1;  /**< true if this is an HW accelerated format */
>      uint8_t x_chroma_shift;  /**< X chroma subsampling factor is 2 ^ shift */
>      uint8_t y_chroma_shift;  /**< Y chroma subsampling factor is 2 ^ shift */
>      uint8_t depth;           /**< bit depth of the color components */
> @@ -263,24 +264,31 @@ static const PixFmtInfo pix_fmt_info[PIX_FMT_NB] = {
>      },
>      [PIX_FMT_XVMC_MPEG2_MC] = {
>          .name = "xvmcmc",
> +        .is_hwaccel = 1,
>      },
>      [PIX_FMT_XVMC_MPEG2_IDCT] = {
>          .name = "xvmcidct",
> +        .is_hwaccel = 1,
>      },
>      [PIX_FMT_VDPAU_MPEG1] = {
>          .name = "vdpau_mpeg1",
> +        .is_hwaccel = 1,
>      },
>      [PIX_FMT_VDPAU_MPEG2] = {
>          .name = "vdpau_mpeg2",
> +        .is_hwaccel = 1,
>      },
>      [PIX_FMT_VDPAU_H264] = {
>          .name = "vdpau_h264",
> +        .is_hwaccel = 1,
>      },
>      [PIX_FMT_VDPAU_WMV3] = {
>          .name = "vdpau_wmv3",
> +        .is_hwaccel = 1,
>      },
>      [PIX_FMT_VDPAU_VC1] = {
>          .name = "vdpau_vc1",
> +        .is_hwaccel = 1,
>      },
>      [PIX_FMT_UYYVYY411] = {
>          .name = "uyyvyy411",

2 hunks ok


> @@ -443,6 +451,12 @@ void avcodec_pix_fmt_string (char *buf, int buf_size, int pix_fmt)
>      }
>  }
>  
> +int ff_is_hwaccel_pix_fmt(enum PixelFormat pix_fmt)
> +{
> +    return pix_fmt >= 0 && pix_fmt < PIX_FMT_NB &&
> +        pix_fmt_info[pix_fmt].is_hwaccel;
> +}
> +

the >= < checks seem useless for an internal function


>  int ff_set_systematic_pal(uint32_t pal[256], enum PixelFormat pix_fmt){
>      int i;
>  
> diff --git a/libavcodec/internal.h b/libavcodec/internal.h
> index a8bed35..e1b3b0d 100644
> --- a/libavcodec/internal.h
> +++ b/libavcodec/internal.h
> @@ -24,6 +24,9 @@
>  #ifndef AVCODEC_INTERNAL_H
>  #define AVCODEC_INTERNAL_H
>  
> +#include <stdint.h>
> +#include "avcodec.h"
> +
>  /**
>   * Logs a generic warning message about a missing feature.
>   * @param[in] avc a pointer to an arbitrary struct of which the first field is

hunk ok

[...]
> diff --git a/libavcodec/mpeg12.c b/libavcodec/mpeg12.c
> index 6cf0335..c14cd6e 100644
> --- a/libavcodec/mpeg12.c
> +++ b/libavcodec/mpeg12.c
> @@ -26,6 +26,7 @@
>   */
>  
>  //#define DEBUG
> +#include "internal.h"
>  #include "avcodec.h"
>  #include "dsputil.h"
>  #include "mpegvideo.h"

hunk ok


[...]
> diff --git a/libavcodec/mpegvideo.c b/libavcodec/mpegvideo.c
> index 2eec211..c213c82 100644
> --- a/libavcodec/mpegvideo.c
> +++ b/libavcodec/mpegvideo.c
> @@ -955,7 +955,8 @@ void MPV_frame_end(MpegEncContext *s)
>      //just to make sure that all data is rendered.
>      if(CONFIG_MPEG_XVMC_DECODER && s->avctx->xvmc_acceleration){
>          ff_xvmc_field_end(s);
> -    }else if(!(s->avctx->codec->capabilities&CODEC_CAP_HWACCEL_VDPAU)
> +    }else if(!s->avctx->hwaccel
> +       && !(s->avctx->codec->capabilities&CODEC_CAP_HWACCEL_VDPAU)
>         && s->unrestricted_mv
>         && s->current_picture.reference
>         && !s->intra_only
> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> index 215029d..90a23c3 100644
> --- a/libavcodec/utils.c
> +++ b/libavcodec/utils.c
> @@ -391,7 +391,9 @@ int avcodec_default_execute(AVCodecContext *c, int (*func)(AVCodecContext *c2, v
>      return 0;
>  }
>  
> -enum PixelFormat avcodec_default_get_format(struct AVCodecContext *s, const enum PixelFormat * fmt){
> +enum PixelFormat avcodec_default_get_format(struct AVCodecContext *s, const enum PixelFormat *fmt){
> +    while (*fmt != PIX_FMT_NONE && ff_is_hwaccel_pix_fmt(*fmt))
> +        ++fmt;
>      return fmt[0];
>  }
>  

2 hunks ok


[...]
> +AVHWAccel *ff_query_hwaccel_codec(AVCodecContext *avctx, enum CodecID codec_id)
> +{
> +    AVHWAccel *hwaccel;
> +    AVHWAccel *hwaccels[PIX_FMT_NB] = { NULL, };
> +    enum PixelFormat pix_fmts[PIX_FMT_NB + 1];
> +    int pix_fmt, n_pix_fmts = 0;
> +
> +    for (hwaccel = first_hwaccel; hwaccel; hwaccel = hwaccel->next) {
> +        if (hwaccel->id == codec_id) {
> +            hwaccels[hwaccel->pix_fmt] = hwaccel;
> +            pix_fmts[n_pix_fmts++] = hwaccel->pix_fmt;
> +        }
> +    }
> +    if (n_pix_fmts == 0)
> +        return NULL;
> +    pix_fmts[n_pix_fmts] = PIX_FMT_NONE;
> +

> +    /* The default ::get_format() filters out HW accelerated formats */
> +    if ((pix_fmt = avctx->get_format(avctx, pix_fmts)) != PIX_FMT_NONE)
> +        return hwaccels[pix_fmt];
> +    return NULL;
> +}

the PIX_FMT_NONE check seems useless using pix_fmt+1 as index seems to achive
the same

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

The educated differ from the uneducated as much as the living from the
dead. -- Aristotle 
-------------- 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/20090220/2a5405e0/attachment.pgp>