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.
pgpEnS2HQHsXF.pgp
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
