[FFmpeg-devel] [PATCH] avcodec: add new Videotoolbox hwaccel.
Clément Bœsch
u at pkh.me
Sat Jun 20 17:35:25 CEST 2015
On Sat, Jun 20, 2015 at 01:33:00PM +0200, Sebastien Zwickert wrote:
> Old videtotoolbox patch rebased and updated to target the new HWAccel API.
> As VDA is a wrapper of VideoToolbox framework, the changes base vda implementation
> upon the videotoolbox implementation to factorize common part of code.
>
> ---
> Changelog | 1 +
> MAINTAINERS | 1 +
> Makefile | 5 +-
> configure | 19 +-
> ffmpeg.h | 2 +
> ffmpeg_opt.c | 5 +-
> ffmpeg_vda.c | 134 --------
> ffmpeg_videotoolbox.c | 147 +++++++++
> libavcodec/Makefile | 12 +-
> libavcodec/allcodecs.c | 5 +
> libavcodec/h263dec.c | 3 +
> libavcodec/h264_slice.c | 4 +
> libavcodec/mpeg12dec.c | 3 +
> libavcodec/vda.c | 2 +-
> libavcodec/vda_h264.c | 154 +---------
> libavcodec/vda_internal.h | 33 --
> libavcodec/vda_vt_internal.h | 57 ++++
> libavcodec/version.h | 2 +-
> libavcodec/videotoolbox.c | 705 +++++++++++++++++++++++++++++++++++++++++++
> libavcodec/videotoolbox.h | 126 ++++++++
> libavutil/pixdesc.c | 4 +
> libavutil/pixfmt.h | 2 +
> 22 files changed, 1113 insertions(+), 313 deletions(-)
> delete mode 100644 ffmpeg_vda.c
> create mode 100644 ffmpeg_videotoolbox.c
> delete mode 100644 libavcodec/vda_internal.h
> create mode 100644 libavcodec/vda_vt_internal.h
> create mode 100644 libavcodec/videotoolbox.c
> create mode 100644 libavcodec/videotoolbox.h
>
Nice, thanks a lot.
[...]
> diff --git a/configure b/configure
> index 3960b73..84aab50 100755
> --- a/configure
> +++ b/configure
> @@ -155,6 +155,7 @@ Hardware accelerators:
> --disable-vaapi disable VAAPI code [autodetect]
> --disable-vda disable VDA code [autodetect]
> --disable-vdpau disable VDPAU code [autodetect]
> + --enable-videotoolbox enable Videotoolbox code [autodetect]
>
--disable-videotoolbox disable VideoToolbox code [autodetect]
[...]
> diff --git a/ffmpeg_videotoolbox.c b/ffmpeg_videotoolbox.c
> new file mode 100644
> index 0000000..a2be112
> --- /dev/null
> +++ b/ffmpeg_videotoolbox.c
> @@ -0,0 +1,147 @@
> +/*
> + * 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
> + */
> +
It would be nice to send the patch with -M diff option that detect
renames...
> +#include "config.h"
> +
> +#include "libavcodec/avcodec.h"
> +#include "libavcodec/vda.h"
> +#include "libavcodec/videotoolbox.h"
> +#include "libavutil/imgutils.h"
> +
> +#include "ffmpeg.h"
> +
> +typedef struct VTContext {
> + AVFrame *tmp_frame;
> +} VTContext;
> +
> +static int videotoolbox_retrieve_data(AVCodecContext *s, AVFrame *frame)
> +{
> + InputStream *ist = s->opaque;
> + VTContext *vt = ist->hwaccel_ctx;
> + CVPixelBufferRef pixbuf = (CVPixelBufferRef)frame->data[3];
> + OSType pixel_format = CVPixelBufferGetPixelFormatType(pixbuf);
> + CVReturn err;
> + uint8_t *data[4] = { 0 };
> + int linesize[4] = { 0 };
> + int planes, ret, i;
> +
> + av_frame_unref(vt->tmp_frame);
> +
> + switch (pixel_format) {
> + case kCVPixelFormatType_420YpCbCr8Planar: vt->tmp_frame->format = AV_PIX_FMT_YUV420P; break;
> + case kCVPixelFormatType_422YpCbCr8: vt->tmp_frame->format = AV_PIX_FMT_UYVY422; break;
> + case kCVPixelFormatType_420YpCbCr8BiPlanarVideoRange: vt->tmp_frame->format = AV_PIX_FMT_NV12; break;
...It would detect the addition of such changes :)
Which brings me to the following questions:
- does this "VideoRange" means we need to set a special colorspace range?
(see av_frame_set_color_range())
- since there seems to exist no way of setting cv_pix_fmt_type from the
CLI and this code is CLI specific, is there really much point in this?
- if you decide to add a bunch of pix fmt here, what about adding bgra?
It's useful to avoid slow sw pixel format convert in sws sometimes...
> + default:
> + av_log(NULL, AV_LOG_ERROR,
> + "Unsupported pixel format: %u\n", pixel_format);
I know this code was already present previously, but it might make sense
to use av_get_codec_tag_string().
[...]
> --- a/libavcodec/version.h
> +++ b/libavcodec/version.h
> @@ -30,7 +30,7 @@
>
> #define LIBAVCODEC_VERSION_MAJOR 56
> #define LIBAVCODEC_VERSION_MINOR 41
> -#define LIBAVCODEC_VERSION_MICRO 100
> +#define LIBAVCODEC_VERSION_MICRO 101
>
In case of addition, we bump minor
[...]
> +CFDataRef ff_videotoolbox_avcc_extradata_create(AVCodecContext *avctx)
> +{
> + CFDataRef data = NULL;
> +
> + /* Each VCL NAL in the bistream sent to the decoder
> + * is preceded by a 4 bytes length header.
> + * Change the avcC atom header if needed, to signal headers of 4 bytes. */
> + if (avctx->extradata_size >= 4 && (avctx->extradata[4] & 0x03) != 0x03) {
> + uint8_t *rw_extradata;
> +
> + if (!(rw_extradata = av_malloc(avctx->extradata_size)))
> + return NULL;
> +
> + memcpy(rw_extradata, avctx->extradata, avctx->extradata_size);
av_memdup() maybe
[...]
> + status = CMBlockBufferCreateWithMemoryBlock(kCFAllocatorDefault,// structureAllocator
> + buffer, // memoryBlock
> + size, // blockLength
> + kCFAllocatorNull, // blockAllocator
> + NULL, // customBlockSource
> + 0, // offsetToData
> + size, // dataLength
> + 0, // flags
> + &block_buf);
> +
> + if (!status) {
> + status = CMSampleBufferCreate(kCFAllocatorDefault, // allocator
> + block_buf, // dataBuffer
> + TRUE, // dataReady
> + 0, // makeDataReadyCallback
> + 0, // makeDataReadyRefcon
> + fmt_desc, // formatDescription
> + 1, // numSamples
> + 0, // numSampleTimingEntries
> + NULL, // sampleTimingArray
> + 0, // numSampleSizeEntries
> + NULL, // sampleSizeArray
> + &sample_buf);
> + }
> +
Is there no need for more complete checks?
[...]
> diff --git a/libavutil/pixdesc.c b/libavutil/pixdesc.c
> index 32dc4b8..c68e4c7 100644
> --- a/libavutil/pixdesc.c
> +++ b/libavutil/pixdesc.c
> @@ -1632,6 +1632,10 @@ const AVPixFmtDescriptor av_pix_fmt_descriptors[AV_PIX_FMT_NB] = {
> },
> .flags = AV_PIX_FMT_FLAG_BE | AV_PIX_FMT_FLAG_ALPHA,
> },
> + [AV_PIX_FMT_VIDEOTOOLBOX_VLD] = {
> + .name = "videotoolbox_vld",
> + .flags = AV_PIX_FMT_FLAG_HWACCEL,
> + },
> [AV_PIX_FMT_GBRP] = {
> .name = "gbrp",
> .nb_components = 3,
> diff --git a/libavutil/pixfmt.h b/libavutil/pixfmt.h
> index eef6444..58264cf 100644
> --- a/libavutil/pixfmt.h
> +++ b/libavutil/pixfmt.h
> @@ -309,6 +309,8 @@ enum AVPixelFormat {
> AV_PIX_FMT_YUV440P12LE, ///< planar YUV 4:4:0,24bpp, (1 Cr & Cb sample per 1x2 Y samples), little-endian
> AV_PIX_FMT_YUV440P12BE, ///< planar YUV 4:4:0,24bpp, (1 Cr & Cb sample per 1x2 Y samples), big-endian
>
> + AV_PIX_FMT_VIDEOTOOLBOX_VLD, ///< hardware decoding through Videotoolbox
> +
Why "_VLD"?
[...]
--
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20150620/daf19895/attachment.asc>
More information about the ffmpeg-devel
mailing list