On Mon, 28 Nov 2016 16:29:32 +0000 Matteo Naccari <matteo.nacc...@bbc.co.uk> wrote:
> - This patch contains the changes to interface the Turing codec > (http://turingcodec.org/) to ffmpeg. The patch was modified to address > the comments in the review as follows: > - Added a pkg-config file to list all dependencies required by > libturing. This should address the issue pointed out by Hendrik > Leppkes on Fri 18/11/2016 > - The buffer to store the turing-params options has now a size which > depends on how many of these parameters have been passed by the user. > This should address the issue pointed out by wm4 and Moritz Barsnick > on the ffmpeg-devel reflector > --- > LICENSE.md | 1 + > configure | 5 + > libavcodec/Makefile | 1 + > libavcodec/allcodecs.c | 1 + > libavcodec/libturing.c | 268 > +++++++++++++++++++++++++++++++++++++++++++++++++ > 5 files changed, 276 insertions(+) > create mode 100644 libavcodec/libturing.c > > diff --git a/LICENSE.md b/LICENSE.md > index 640633c..86e5371 100644 > --- a/LICENSE.md > +++ b/LICENSE.md > @@ -85,6 +85,7 @@ The following libraries are under GPL: > - frei0r > - libcdio > - librubberband > +- libturing > - libvidstab > - libx264 > - libx265 > diff --git a/configure b/configure > index 6345fc2..022ffa9 100755 > --- a/configure > +++ b/configure > @@ -255,6 +255,7 @@ External library support: > --enable-libssh enable SFTP protocol via libssh [no] > --enable-libtesseract enable Tesseract, needed for ocr filter [no] > --enable-libtheora enable Theora encoding via libtheora [no] > + --enable-libturing enable H.265/HEVC encoding via libturing [no] > --enable-libtwolame enable MP2 encoding via libtwolame [no] > --enable-libv4l2 enable libv4l2/v4l-utils [no] > --enable-libvidstab enable video stabilization using vid.stab [no] > @@ -1534,6 +1535,7 @@ EXTERNAL_LIBRARY_LIST=" > libssh > libtesseract > libtheora > + libturing > libtwolame > libv4l2 > libvidstab > @@ -2831,6 +2833,7 @@ libspeex_decoder_deps="libspeex" > libspeex_encoder_deps="libspeex" > libspeex_encoder_select="audio_frame_queue" > libtheora_encoder_deps="libtheora" > +libturing_encoder_deps="libturing" > libtwolame_encoder_deps="libtwolame" > libvo_amrwbenc_encoder_deps="libvo_amrwbenc" > libvorbis_decoder_deps="libvorbis" > @@ -5096,6 +5099,7 @@ die_license_disabled gpl frei0r > die_license_disabled gpl libcdio > die_license_disabled gpl librubberband > die_license_disabled gpl libsmbclient > +die_license_disabled gpl libturing > die_license_disabled gpl libvidstab > die_license_disabled gpl libx264 > die_license_disabled gpl libx265 > @@ -5754,6 +5758,7 @@ enabled libssh && require_pkg_config libssh > libssh/sftp.h sftp_init > enabled libspeex && require_pkg_config speex speex/speex.h > speex_decoder_init -lspeex > enabled libtesseract && require_pkg_config tesseract tesseract/capi.h > TessBaseAPICreate > enabled libtheora && require libtheora theora/theoraenc.h > th_info_init -ltheoraenc -ltheoradec -logg > +enabled libturing && require_pkg_config libturing turing.h > turing_version > enabled libtwolame && require libtwolame twolame.h twolame_init > -ltwolame && > { check_lib twolame.h > twolame_encode_buffer_float32_interleaved -ltwolame || > die "ERROR: libtwolame must be installed and > version must be >= 0.3.10"; } > diff --git a/libavcodec/Makefile b/libavcodec/Makefile > index 82f7fa2..cadefdc 100644 > --- a/libavcodec/Makefile > +++ b/libavcodec/Makefile > @@ -880,6 +880,7 @@ OBJS-$(CONFIG_LIBSPEEX_DECODER) += libspeexdec.o > OBJS-$(CONFIG_LIBSPEEX_ENCODER) += libspeexenc.o > OBJS-$(CONFIG_LIBTHEORA_ENCODER) += libtheoraenc.o > OBJS-$(CONFIG_LIBTWOLAME_ENCODER) += libtwolame.o > +OBJS-$(CONFIG_LIBTURING_ENCODER) += libturing.o > OBJS-$(CONFIG_LIBVO_AMRWBENC_ENCODER) += libvo-amrwbenc.o > OBJS-$(CONFIG_LIBVORBIS_DECODER) += libvorbisdec.o > OBJS-$(CONFIG_LIBVORBIS_ENCODER) += libvorbisenc.o \ > diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c > index ada9481..0e61a4a 100644 > --- a/libavcodec/allcodecs.c > +++ b/libavcodec/allcodecs.c > @@ -610,6 +610,7 @@ void avcodec_register_all(void) > REGISTER_ENCDEC (LIBSPEEX, libspeex); > REGISTER_ENCODER(LIBTHEORA, libtheora); > REGISTER_ENCODER(LIBTWOLAME, libtwolame); > + REGISTER_ENCODER(LIBTURING, libturing); > REGISTER_ENCODER(LIBVO_AMRWBENC, libvo_amrwbenc); > REGISTER_ENCDEC (LIBVORBIS, libvorbis); > REGISTER_ENCDEC (LIBVPX_VP8, libvpx_vp8); > diff --git a/libavcodec/libturing.c b/libavcodec/libturing.c > new file mode 100644 > index 0000000..eedaae0 > --- /dev/null > +++ b/libavcodec/libturing.c > @@ -0,0 +1,268 @@ > +/* > + * libturing encoder > + * > + * Copyright (c) 2016 Turing Codec contributors > + * > + * 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 > + */ > + > +#include <turing.h> > +#include <float.h> > +#include "libavutil/internal.h" > +#include "libavutil/common.h" > +#include "libavutil/opt.h" > +#include "libavutil/pixdesc.h" > +#include "avcodec.h" > +#include "internal.h" > + > +#define MAX_LENGTH_OPTION 32 > + > +typedef struct libturingEncodeContext { > + const AVClass *class; > + turing_encoder *encoder; > + const char *options; > +} libturingEncodeContext; > + > +static av_cold int libturing_encode_close(AVCodecContext *avctx) > +{ > + libturingEncodeContext *ctx = avctx->priv_data; > + > + if (ctx->encoder) > + turing_destroy_encoder(ctx->encoder); > + > + return 0; > +} > + > +static av_cold int libturing_encode_init(AVCodecContext *avctx) > +{ > + libturingEncodeContext *ctx = avctx->priv_data; > + > + char *options; > + char* s; > + > + char const** argv; > + char const** p; > + turing_encoder_settings settings; > + int option_counter = 0; > + > + if (ctx->options) { > + AVDictionary *dict = NULL; > + AVDictionaryEntry *en = NULL; > + > + if (!av_dict_parse_string(&dict, ctx->options, "=", ":", 0)) { > + while ((en = av_dict_get(dict, "", en, AV_DICT_IGNORE_SUFFIX))) { > + int const illegal_option = > + !strcmp("input-res", en->key) || > + !strcmp("frame-rate", en->key) || > + !strcmp("f", en->key) || > + !strcmp("frames", en->key) || > + !strcmp("sar", en->key) || > + !strcmp("bit-depth", en->key) || > + !strcmp("internal-bit-depth", en->key) || > + strlen(en->key) > MAX_LENGTH_OPTION; > + if (!illegal_option) > + option_counter++; > + } > + av_dict_free(&dict); > + } > + } > + > + option_counter += 8; > + > + argv = (char const**)malloc(option_counter*sizeof(const char*)); > + options = (char*)malloc(option_counter*MAX_LENGTH_OPTION*sizeof(char)); Casting malloc return values is 100% pointless. sizeof(char) is equivalent to 1 and should be removed here. Also I think you actually wanted (MAX_LENGTH_OPTION+1) to account for the 0 termination. > + p = argv; > + s = options; > + > + *p++ = s; > + *p++ = s += 1 + sprintf(s, "turing"); > + *p++ = s += 1 + sprintf(s, "--input-res=%dx%d", avctx->width, > avctx->height); > + *p++ = s += 1 + sprintf(s, "--frame-rate=%f", > (double)avctx->time_base.den / (avctx->time_base.num * > avctx->ticks_per_frame)); > + *p++ = s += 1 + sprintf(s, "--frames=0"); I don't think using sprintf() is a very good idea. Also, in theory, spring() could still return a negative value on error. Considering all these messy and subtle issues, I think the command line appending code should be moved to a function or so. > + > + { > + int const bit_depth = > av_pix_fmt_desc_get(avctx->pix_fmt)->comp[0].depth; > + if (bit_depth != 8 && bit_depth != 10) { > + av_log(avctx, AV_LOG_ERROR, "Encoder input must be 8- or > 10-bit.\n"); > + turing_destroy_encoder(ctx->encoder); > + return AVERROR_INVALIDDATA; > + } > + *p++ = s += 1 + sprintf(s, "--bit-depth=%d", bit_depth); > + *p++ = s += 1 + sprintf(s, "--internal-bit-depth=%d", bit_depth); > + } > + > + if (avctx->sample_aspect_ratio.num > 0 && avctx->sample_aspect_ratio.den > > 0) { > + int sar_num, sar_den; > + > + av_reduce(&sar_num, &sar_den, > + avctx->sample_aspect_ratio.num, > + avctx->sample_aspect_ratio.den, 65535); > + > + *p++ = s += 1 + sprintf(s, "--sar=%d:%d", sar_num, sar_den); > + } > + > + if (ctx->options) { > + AVDictionary *dict = NULL; > + AVDictionaryEntry *en = NULL; > + > + if (!av_dict_parse_string(&dict, ctx->options, "=", ":", 0)) { > + while ((en = av_dict_get(dict, "", en, AV_DICT_IGNORE_SUFFIX))) { > + int const illegal_option = > + !strcmp("input-res", en->key) || > + !strcmp("frame-rate", en->key) || > + !strcmp("f", en->key) || > + !strcmp("frames", en->key) || > + !strcmp("sar", en->key) || > + !strcmp("bit-depth", en->key) || > + !strcmp("internal-bit-depth", en->key) || > + strlen(en->key) > MAX_LENGTH_OPTION; > + if (illegal_option) > + av_log(avctx, AV_LOG_WARNING, "%s=%s ignored.\n", > en->key, en->value); > + else { > + if (turing_check_binary_option(en->key)) > + *p++ = s += 1 + sprintf(s, "--%s", en->key); > + else > + *p++ = s += 1 + sprintf(s, "--%s=%s", en->key, > en->value); "--%s=%s" can overflow MAX_LENGTH_OPTION. Maybe I missed something, but p can still overflow argv. > + } > + } > + av_dict_free(&dict); > + } > + } > + > + *p++ = s += 1 + sprintf(s, "dummy-input-filename"); > + > + settings.argv = argv; > + settings.argc = p - argv - 1; > + > + for (int i=0; i<settings.argc; ++i) > + av_log(avctx, AV_LOG_INFO, "arg %d: %s\n", i, settings.argv[i]); > + > + ctx->encoder = turing_create_encoder(settings); > + > + if (!ctx->encoder) { > + av_log(avctx, AV_LOG_ERROR, "Failed to create libturing encoder.\n"); > + return AVERROR_INVALIDDATA; > + } > + > + if (avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER) { > + turing_bitstream const *bitstream; > + bitstream = turing_encode_headers(ctx->encoder); > + if (bitstream->size <= 0) { > + av_log(avctx, AV_LOG_ERROR, "Failed to encode headers.\n"); > + turing_destroy_encoder(ctx->encoder); > + return AVERROR_INVALIDDATA; > + } > + > + avctx->extradata_size = bitstream->size; > + > + avctx->extradata = av_malloc(avctx->extradata_size + > AV_INPUT_BUFFER_PADDING_SIZE); > + if (!avctx->extradata) { > + av_log(avctx, AV_LOG_ERROR, "Failed to allocate HEVC extradata > %d bytes\n", avctx->extradata_size); > + turing_destroy_encoder(ctx->encoder); > + return AVERROR(ENOMEM); > + } > + > + memcpy(avctx->extradata, bitstream->p, bitstream->size); > + } > + > + free(options); > + free(argv); > + > + return 0; > +} > + > +static int libturing_encode_frame(AVCodecContext *avctx, AVPacket *pkt, > const AVFrame *pic, int *got_packet) > +{ > + libturingEncodeContext *ctx = avctx->priv_data; > + turing_encoder_output const *output; > + int ret = 0; > + > + if (pic) { > + turing_picture picture; > + > + picture.image[0].p = pic->data[0]; > + picture.image[1].p = pic->data[1]; > + picture.image[2].p = pic->data[2]; > + picture.image[0].stride = pic->linesize[0]; > + picture.image[1].stride = pic->linesize[1]; > + picture.image[2].stride = pic->linesize[2]; > + picture.pts = pic->pts; > + > + output = turing_encode_picture(ctx->encoder, &picture); > + } else { > + output = turing_encode_picture(ctx->encoder, 0); > + } > + > + if (output->bitstream.size < 0) > + return AVERROR_EXTERNAL; > + > + if (output->bitstream.size ==0) > + return 0; > + > + ret = ff_alloc_packet(pkt, output->bitstream.size); > + if (ret < 0) { > + av_log(avctx, AV_LOG_ERROR, "Error getting output packet.\n"); > + return ret; > + } > + > + memcpy(pkt->data, output->bitstream.p, output->bitstream.size); > + > + pkt->pts = output->pts; > + pkt->dts = output->dts; > + if (output->keyframe) > + pkt->flags |= AV_PKT_FLAG_KEY; > + > + *got_packet = 1; > + return 0; > +} > + > +static const enum AVPixelFormat turing_csp[] = { > + AV_PIX_FMT_YUV420P10, > + AV_PIX_FMT_YUV420P, > + AV_PIX_FMT_NONE > +}; > + > +static av_cold void libturing_encode_init_csp(AVCodec *codec) > +{ > + codec->pix_fmts = turing_csp; > +} > + > +static const AVOption options[] = { > + { "turing-params", "configure additional turing encoder paremeters", > offsetof(libturingEncodeContext, options), AV_OPT_TYPE_STRING,{ 0 }, 0, 0, > AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_ENCODING_PARAM }, > + { NULL } > +}; > + > +static const AVClass class = { > + .class_name = "libturing", > + .item_name = av_default_item_name, > + .option = options, > + .version = LIBAVUTIL_VERSION_INT, > +}; > + > +AVCodec ff_libturing_encoder = { > + .name = "libturing", > + .long_name = NULL_IF_CONFIG_SMALL("libturing HEVC"), > + .type = AVMEDIA_TYPE_VIDEO, > + .id = AV_CODEC_ID_HEVC, > + .init = libturing_encode_init, > + .init_static_data = libturing_encode_init_csp, > + .encode2 = libturing_encode_frame, > + .close = libturing_encode_close, > + .priv_data_size = sizeof(libturingEncodeContext), > + .priv_class = &class, > + .capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_AUTO_THREADS, > +}; _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel