diff mbox

[FFmpeg-devel,v11] - Added Turing codec interface for ffmpeg

Message ID 1498745181-29702-1-git-send-email-saverio.blasi@bbc.co.uk
State Superseded
Headers show

Commit Message

Saverio Blasi June 29, 2017, 2:06 p.m. UTC 
- This patch contains the changes to interface the Turing codec (http://turingcodec.org/) with 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
  - As per suggestions of wm4, two functions (add_option and finalise_options) have been created. The former appends new options while the latter sets up the argv array of pointers to char* accordingly. add_option re-allocates the buffer for options using av_realloc
  - Additionally, both these functions handle the errors in case the memory wasn't allocated correctly
  - malloc|free|realloc have been substituted with their corresponding av_{malloc|free|realloc} version
  - Check on bit-depth has been removed since the ffmpeg already casts the right pix_fmt and bit depth
  - pix_fmts is now set in ff_libturing_encoder as in h264dec.c.
  - Changed usage of av_free with av_freep and fixed calls to free arrays
  - Added brackets to all if and for statements
  - Avoid repetition of code to free arrays in case of failure to initialise the libturing encoder
  - Some fixes to address the review from wm4 and Mark Thompson received on Wed 08/02/2017
  - Fixed indentation
---
 LICENSE.md             |   1 +
 configure              |   6 +
 libavcodec/Makefile    |   1 +
 libavcodec/allcodecs.c |   1 +
 libavcodec/libturing.c | 313 +++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 322 insertions(+)
 create mode 100755 libavcodec/libturing.c

Comments

wm4 June 29, 2017, 2:12 p.m. UTC | #1 
On Thu, 29 Jun 2017 14:06:21 +0000
Saverio Blasi <saverio.blasi@bbc.co.uk> wrote:

> - This patch contains the changes to interface the Turing codec (http://turingcodec.org/) with 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
>   - As per suggestions of wm4, two functions (add_option and finalise_options) have been created. The former appends new options while the latter sets up the argv array of pointers to char* accordingly. add_option re-allocates the buffer for options using av_realloc
>   - Additionally, both these functions handle the errors in case the memory wasn't allocated correctly
>   - malloc|free|realloc have been substituted with their corresponding av_{malloc|free|realloc} version
>   - Check on bit-depth has been removed since the ffmpeg already casts the right pix_fmt and bit depth
>   - pix_fmts is now set in ff_libturing_encoder as in h264dec.c.
>   - Changed usage of av_free with av_freep and fixed calls to free arrays
>   - Added brackets to all if and for statements
>   - Avoid repetition of code to free arrays in case of failure to initialise the libturing encoder
>   - Some fixes to address the review from wm4 and Mark Thompson received on Wed 08/02/2017
>   - Fixed indentation
> ---

Will apply in 3 days (Monday) unless someone objects.
Derek Buitenhuis June 29, 2017, 7:45 p.m. UTC | #2 
On 6/29/2017 3:06 PM, Saverio Blasi wrote:
> ---
>  LICENSE.md             |   1 +
>  configure              |   6 +
>  libavcodec/Makefile    |   1 +
>  libavcodec/allcodecs.c |   1 +
>  libavcodec/libturing.c | 313 +++++++++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 322 insertions(+)
>  create mode 100755 libavcodec/libturing.c

1. Missing version bump.

2. patcheck says:
    Possible security issue, make sure this is safe or use snprintf/av_strl*
    patcheck.stdout:186:+    strcpy(option_ctx->s, current_option);

3. libturing git HEAD won't even build with this patch, because it's broken:

    END /tmp/ffconf.VbgfCWVe/test.c
    gcc -D_ISOC99_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_POSIX_C_SOURCE=200112 -D_XOPEN_SOURCE=600 -std=c11 -fomit-frame-pointer -pthread -I/usr/local/include -L/usr/local/lib -L/usr/local/lib/boost -c -o /tmp/ffconf.VbgfCWVe/test.o /tmp/ffconf.VbgfCWVe/test.c
    In file included from /tmp/ffconf.VbgfCWVe/test.c:1:0:
    /usr/local/include/turing.h:87:1: error: unknown type name 'bool'
     bool turing_check_binary_option(const char *option);
     ^
    ERROR: libturing not found using pkg-config

The API apparently uses C++ bool or C99 stdbool (but doesn't include stdbool.h),
neither of which is OK in FFmpeg, AFAIK. Keep in mind that C99 bool and C++
bool are not compatible.                                                               

> +    if (option_ctx->buffer_filled + option_length + 1 > option_ctx->options_buffer_size) {
> +        if (!(option_ctx->options)) {
> +            option_ctx->options = av_malloc(option_length + 1);
> +            if (!(option_ctx->options)) {
> +                return AVERROR(ENOMEM);
> +            }
> +        } else {
> +            temp_ptr = av_realloc(option_ctx->options, option_ctx->options_buffer_size + option_length + 1);
> +            if (!(temp_ptr)) {
> +                return AVERROR(ENOMEM);
> +            }
> +            option_ctx->options = temp_ptr;
> +        }

You are not allowed to pass memory allocated with av_malloc to av_realloc. This is explicitly
stated in the documentation.

> +                    if (turing_check_binary_option(en->key)) {
> +                        snprintf(option_string, MAX_OPTION_LENGTH, "--%s", en->key);
> +                    } else {
> +                        snprintf(option_string, MAX_OPTION_LENGTH, "--%s=%s", en->key, en->value);
> +                    }
> +                    if (error_code = add_option(option_string, &encoder_options)) {
> +                        goto fail;

dict gets leaked here.
> +    } else {
> +        output = turing_encode_picture(ctx->encoder, 0);

NULL instead of 0.

- Derek
wm4 June 29, 2017, 8:09 p.m. UTC | #3 
On Thu, 29 Jun 2017 20:45:10 +0100
Derek Buitenhuis <derek.buitenhuis@gmail.com> wrote:

> Keep in mind that C99 bool and C++
> bool are not compatible.                                                               

(Actually they are.)
Derek Buitenhuis June 29, 2017, 8:26 p.m. UTC | #4 
On 6/29/2017 9:09 PM, wm4 wrote:
> On Thu, 29 Jun 2017 20:45:10 +0100
> Derek Buitenhuis <derek.buitenhuis@gmail.com> wrote:
> 
>> Keep in mind that C99 bool and C++
>> bool are not compatible.                                                               
> 
> (Actually they are.)

Source? Because, from e.g. glibc:

    /* Supporting <stdbool.h> in C++ is a GCC extension.  */

And AFAICT, there is *nothing* that guarantees C99's _Bool matches
C++'s bool in size or alignment. It's compiler and ABI dependent
and just happens to work with GCC/G++, no?

- Derek
wm4 June 29, 2017, 8:44 p.m. UTC | #5 
On Thu, 29 Jun 2017 21:26:47 +0100
Derek Buitenhuis <derek.buitenhuis@gmail.com> wrote:

> On 6/29/2017 9:09 PM, wm4 wrote:
> > On Thu, 29 Jun 2017 20:45:10 +0100
> > Derek Buitenhuis <derek.buitenhuis@gmail.com> wrote:
> >   
> >> Keep in mind that C99 bool and C++
> >> bool are not compatible.                                                                 
> > 
> > (Actually they are.)  
> 
> Source? Because, from e.g. glibc:
> 
>     /* Supporting <stdbool.h> in C++ is a GCC extension.  */
> 
> And AFAICT, there is *nothing* that guarantees C99's _Bool matches
> C++'s bool in size or alignment. It's compiler and ABI dependent
> and just happens to work with GCC/G++, no?

I meant that both "bool" types are meant to be ABI compatible. (And it
holds up even on Windows with MSVC vs. GCC.)

But yeah, this would at least require including the correct header for
whatever language is used, and C would be forced to at least C99.

(FFmpeg uses C99 though.)

Anyway, getting offtopic. A build failure seems a bit like a deal
breaker, and maybe the strcpy should be killed.
Hendrik Leppkes June 29, 2017, 10:12 p.m. UTC | #6 
Am 29.06.2017 21:45 schrieb "Derek Buitenhuis" <derek.buitenhuis@gmail.com>:

On 6/29/2017 3:06 PM, Saverio Blasi wrote:
> ---
>  LICENSE.md             |   1 +
>  configure              |   6 +
>  libavcodec/Makefile    |   1 +
>  libavcodec/allcodecs.c |   1 +
>  libavcodec/libturing.c | 313 ++++++++++++++++++++++++++++++
+++++++++++++++++++
>  5 files changed, 322 insertions(+)
>  create mode 100755 libavcodec/libturing.c

1. Missing version bump.

2. patcheck says:
    Possible security issue, make sure this is safe or use snprintf/av_strl*
    patcheck.stdout:186:+    strcpy(option_ctx->s, current_option);

3. libturing git HEAD won't even build with this patch, because it's broken:

    END /tmp/ffconf.VbgfCWVe/test.c
    gcc -D_ISOC99_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE
-D_POSIX_C_SOURCE=200112 -D_XOPEN_SOURCE=600 -std=c11 -fomit-frame-pointer
-pthread -I/usr/local/include -L/usr/local/lib -L/usr/local/lib/boost -c -o
/tmp/ffconf.VbgfCWVe/test.o /tmp/ffconf.VbgfCWVe/test.c
    In file included from /tmp/ffconf.VbgfCWVe/test.c:1:0:
    /usr/local/include/turing.h:87:1: error: unknown type name 'bool'
     bool turing_check_binary_option(const char *option);
     ^
    ERROR: libturing not found using pkg-config

The API apparently uses C++ bool or C99 stdbool (but doesn't include
stdbool.h),
neither of which is OK in FFmpeg, AFAIK. Keep in mind that C99 bool and C++
bool are not compatible.

> +    if (option_ctx->buffer_filled + option_length + 1 >
option_ctx->options_buffer_size) {
> +        if (!(option_ctx->options)) {
> +            option_ctx->options = av_malloc(option_length + 1);
> +            if (!(option_ctx->options)) {
> +                return AVERROR(ENOMEM);
> +            }
> +        } else {
> +            temp_ptr = av_realloc(option_ctx->options,
option_ctx->options_buffer_size + option_length + 1);
> +            if (!(temp_ptr)) {
> +                return AVERROR(ENOMEM);
> +            }
> +            option_ctx->options = temp_ptr;
> +        }

You are not allowed to pass memory allocated with av_malloc to av_realloc.
This is explicitly
stated in the documentation.


This requirement is no longer documented for ffmpeg as no systems are known
that actually require this.


> +                    if (turing_check_binary_option(en->key)) {
> +                        snprintf(option_string, MAX_OPTION_LENGTH,
"--%s", en->key);
> +                    } else {
> +                        snprintf(option_string, MAX_OPTION_LENGTH,
"--%s=%s", en->key, en->value);
> +                    }
> +                    if (error_code = add_option(option_string,
&encoder_options)) {
> +                        goto fail;

dict gets leaked here.
> +    } else {
> +        output = turing_encode_picture(ctx->encoder, 0);

NULL instead of 0.

- Derek
Saverio Blasi June 30, 2017, 9:34 a.m. UTC | #7 
Dear all,

>> A build failure seems a bit like a deal breaker


Sorry for this. I have just pushed a fix to the Turing repository to remove bool type from the libturing APIs. The patch builds and runs fine now in our tests.

Thanks,
Saverio
wm4 July 4, 2017, 11:18 a.m. UTC | #8 
On Thu, 29 Jun 2017 14:06:21 +0000
Saverio Blasi <saverio.blasi@bbc.co.uk> wrote:

> - This patch contains the changes to interface the Turing codec (http://turingcodec.org/) with 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
>   - As per suggestions of wm4, two functions (add_option and finalise_options) have been created. The former appends new options while the latter sets up the argv array of pointers to char* accordingly. add_option re-allocates the buffer for options using av_realloc
>   - Additionally, both these functions handle the errors in case the memory wasn't allocated correctly
>   - malloc|free|realloc have been substituted with their corresponding av_{malloc|free|realloc} version
>   - Check on bit-depth has been removed since the ffmpeg already casts the right pix_fmt and bit depth
>   - pix_fmts is now set in ff_libturing_encoder as in h264dec.c.
>   - Changed usage of av_free with av_freep and fixed calls to free arrays
>   - Added brackets to all if and for statements
>   - Avoid repetition of code to free arrays in case of failure to initialise the libturing encoder
>   - Some fixes to address the review from wm4 and Mark Thompson received on Wed 08/02/2017
>   - Fixed indentation
> ---
>  LICENSE.md             |   1 +
>  configure              |   6 +
>  libavcodec/Makefile    |   1 +
>  libavcodec/allcodecs.c |   1 +
>  libavcodec/libturing.c | 313 +++++++++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 322 insertions(+)
>  create mode 100755 libavcodec/libturing.c
> 
> diff --git a/LICENSE.md b/LICENSE.md
> index ba65b05..03787c0 100644
> --- a/LICENSE.md
> +++ b/LICENSE.md
> @@ -84,6 +84,7 @@ The following libraries are under GPL:
>  - frei0r
>  - libcdio
>  - librubberband
> +- libturing
>  - libvidstab
>  - libx264
>  - libx265
> diff --git a/configure b/configure
> index 2e1786a..0adc4da 100755
> --- a/configure
> +++ b/configure
> @@ -256,6 +256,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]
> @@ -1497,6 +1498,7 @@ EXTERNAL_LIBRARY_GPL_LIST="
>      frei0r
>      libcdio
>      librubberband
> +    libturing
>      libvidstab
>      libx264
>      libx265
> @@ -2875,6 +2877,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"
> @@ -5831,6 +5834,9 @@ 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 &&
> +                             { check_cpp_condition turing.h "TURING_API_VERSION > 1" ||
> +                             die "ERROR: libturing requires turing api version 2 or greater."; }
>  enabled libtwolame        && require libtwolame twolame.h twolame_init -ltwolame &&
>                               { check_lib libtwolame 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 44acc95..0a11a6b 100644
> --- a/libavcodec/Makefile
> +++ b/libavcodec/Makefile
> @@ -909,6 +909,7 @@ OBJS-$(CONFIG_LIBSHINE_ENCODER)           += libshine.o
>  OBJS-$(CONFIG_LIBSPEEX_DECODER)           += libspeexdec.o
>  OBJS-$(CONFIG_LIBSPEEX_ENCODER)           += libspeexenc.o
>  OBJS-$(CONFIG_LIBTHEORA_ENCODER)          += libtheoraenc.o
> +OBJS-$(CONFIG_LIBTURING_ENCODER)          += libturing.o
>  OBJS-$(CONFIG_LIBTWOLAME_ENCODER)         += libtwolame.o
>  OBJS-$(CONFIG_LIBVO_AMRWBENC_ENCODER)     += libvo-amrwbenc.o
>  OBJS-$(CONFIG_LIBVORBIS_DECODER)          += libvorbisdec.o
> diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
> index 7fcc26f..c729b8d 100644
> --- a/libavcodec/allcodecs.c
> +++ b/libavcodec/allcodecs.c
> @@ -620,6 +620,7 @@ static void register_all(void)
>      REGISTER_ENCODER(LIBSHINE,          libshine);
>      REGISTER_ENCDEC (LIBSPEEX,          libspeex);
>      REGISTER_ENCODER(LIBTHEORA,         libtheora);
> +    REGISTER_ENCODER(LIBTURING,         libturing);
>      REGISTER_ENCODER(LIBTWOLAME,        libtwolame);
>      REGISTER_ENCODER(LIBVO_AMRWBENC,    libvo_amrwbenc);
>      REGISTER_ENCDEC (LIBVORBIS,         libvorbis);
> diff --git a/libavcodec/libturing.c b/libavcodec/libturing.c
> new file mode 100755
> index 0000000..c7b2311
> --- /dev/null
> +++ b/libavcodec/libturing.c
> @@ -0,0 +1,313 @@
> +/*
> + * libturing encoder
> + *
> + * Copyright (c) 2017 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 "libavutil/internal.h"
> +#include "libavutil/common.h"
> +#include "libavutil/avstring.h"
> +#include "libavutil/opt.h"
> +#include "libavutil/pixdesc.h"
> +#include "avcodec.h"
> +#include "internal.h"
> +
> +#define MAX_OPTION_LENGTH 256
> +
> +typedef struct libturingEncodeContext {
> +    const AVClass *class;
> +    turing_encoder *encoder;
> +    const char *options;
> +} libturingEncodeContext;
> +
> +typedef struct optionContext {
> +    char **argv;
> +    char *options;
> +    char *s;
> +    int options_buffer_size;
> +    int buffer_filled;
> +    int options_added;
> +} optionContext;
> +
> +static av_cold int libturing_encode_close(AVCodecContext *avctx)
> +{
> +    libturingEncodeContext *ctx = avctx->priv_data;
> +    turing_destroy_encoder(ctx->encoder);
> +    return 0;
> +}
> +
> +static av_cold int add_option(const char *current_option, optionContext *option_ctx)
> +{
> +    int option_length = strlen(current_option);
> +    char *temp_ptr;
> +
> +
> +    if (option_ctx->buffer_filled + option_length + 1 > option_ctx->options_buffer_size) {
> +        if (!(option_ctx->options)) {
> +            option_ctx->options = av_malloc(option_length + 1);
> +            if (!(option_ctx->options)) {
> +                return AVERROR(ENOMEM);
> +            }
> +        } else {
> +            temp_ptr = av_realloc(option_ctx->options, option_ctx->options_buffer_size + option_length + 1);
> +            if (!(temp_ptr)) {
> +                return AVERROR(ENOMEM);
> +            }
> +            option_ctx->options = temp_ptr;
> +        }
> +        option_ctx->options_buffer_size += option_length + 1;
> +        option_ctx->s = option_ctx->options + option_ctx->buffer_filled;
> +    }
> +    strcpy(option_ctx->s, current_option);
> +    option_ctx->s += 1 + option_length;
> +    option_ctx->options_added++;
> +    option_ctx->buffer_filled += option_length + 1;
> +    return 0;
> +}
> +
> +static av_cold int finalise_options(optionContext *option_ctx)
> +{
> +    int option_idx = 0;
> +    if (option_ctx->options_added) {
> +        char *p;
> +        option_ctx->argv = av_malloc(option_ctx->options_added * sizeof(char*));
> +        if (!(option_ctx->argv)) {
> +            return AVERROR(ENOMEM);
> +        }
> +        p = option_ctx->options;
> +        for (option_idx=0; option_idx<option_ctx->options_added; option_idx++) {
> +            option_ctx->argv[option_idx] = p;
> +            p += strlen(p) + 1;
> +        }
> +    }
> +    return 0;
> +}
> +
> +static av_cold int libturing_encode_init(AVCodecContext *avctx)
> +{
> +    libturingEncodeContext *ctx = avctx->priv_data;
> +    const int bit_depth = av_pix_fmt_desc_get(avctx->pix_fmt)->comp[0].depth;
> +    int error_code = 0;
> +    int i = 0;
> +
> +    optionContext encoder_options = {0};
> +    turing_encoder_settings settings;
> +    char option_string[MAX_OPTION_LENGTH];
> +    double frame_rate;
> +
> +    frame_rate = (double)avctx->time_base.den / (avctx->time_base.num * avctx->ticks_per_frame);
> +
> +    encoder_options.buffer_filled = 0;
> +    encoder_options.options_added = 0;
> +    encoder_options.options_buffer_size = 0;
> +    encoder_options.options = NULL;
> +    encoder_options.s = encoder_options.options;
> +    encoder_options.argv = NULL;
> +
> +    if (error_code = add_option("turing", &encoder_options)) {
> +        goto fail;
> +    }
> +
> +    if (error_code = add_option("--frames=0", &encoder_options)) {
> +        goto fail;
> +    }
> +
> +    snprintf(option_string, MAX_OPTION_LENGTH, "--input-res=%dx%d", avctx->width, avctx->height);
> +    if (error_code = add_option(option_string, &encoder_options)) {
> +        goto fail;
> +    }
> +
> +    snprintf(option_string, MAX_OPTION_LENGTH, "--frame-rate=%f", frame_rate);
> +    if (error_code = add_option(option_string, &encoder_options)) {
> +        goto fail;
> +    }
> +
> +    snprintf(option_string, MAX_OPTION_LENGTH, "--bit-depth=%d", bit_depth);
> +    if (error_code = add_option(option_string, &encoder_options)) {
> +        goto fail;
> +    }
> +
> +    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);
> +        snprintf(option_string, MAX_OPTION_LENGTH, "--sar=%d:%d", sar_num, sar_den);
> +        if (error_code = add_option(option_string, &encoder_options)) {
> +            goto fail;
> +        }
> +    }
> +
> +    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 = av_match_name(en->key, "input-res,frame-rate,f,frames,sar,bit-depth,internal-bit-depth");
> +                if (illegal_option) {
> +                    av_log(avctx, AV_LOG_WARNING, "%s=%s ignored - this parameter is inferred from ffmpeg.\n", en->key, en->value);
> +                } else {
> +                    if (turing_check_binary_option(en->key)) {
> +                        snprintf(option_string, MAX_OPTION_LENGTH, "--%s", en->key);
> +                    } else {
> +                        snprintf(option_string, MAX_OPTION_LENGTH, "--%s=%s", en->key, en->value);
> +                    }
> +                    if (error_code = add_option(option_string, &encoder_options)) {
> +                        goto fail;
> +                    }
> +                }
> +            }
> +            av_dict_free(&dict);
> +        }
> +    }
> +
> +    if (error_code = add_option("dummy-input-filename", &encoder_options)) {
> +        goto fail;
> +    }
> +
> +    if (error_code = finalise_options(&encoder_options)) {
> +        goto fail;
> +    }
> +
> +    settings.argv = (char const**)encoder_options.argv;
> +    settings.argc = encoder_options.options_added;
> +
> +    for (i = 0; i < settings.argc; i++) {
> +        av_log(avctx, AV_LOG_VERBOSE, "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");
> +        error_code = AVERROR_INVALIDDATA;
> +        goto fail;
> +    }
> +
> +    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);
> +            error_code = AVERROR_INVALIDDATA;
> +            goto fail;
> +        }
> +
> +        avctx->extradata_size = bitstream->size;
> +
> +        avctx->extradata = av_mallocz(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);
> +            error_code =  AVERROR(ENOMEM);
> +            goto fail;
> +        }
> +
> +        memcpy(avctx->extradata, bitstream->p, bitstream->size);
> +    }
> +
> +    av_freep(&encoder_options.argv);
> +    av_freep(&encoder_options.options);
> +    return 0;
> +
> +fail:
> +    av_log(avctx, AV_LOG_ERROR, "Error while initialising the Turing codec.\n");
> +    av_freep(&encoder_options.argv);
> +    av_freep(&encoder_options.options);
> +    return error_code;
> +}
> +
> +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)) {
> +        return 0;
> +    }
> +
> +    ret = ff_alloc_packet2(avctx, pkt, output->bitstream.size, 0);
> +    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 AVOption options[] = {
> +    { "turing-params", "configure additional turing encoder parameters", offsetof(libturingEncodeContext, options), AV_OPT_TYPE_STRING,{ .str = NULL }, 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,
> +    .encode2        = libturing_encode_frame,
> +    .close          = libturing_encode_close,
> +    .priv_data_size = sizeof(libturingEncodeContext),
> +    .priv_class     = &class,
> +    .capabilities   = AV_CODEC_CAP_DELAY,
> +    .pix_fmts       = (const enum AVPixelFormat[]){AV_PIX_FMT_YUV420P10, AV_PIX_FMT_YUV420P, AV_PIX_FMT_NONE},
> +};

I noticed there is no new patch for removing the strcpy() call, the
version bump, and an adjustment to the API version that excludes bool
use in the headers.

Can you add those?
diff mbox

Patch

diff --git a/LICENSE.md b/LICENSE.md
index ba65b05..03787c0 100644
--- a/LICENSE.md
+++ b/LICENSE.md
@@ -84,6 +84,7 @@  The following libraries are under GPL:
 - frei0r
 - libcdio
 - librubberband
+- libturing
 - libvidstab
 - libx264
 - libx265
diff --git a/configure b/configure
index 2e1786a..0adc4da 100755
--- a/configure
+++ b/configure
@@ -256,6 +256,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]
@@ -1497,6 +1498,7 @@  EXTERNAL_LIBRARY_GPL_LIST="
     frei0r
     libcdio
     librubberband
+    libturing
     libvidstab
     libx264
     libx265
@@ -2875,6 +2877,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"
@@ -5831,6 +5834,9 @@  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 &&
+                             { check_cpp_condition turing.h "TURING_API_VERSION > 1" ||
+                             die "ERROR: libturing requires turing api version 2 or greater."; }
 enabled libtwolame        && require libtwolame twolame.h twolame_init -ltwolame &&
                              { check_lib libtwolame 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 44acc95..0a11a6b 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -909,6 +909,7 @@  OBJS-$(CONFIG_LIBSHINE_ENCODER)           += libshine.o
 OBJS-$(CONFIG_LIBSPEEX_DECODER)           += libspeexdec.o
 OBJS-$(CONFIG_LIBSPEEX_ENCODER)           += libspeexenc.o
 OBJS-$(CONFIG_LIBTHEORA_ENCODER)          += libtheoraenc.o
+OBJS-$(CONFIG_LIBTURING_ENCODER)          += libturing.o
 OBJS-$(CONFIG_LIBTWOLAME_ENCODER)         += libtwolame.o
 OBJS-$(CONFIG_LIBVO_AMRWBENC_ENCODER)     += libvo-amrwbenc.o
 OBJS-$(CONFIG_LIBVORBIS_DECODER)          += libvorbisdec.o
diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
index 7fcc26f..c729b8d 100644
--- a/libavcodec/allcodecs.c
+++ b/libavcodec/allcodecs.c
@@ -620,6 +620,7 @@  static void register_all(void)
     REGISTER_ENCODER(LIBSHINE,          libshine);
     REGISTER_ENCDEC (LIBSPEEX,          libspeex);
     REGISTER_ENCODER(LIBTHEORA,         libtheora);
+    REGISTER_ENCODER(LIBTURING,         libturing);
     REGISTER_ENCODER(LIBTWOLAME,        libtwolame);
     REGISTER_ENCODER(LIBVO_AMRWBENC,    libvo_amrwbenc);
     REGISTER_ENCDEC (LIBVORBIS,         libvorbis);
diff --git a/libavcodec/libturing.c b/libavcodec/libturing.c
new file mode 100755
index 0000000..c7b2311
--- /dev/null
+++ b/libavcodec/libturing.c
@@ -0,0 +1,313 @@ 
+/*
+ * libturing encoder
+ *
+ * Copyright (c) 2017 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 "libavutil/internal.h"
+#include "libavutil/common.h"
+#include "libavutil/avstring.h"
+#include "libavutil/opt.h"
+#include "libavutil/pixdesc.h"
+#include "avcodec.h"
+#include "internal.h"
+
+#define MAX_OPTION_LENGTH 256
+
+typedef struct libturingEncodeContext {
+    const AVClass *class;
+    turing_encoder *encoder;
+    const char *options;
+} libturingEncodeContext;
+
+typedef struct optionContext {
+    char **argv;
+    char *options;
+    char *s;
+    int options_buffer_size;
+    int buffer_filled;
+    int options_added;
+} optionContext;
+
+static av_cold int libturing_encode_close(AVCodecContext *avctx)
+{
+    libturingEncodeContext *ctx = avctx->priv_data;
+    turing_destroy_encoder(ctx->encoder);
+    return 0;
+}
+
+static av_cold int add_option(const char *current_option, optionContext *option_ctx)
+{
+    int option_length = strlen(current_option);
+    char *temp_ptr;
+
+
+    if (option_ctx->buffer_filled + option_length + 1 > option_ctx->options_buffer_size) {
+        if (!(option_ctx->options)) {
+            option_ctx->options = av_malloc(option_length + 1);
+            if (!(option_ctx->options)) {
+                return AVERROR(ENOMEM);
+            }
+        } else {
+            temp_ptr = av_realloc(option_ctx->options, option_ctx->options_buffer_size + option_length + 1);
+            if (!(temp_ptr)) {
+                return AVERROR(ENOMEM);
+            }
+            option_ctx->options = temp_ptr;
+        }
+        option_ctx->options_buffer_size += option_length + 1;
+        option_ctx->s = option_ctx->options + option_ctx->buffer_filled;
+    }
+    strcpy(option_ctx->s, current_option);
+    option_ctx->s += 1 + option_length;
+    option_ctx->options_added++;
+    option_ctx->buffer_filled += option_length + 1;
+    return 0;
+}
+
+static av_cold int finalise_options(optionContext *option_ctx)
+{
+    int option_idx = 0;
+    if (option_ctx->options_added) {
+        char *p;
+        option_ctx->argv = av_malloc(option_ctx->options_added * sizeof(char*));
+        if (!(option_ctx->argv)) {
+            return AVERROR(ENOMEM);
+        }
+        p = option_ctx->options;
+        for (option_idx=0; option_idx<option_ctx->options_added; option_idx++) {
+            option_ctx->argv[option_idx] = p;
+            p += strlen(p) + 1;
+        }
+    }
+    return 0;
+}
+
+static av_cold int libturing_encode_init(AVCodecContext *avctx)
+{
+    libturingEncodeContext *ctx = avctx->priv_data;
+    const int bit_depth = av_pix_fmt_desc_get(avctx->pix_fmt)->comp[0].depth;
+    int error_code = 0;
+    int i = 0;
+
+    optionContext encoder_options = {0};
+    turing_encoder_settings settings;
+    char option_string[MAX_OPTION_LENGTH];
+    double frame_rate;
+
+    frame_rate = (double)avctx->time_base.den / (avctx->time_base.num * avctx->ticks_per_frame);
+
+    encoder_options.buffer_filled = 0;
+    encoder_options.options_added = 0;
+    encoder_options.options_buffer_size = 0;
+    encoder_options.options = NULL;
+    encoder_options.s = encoder_options.options;
+    encoder_options.argv = NULL;
+
+    if (error_code = add_option("turing", &encoder_options)) {
+        goto fail;
+    }
+
+    if (error_code = add_option("--frames=0", &encoder_options)) {
+        goto fail;
+    }
+
+    snprintf(option_string, MAX_OPTION_LENGTH, "--input-res=%dx%d", avctx->width, avctx->height);
+    if (error_code = add_option(option_string, &encoder_options)) {
+        goto fail;
+    }
+
+    snprintf(option_string, MAX_OPTION_LENGTH, "--frame-rate=%f", frame_rate);
+    if (error_code = add_option(option_string, &encoder_options)) {
+        goto fail;
+    }
+
+    snprintf(option_string, MAX_OPTION_LENGTH, "--bit-depth=%d", bit_depth);
+    if (error_code = add_option(option_string, &encoder_options)) {
+        goto fail;
+    }
+
+    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);
+        snprintf(option_string, MAX_OPTION_LENGTH, "--sar=%d:%d", sar_num, sar_den);
+        if (error_code = add_option(option_string, &encoder_options)) {
+            goto fail;
+        }
+    }
+
+    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 = av_match_name(en->key, "input-res,frame-rate,f,frames,sar,bit-depth,internal-bit-depth");
+                if (illegal_option) {
+                    av_log(avctx, AV_LOG_WARNING, "%s=%s ignored - this parameter is inferred from ffmpeg.\n", en->key, en->value);
+                } else {
+                    if (turing_check_binary_option(en->key)) {
+                        snprintf(option_string, MAX_OPTION_LENGTH, "--%s", en->key);
+                    } else {
+                        snprintf(option_string, MAX_OPTION_LENGTH, "--%s=%s", en->key, en->value);
+                    }
+                    if (error_code = add_option(option_string, &encoder_options)) {
+                        goto fail;
+                    }
+                }
+            }
+            av_dict_free(&dict);
+        }
+    }
+
+    if (error_code = add_option("dummy-input-filename", &encoder_options)) {
+        goto fail;
+    }
+
+    if (error_code = finalise_options(&encoder_options)) {
+        goto fail;
+    }
+
+    settings.argv = (char const**)encoder_options.argv;
+    settings.argc = encoder_options.options_added;
+
+    for (i = 0; i < settings.argc; i++) {
+        av_log(avctx, AV_LOG_VERBOSE, "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");
+        error_code = AVERROR_INVALIDDATA;
+        goto fail;
+    }
+
+    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);
+            error_code = AVERROR_INVALIDDATA;
+            goto fail;
+        }
+
+        avctx->extradata_size = bitstream->size;
+
+        avctx->extradata = av_mallocz(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);
+            error_code =  AVERROR(ENOMEM);
+            goto fail;
+        }
+
+        memcpy(avctx->extradata, bitstream->p, bitstream->size);
+    }
+
+    av_freep(&encoder_options.argv);
+    av_freep(&encoder_options.options);
+    return 0;
+
+fail:
+    av_log(avctx, AV_LOG_ERROR, "Error while initialising the Turing codec.\n");
+    av_freep(&encoder_options.argv);
+    av_freep(&encoder_options.options);
+    return error_code;
+}
+
+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)) {
+        return 0;
+    }
+
+    ret = ff_alloc_packet2(avctx, pkt, output->bitstream.size, 0);
+    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 AVOption options[] = {
+    { "turing-params", "configure additional turing encoder parameters", offsetof(libturingEncodeContext, options), AV_OPT_TYPE_STRING,{ .str = NULL }, 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,
+    .encode2        = libturing_encode_frame,
+    .close          = libturing_encode_close,
+    .priv_data_size = sizeof(libturingEncodeContext),
+    .priv_class     = &class,
+    .capabilities   = AV_CODEC_CAP_DELAY,
+    .pix_fmts       = (const enum AVPixelFormat[]){AV_PIX_FMT_YUV420P10, AV_PIX_FMT_YUV420P, AV_PIX_FMT_NONE},
+};