[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