> On 20 Jun 2015, at 17:35, Clément Bœsch <u...@pkh.me> wrote:
>
> 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]
Done locally.
>
> [...]
>> 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…
Thanks, I’ll add this option.
>
>> +#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())
As far I understand, there’s no need to set a special colorspace range. this
pixel format type is standard,
it’s commented like this when defined in CVPixelBuffer.h:
// Bi-Planar Component Y'CbCr 8-bit 4:2:0, video-range (luma=[16,235]
chroma=[16,240])
> - 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?
Right, since I sent the current patch I added locally this option from the CLI.
> - 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…
Added :)
>
>> + 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().
Done.
>
> [...]
>> --- 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
Minor dumped instead of micro, thanks for explanation.
>
> [...]
>> +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
I’ll give it a try.
>
> [...]
>> + 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?
Not sure, but I’ll have a second look a this part of code.
>
> [...]
>> 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”?
Removed. There’s no particular reason to add this suffix.
—
Sebastien Zwickert
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
