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

Michael Niedermayer michaelni
Thu Feb 19 15:53:22 CET 2009


On Thu, Feb 19, 2009 at 10:03:13AM +0100, Gwenole Beauchesne wrote:
> Hi,
>
> On Wed, 18 Feb 2009, Michael Niedermayer wrote:
>
>>>> that should work, excpt why not send
>>>> buf+offset and buf_end in end_slice, is start still needed then?
>>>
>>> Still and only for h263dec.c, it only depends on the level of ugliness
>>> you'd accept. ;-) I am all for a single decode_slice() instead of both
>>> start_slice(), end_slice() but I still haven't found a beautiful way
>>> for h263dec.c (and that actually works). If you have ideas...
>>
>> please explain me how
>> start_slice(buf)
>> <code>
>> end_slice(buf_end)
>>
>> can be much cleaner than
>> tmp=buf
>> <code>
>> slice(tmp, buf_end)
>>
>> ?
>
> See by yourself if this suits you. ;-) I hope it's OK now.
[..]

> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index 4dfd6d7..eba9fb9 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


> @@ -2360,6 +2367,64 @@ typedef struct AVCodec {
>  } AVCodec;
>  
>  /**
> + * AVHWAccel.
> + */
> +typedef struct AVHWAccel {
> +    /**
> +     * Name of the hardware accelerated codec.
> +     * The name is globally unique among encoders and among decoders (but an
> +     * encoder and a decoder can share the same name).
> +     */
> +    const char *name;
> +    enum CodecType type;
> +    enum CodecID id;
> +    enum PixelFormat pix_fmt;
> +    /**
> +     * Hardware accelerated codec capabilities.
> +     * see FF_HWACCEL_CODEC_CAP_*
> +     */
> +    int capabilities;
> +    struct AVHWAccel *next;
> +    /**
> +     * 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);
> +    /**
> +     * Callback for each slice.
> +     *
> +     * Meaningful slice information (codec specific) is guaranteed to
> +     * be parsed at this point. This function is mandatory.
> +     *
> +     * @param avctx the codec context
> +     * @param buf the slice data buffer base
> +     * @param buf_size the size of the slice in bytes
> +     * @return zero if successful, a negative value otherwise
> +     */
> +    int (*decode_slice)(AVCodecContext *avctx, const uint8_t *buf, uint32_t buf_size);
> +    /**
> +     * Called at the end of each frame or field picture.
> +     *
> +     * The whole picture is parsed at this point and can now be sent
> +     * to the hardware accelerator. This function is mandatory.
> +     *
> +     * @param avctx the codec context
> +     * @return zero if successful, a negative value otherwise
> +     */
> +    int (*end_frame)(AVCodecContext *avctx);
> +} AVHWAccel;

an empty line after each field could help readability


> +
> +/**
>   * four components are given, that's all.
>   * the last component is alpha
>   */


> @@ -3158,4 +3223,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..88c2ee7 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


[...]
> @@ -327,6 +339,8 @@ int ff_h263_decode_frame(AVCodecContext *avctx,
>      MpegEncContext *s = avctx->priv_data;
>      int ret;
>      AVFrame *pict = data;
> +    const uint8_t *slice_start;

> +    int slice_size;

this declaration can be moved into the if() where it is used


>  
>  #ifdef PRINT_FRAME_TIME
>  uint64_t time= rdtsc();

> @@ -612,6 +626,11 @@ retry:
>      if(MPV_frame_start(s, avctx) < 0)
>          return -1;
>  
> +    if (avctx->hwaccel) {
> +        if (avctx->hwaccel->start_frame(avctx, s->gb.buffer, s->gb.size_in_bits/8) < 0)
> +            return -1;
> +    }
> +
>  #ifdef DEBUG
>      av_log(avctx, AV_LOG_DEBUG, "qscale=%d\n", s->qscale);
>  #endif

could avctx->hwaccel->start_frame() be called from MPV_frame_start() instead?


> @@ -630,22 +649,38 @@ retry:
>      s->mb_x=0;
>      s->mb_y=0;
>  
> +    slice_start = s->gb.buffer + get_bits_count(&s->gb)/8;
>      decode_slice(s);
>      while(s->mb_y<s->mb_height){

> +        int resync_marker_pos = -1;

useless init


>          if(s->msmpeg4_version){
>              if(s->slice_height==0 || s->mb_x!=0 || (s->mb_y%s->slice_height)!=0 || get_bits_count(&s->gb) > s->gb.size_in_bits)
>                  break;
>          }else{
> -            if(ff_h263_resync(s)<0)
> +            if ((resync_marker_pos = ff_h263_resync(s)) < 0)
>                  break;
>          }
>  
>          if(s->msmpeg4_version<4 && s->h263_pred)
>              ff_mpeg4_clean_buffers(s);
>  
> +        if (avctx->hwaccel) {

> +            assert(resync_marker_pos >= 0);

also useless


[...]
> @@ -683,6 +718,11 @@ retry:
>  intrax8_decoded:
>      ff_er_frame_end(s);
>  
> +    if (avctx->hwaccel) {
> +        if (avctx->hwaccel->end_frame(avctx) < 0)
> +            return -1;
> +    }
> +
>      MPV_frame_end(s);
>  
>  assert(s->current_picture.pict_type == s->current_picture_ptr->pict_type);

could avctx->hwaccel->end_frame() be called from MPV_frame_end() instead?


> diff --git a/libavcodec/h264.c b/libavcodec/h264.c
> index 29493a3..f229634 100644
> --- a/libavcodec/h264.c
> +++ b/libavcodec/h264.c
> @@ -25,6 +25,7 @@
>   * @author Michael Niedermayer <michaelni at gmx.at>
>   */
>  
> +#include "internal.h"
>  #include "dsputil.h"
>  #include "avcodec.h"
>  #include "mpegvideo.h"

hunk ok


[...]
> diff --git a/libavcodec/internal.h b/libavcodec/internal.h
> index a8bed35..e78ff59 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 409661e..63361a3 100644
> --- a/libavcodec/mpegvideo.c
> +++ b/libavcodec/mpegvideo.c
> @@ -950,7 +950,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

hunk ok


> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> index 215029d..31636c7 100644
> --- a/libavcodec/utils.c
> +++ b/libavcodec/utils.c
> @@ -1120,3 +1120,57 @@ void ff_log_ask_for_sample(void *avc, const char *msg)
>              "of this file to ftp://upload.ffmpeg.org/MPlayer/incoming/ "
>              "and contact the ffmpeg-devel mailing list.\n");
>  }
> +
> +static AVHWAccel *first_hwaccel = NULL;
> +
> +void av_register_hwaccel(AVHWAccel *hwaccel)
> +{

> +    AVHWAccel **p;
> +    p = &first_hwaccel;

can be merged


> +    while (*p != NULL)

superflous != NULL


[...]
> +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.


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

Frequently ignored awnser#1 FFmpeg bugs should be sent to our bugtracker. User
questions about the command line tools should be sent to the ffmpeg-user ML.
And questions about how to use libav* should be sent to the libav-user ML.
-------------- 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/20090219/9e4970d3/attachment.pgp>



More information about the ffmpeg-devel mailing list