[FFmpeg-devel] [WIP] libcodec2 wrapper + de/muxer in FFmpeg

Nicolas George george at nsup.org
Thu Aug 3 14:00:26 EEST 2017


Le sextidi 16 thermidor, an CCXXV, Tomas Härdin a écrit :
> Posting this to both [ffmpeg-devel] and [Freetel-codec2] in hopes of
> maximum feedback.
> 
> I've been spending the last few days getting codec2 (http://freedv.org/
> ) hooked up in libavcodec, and a set of muxers and demuxers for both
> raw codec2 streams and the recently created .c2 format. codec2 is very
> low bitrate (3200 bit/s down to 700 bit/s) speech codec developed for
> digital voice in amateur radio, but is now finding use in other
> applications where compressing large amounts of human speech is useful
> (audiobooks or podcasts for example)
> 
> Sample: http://www.nivex.net/images/tmp/report2074.c2
> 
> With both the raw demuxer and the encoder you need to specify the
> desired mode, like this:

The encoder could default to one the two.

> Some remarks:
> 
> * I had to make the ffmpeg CLI not complain about the 700 modes, since
> it thinks encoding at below 1 kbps is a user error

It is just a warning. I am not really a fan of a special case like that,
and it would be better split into a separate patch I think.

> * I have duplicated some minor functions in libcodec2 in
> libavcodec/codec2utils.*. This avoid having to link libcodec2 just for
> remuxing, and in case someone writes a native decoder for libavcodec

The license allows it, but you neglected to give copyright attribution.

> * Not sure if codec2utils should go in libavutil, libavcodec felt good
> enough

Definitely libavcodec.

> * The demuxer tries to read up to 0.1 seconds worth of frames to speed
> things up a little without making seeking too broken. 3 frames = 12
> bytes for the 700 bit/s modes, which decode to 960 samples

I do not like the sound of that, but I will see the code.

> * The decoder is able to deal with multiple frames at a time, the
> encoder does not need to

???

> Feel free to bikeshed around whether extradata should be the entire .c2
> header or just the mode byte. It really only matters if we go with RIFF
> or ISOM mappings (.wav .avi .mov and friends), which I decided to leave
> out for now.

It matters for inclusion in any format: Matroska, Nut, etc. Is anybody
considering normalization?

> From 569a252536ea224bcd44f55f0f5102ce1aa4ec77 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Tomas=20H=C3=A4rdin?= <tjoppen at acc.umu.se>
> Date: Wed, 2 Aug 2017 22:17:19 +0200
> Subject: [PATCH] Add codec2 muxers, demuxers and en/decoder via libcodec2
> 
> ---
>  Changelog                |   2 +
>  configure                |  12 +++
>  doc/general.texi         |  13 +++
>  ffmpeg.c                 |   3 +-
>  libavcodec/Makefile      |   3 +
>  libavcodec/allcodecs.c   |   1 +
>  libavcodec/avcodec.h     |   1 +
>  libavcodec/codec2utils.c | 118 +++++++++++++++++++++++
>  libavcodec/codec2utils.h |  58 +++++++++++
>  libavcodec/codec_desc.c  |   7 ++
>  libavcodec/libcodec2.c   | 190 ++++++++++++++++++++++++++++++++++++
>  libavcodec/version.h     |   2 +-
>  libavformat/Makefile     |   4 +
>  libavformat/allformats.c |   2 +
>  libavformat/codec2.c     | 244 +++++++++++++++++++++++++++++++++++++++++++++++
>  libavformat/rawenc.c     |  14 +++
>  libavformat/utils.c      |   1 +
>  libavformat/version.h    |   2 +-
>  18 files changed, 674 insertions(+), 3 deletions(-)
>  create mode 100644 libavcodec/codec2utils.c
>  create mode 100644 libavcodec/codec2utils.h
>  create mode 100644 libavcodec/libcodec2.c
>  create mode 100644 libavformat/codec2.c
> 
> diff --git a/Changelog b/Changelog
> index 187ae7950a..e28da7dcc4 100644
> --- a/Changelog
> +++ b/Changelog
> @@ -29,6 +29,8 @@ version <next>:
>  - limiter video filter
>  - libvmaf video filter
>  - Dolby E decoder and SMPTE 337M demuxer
> +- codec2 en/decoding via libcodec2
> +- muxer/demuxer for raw codec2 files and .c2 files
>  
>  version 3.3:
>  - CrystalHD decoder moved to new decode API
> diff --git a/configure b/configure
> index 66c7b948e4..05af25cb22 100755
> --- a/configure
> +++ b/configure
> @@ -220,6 +220,7 @@ External library support:
>    --enable-libcaca         enable textual display using libcaca [no]
>    --enable-libcelt         enable CELT decoding via libcelt [no]
>    --enable-libcdio         enable audio CD grabbing with libcdio [no]
> +  --enable-libcodec2       enable codec2 en/decoding using libcodec2 [no]
>    --enable-libdc1394       enable IIDC-1394 grabbing using libdc1394
>                             and libraw1394 [no]
>    --enable-libfdk-aac      enable AAC de/encoding via libfdk-aac [no]
> @@ -1540,6 +1541,7 @@ EXTERNAL_LIBRARY_LIST="
>      libbs2b
>      libcaca
>      libcelt
> +    libcodec2
>      libdc1394
>      libflite
>      libfontconfig
> @@ -2087,6 +2089,7 @@ CONFIG_EXTRA="
>      blockdsp
>      bswapdsp
>      cabac
> +    codec2utils
>      dirac_parse
>      dvprofile
>      exif
> @@ -2863,6 +2866,10 @@ pcm_mulaw_at_encoder_select="audio_frame_queue"
>  chromaprint_muxer_deps="chromaprint"
>  h264_videotoolbox_encoder_deps="videotoolbox_encoder pthreads"
>  libcelt_decoder_deps="libcelt"

> +libcodec2_decoder_deps="libcodec2"
> +libcodec2_decoder_select="codec2utils"
> +libcodec2_encoder_deps="libcodec2"
> +libcodec2_encoder_select="codec2utils"

This and the similar hunks are not necessary, see below the comments
about the Makefile.

>  libfdk_aac_decoder_deps="libfdk_aac"
>  libfdk_aac_encoder_deps="libfdk_aac"
>  libfdk_aac_encoder_select="audio_frame_queue"
> @@ -2935,6 +2942,10 @@ avi_demuxer_select="iso_media riffdec exif"
>  avi_muxer_select="riffenc"
>  caf_demuxer_select="iso_media riffdec"
>  caf_muxer_select="iso_media"
> +codec2_demuxer_select="codec2utils"
> +codec2_muxer_select="codec2utils"
> +codec2raw_demuxer_select="codec2utils"
> +codec2raw_muxer_select="codec2utils"
>  dash_muxer_select="mp4_muxer"
>  dirac_demuxer_select="dirac_parser"
>  dts_demuxer_select="dca_parser"
> @@ -5837,6 +5848,7 @@ enabled libcelt           && require libcelt celt/celt.h celt_decode -lcelt0 &&
>                               { check_lib libcelt celt/celt.h celt_decoder_create_custom -lcelt0 ||
>                                 die "ERROR: libcelt must be installed and version must be >= 0.11.0."; }
>  enabled libcaca           && require_pkg_config caca caca.h caca_create_canvas
> +enabled libcodec2         && require libcodec2 codec2/codec2.h codec2_create -lcodec2
>  enabled libdc1394         && require_pkg_config libdc1394-2 dc1394/dc1394.h dc1394_new
>  enabled libfdk_aac        && { use_pkg_config fdk-aac "fdk-aac/aacenc_lib.h" aacEncOpen ||
>                                 { require libfdk_aac fdk-aac/aacenc_lib.h aacEncOpen -lfdk-aac &&
> diff --git a/doc/general.texi b/doc/general.texi
> index 036c8c25d4..4bcc2b2d91 100644
> --- a/doc/general.texi
> +++ b/doc/general.texi
> @@ -85,6 +85,15 @@ Go to @url{http://www.twolame.org/} and follow the
>  instructions for installing the library.
>  Then pass @code{--enable-libtwolame} to configure to enable it.
>  
> + at section libcodec2
> +
> +FFmpeg can make use of libcodec2 to codec2 encoding and decoding.
> +There is currently no native decoder, so libcodec2 must be used for decoding.
> +
> +Go to @url{http://freedv.org/}, download "Codec 2 source archive".
> +Build and install using CMake. Debian users can install the libcodec2-dev package instead.
> +Once libcodec2 is installed you can pass @code{--enable-libcodec2} to configure to enable it.
> +
>  @section libvpx
>  
>  FFmpeg can make use of the libvpx library for VP8/VP9 encoding.
> @@ -290,6 +299,8 @@ library:
>  @item BRSTM                     @tab   @tab X
>      @tab Audio format used on the Nintendo Wii.
>  @item BWF                       @tab X @tab X
> + at item codec2 (raw)              @tab X @tab X
> + at item codec2 (.c2 files)        @tab X @tab X
>  @item CRI ADX                   @tab X @tab X
>      @tab Audio-only format used in console video games.
>  @item Discworld II BMV          @tab   @tab X
> @@ -994,6 +1005,8 @@ following image formats are supported:
>      @tab Used in Bink and Smacker files in many games.
>  @item CELT                   @tab     @tab  E
>      @tab decoding supported through external library libcelt
> + at item codec2                 @tab  E  @tab  E
> +    @tab en/decoding supported through external library libcodec2
>  @item Delphine Software International CIN audio  @tab     @tab  X
>      @tab Codec used in Delphine Software International games.
>  @item Digital Speech Standard - Standard Play mode (DSS SP) @tab     @tab  X
> diff --git a/ffmpeg.c b/ffmpeg.c
> index 888d19a647..09a5b541c0 100644
> --- a/ffmpeg.c
> +++ b/ffmpeg.c
> @@ -3480,7 +3480,8 @@ static int init_output_stream(OutputStream *ost, char *error, int error_len)
>              av_buffersink_set_frame_size(ost->filter->filter,
>                                              ost->enc_ctx->frame_size);
>          assert_avoptions(ost->encoder_opts);
> -        if (ost->enc_ctx->bit_rate && ost->enc_ctx->bit_rate < 1000)
> +        if (ost->enc_ctx->bit_rate && ost->enc_ctx->bit_rate < 1000 &&
> +            ost->enc_ctx->codec_id != AV_CODEC_ID_CODEC2 /* don't complain about 700 bit/s modes */)
>              av_log(NULL, AV_LOG_WARNING, "The bitrate parameter is set too low."
>                                           " It takes bits/s as argument, not kbits/s\n");
>  
> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> index 74de41ab0f..f5531ab3f1 100644
> --- a/libavcodec/Makefile
> +++ b/libavcodec/Makefile
> @@ -59,6 +59,7 @@ OBJS-$(CONFIG_AUDIODSP)                += audiodsp.o
>  OBJS-$(CONFIG_BLOCKDSP)                += blockdsp.o
>  OBJS-$(CONFIG_BSWAPDSP)                += bswapdsp.o
>  OBJS-$(CONFIG_CABAC)                   += cabac.o

> +OBJS-$(CONFIG_CODEC2UTILS)             += codec2utils.o

You do not need a separate configuration option, you can just depend on
the actual visible option:

+OBJS-$(CONFIG_CODEC2_DEMUXER)           += codec2utils.o
+OBJS-$(CONFIG_CODEC2_MUXER)             += codec2utils.o

Having the same file several times will not cause it to be included
several times.

>  OBJS-$(CONFIG_CRYSTALHD)               += crystalhd.o
>  OBJS-$(CONFIG_DCT)                     += dct.o dct32_fixed.o dct32_float.o
>  OBJS-$(CONFIG_ERROR_RESILIENCE)        += error_resilience.o
> @@ -885,6 +886,8 @@ OBJS-$(CONFIG_ILBC_AT_ENCODER)            += audiotoolboxenc.o
>  OBJS-$(CONFIG_PCM_ALAW_AT_ENCODER)        += audiotoolboxenc.o
>  OBJS-$(CONFIG_PCM_MULAW_AT_ENCODER)       += audiotoolboxenc.o
>  OBJS-$(CONFIG_LIBCELT_DECODER)            += libcelt_dec.o

> +OBJS-$(CONFIG_LIBCODEC2_DECODER)          += libcodec2.o
> +OBJS-$(CONFIG_LIBCODEC2_ENCODER)          += libcodec2.o

+OBJS-$(CONFIG_LIBCODEC2_DECODER)          += libcodec2.o codec2utils.o
+OBJS-$(CONFIG_LIBCODEC2_ENCODER)          += libcodec2.o codec2utils.o

>  OBJS-$(CONFIG_LIBFDK_AAC_DECODER)         += libfdk-aacdec.o
>  OBJS-$(CONFIG_LIBFDK_AAC_ENCODER)         += libfdk-aacenc.o
>  OBJS-$(CONFIG_LIBGSM_DECODER)             += libgsmdec.o
> diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
> index 4712592a5f..5296fac507 100644
> --- a/libavcodec/allcodecs.c
> +++ b/libavcodec/allcodecs.c
> @@ -618,6 +618,7 @@ static void register_all(void)
>      REGISTER_DECODER(QDMC_AT,           qdmc_at);
>      REGISTER_DECODER(QDM2_AT,           qdm2_at);
>      REGISTER_DECODER(LIBCELT,           libcelt);
> +    REGISTER_ENCDEC (LIBCODEC2,         libcodec2);
>      REGISTER_ENCDEC (LIBFDK_AAC,        libfdk_aac);
>      REGISTER_ENCDEC (LIBGSM,            libgsm);
>      REGISTER_ENCDEC (LIBGSM_MS,         libgsm_ms);
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index c594993766..488eb8b1f5 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -622,6 +622,7 @@ enum AVCodecID {
>      AV_CODEC_ID_PAF_AUDIO,
>      AV_CODEC_ID_ON2AVC,
>      AV_CODEC_ID_DSS_SP,
> +    AV_CODEC_ID_CODEC2,
>  
>      AV_CODEC_ID_FFWAVESYNTH = 0x15800,
>      AV_CODEC_ID_SONIC,
> diff --git a/libavcodec/codec2utils.c b/libavcodec/codec2utils.c
> new file mode 100644
> index 0000000000..8f5012f845
> --- /dev/null
> +++ b/libavcodec/codec2utils.c
> @@ -0,0 +1,118 @@
> +/*
> + * codec2 utility functions

> + * Copyright (c) 2017 Tomas Härdin

Missing copyright attribution if the imported code is non-trivial
(better err on the side of more attribution).

> + *
> + * 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 <string.h>
> +#include "internal.h"
> +#include "libavcodec/codec2utils.h"
> +
> +//from codec2.h, repeated here to avoid a dependency

> +#define CODEC2_MODE_3200 0
> +#define CODEC2_MODE_2400 1
> +#define CODEC2_MODE_1600 2
> +#define CODEC2_MODE_1400 3
> +#define CODEC2_MODE_1300 4
> +#define CODEC2_MODE_1200 5
> +#define CODEC2_MODE_700  6
> +#define CODEC2_MODE_700B 7
> +#define CODEC2_MODE_700C 8
> +

> +int avpriv_codec2_mode_from_str(void *logctx, const char *modestr) {

Please normalize the coding style. Same below.

> +    //statically assert the size of avpriv_codec2_header
> +    //putting it here because all codec2 things depend on codec2utils
> +    switch(0) {
> +    case 0:
> +    case sizeof(avpriv_codec2_header) == 7: //if false then the compiler will complain
> +        break;
> +    }

I see how it works. This is a nice trick. Want to make it an official
macro FF_ASSERT_STATIC()?

> +
> +    if (!modestr) {
> +        av_log(logctx, AV_LOG_ERROR, "raw codec2 streams need -mode set\n");
> +        return AVERROR(EINVAL);
> +    }
> +
> +#define MATCH(x) do { if (!strcmp(modestr, #x)) { return CODEC2_MODE_##x; } } while (0)
> +    MATCH(3200);
> +    MATCH(2400);
> +    MATCH(1600);
> +    MATCH(1400);
> +    MATCH(1300);
> +    MATCH(1200);
> +    MATCH(700);
> +    MATCH(700B);
> +    MATCH(700C);
> +
> +    av_log(logctx, AV_LOG_ERROR, "invalid codec2 mode: %s\n", modestr);
> +    return AVERROR(EINVAL);
> +}
> +
> +int avpriv_codec2_mode_bit_rate(void *logctx, int mode) {

> +    int ret = 8 * 8000 * avpriv_codec2_mode_block_align(logctx, mode) / avpriv_codec2_mode_frame_size(logctx, mode);
> +    if (ret <= 0) {
> +        av_log(logctx, AV_LOG_WARNING, "unknown codec2 mode %i, can't estimate bitrate\n", mode);

I bet you did not test the invalid case. Otherwise, I think you would
have found a division by 0.

> +    }
> +    return ret;
> +}
> +
> +int avpriv_codec2_mode_frame_size(void *logctx, int mode) {
> +    switch (mode) {

> +    case CODEC2_MODE_3200: return 160;
> +    case CODEC2_MODE_2400: return 160;
> +    case CODEC2_MODE_1600: return 320;
> +    case CODEC2_MODE_1400: return 320;
> +    case CODEC2_MODE_1300: return 320;
> +    case CODEC2_MODE_1200: return 320;
> +    case CODEC2_MODE_700:  return 320;
> +    case CODEC2_MODE_700B: return 320;
> +    case CODEC2_MODE_700C: return 320;

For all these mappings, I think a table would be more elegant.

> +    default:
> +        av_log(logctx, AV_LOG_ERROR, "unknown codec2 mode %i, can't find frame_size\n", mode);
> +        return 0;
> +    }
> +}
> +
> +int avpriv_codec2_mode_block_align(void *logctx, int mode) {
> +    switch (mode) {
> +    case CODEC2_MODE_3200: return 8;
> +    case CODEC2_MODE_2400: return 6;
> +    case CODEC2_MODE_1600: return 8;
> +    case CODEC2_MODE_1400: return 7;
> +    case CODEC2_MODE_1300: return 7;
> +    case CODEC2_MODE_1200: return 6;
> +    case CODEC2_MODE_700:  return 4;
> +    case CODEC2_MODE_700B: return 4;
> +    case CODEC2_MODE_700C: return 4;
> +    default:
> +        av_log(logctx, AV_LOG_ERROR, "unknown codec2 mode %i, can't find block_align\n", mode);
> +        return 0;
> +    }
> +}
> +

> +avpriv_codec2_header avpriv_codec2_make_header(int mode) {
> +    avpriv_codec2_header header = {
> +        .magic = {0xC0, 0xDE, 0xC2},
> +        //version 0.8 as of 2017-08-02 (r3345)
> +        .version_major = 0,
> +        .version_minor = 8,
> +        .mode = mode,
> +        .flags = 0,
> +    };
> +    return header;
> +}

This would be more efficient as static inline.

> diff --git a/libavcodec/codec2utils.h b/libavcodec/codec2utils.h
> new file mode 100644
> index 0000000000..0551e88a20
> --- /dev/null
> +++ b/libavcodec/codec2utils.h
> @@ -0,0 +1,58 @@
> +/*
> + * codec2 utility functions
> + * Copyright (c) 2017 Tomas Härdin
> + *
> + * 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
> + */
> +
> +#ifndef AVCODEC_CODEC2UTILS_H
> +#define AVCODEC_CODEC2UTILS_H
> +

> +#include "internal.h"

Why?

> +
> +//Converts strings like "1400" -> 3 and so on
> +//logctx is used for av_log()
> +//Returns <0 if modestr is invalid
> +int avpriv_codec2_mode_from_str(void *logctx, const char *modestr);
> +
> +//The three following functions are here to avoid needing libavformat/codec2.c to depend on libcodec2
> +
> +//Computes bitrate from mode, with frames rounded up to the nearest octet.
> +//So 700 bit/s (28 bits/frame) becomes 800 bits/s (32 bits/frame).
> +//logctx is used for av_log()
> +//Returns <0 if mode is invalid
> +int avpriv_codec2_mode_bit_rate(void *logctx, int mode);
> +
> +//duplicates codec2_samples_per_frame()
> +int avpriv_codec2_mode_frame_size(void *logctx, int mode);
> +
> +//duplicates (codec2_bits_per_frame()+7)/8
> +int avpriv_codec2_mode_block_align(void *logctx, int mode);
> +
> +//Used as extradata

> +typedef struct {

Anonymous structures are discouraged.

> +    uint8_t magic[3];
> +    uint8_t version_major;
> +    uint8_t version_minor;
> +    uint8_t mode;
> +    uint8_t flags;

> +} avpriv_codec2_header;

Types and structures names do not need to be namespaced when they are
private. On the other hand, the case is not consistent with the rest of
the code.

> +
> +//Used in codec2raw demuxer and libcodec2 encoder to make up .c2 headers
> +avpriv_codec2_header avpriv_codec2_make_header(int mode);
> +
> +#endif /* AVCODEC_CODEC2UTILS_H */
> diff --git a/libavcodec/codec_desc.c b/libavcodec/codec_desc.c
> index 6f43b68b83..09727d7de2 100644
> --- a/libavcodec/codec_desc.c
> +++ b/libavcodec/codec_desc.c
> @@ -2657,6 +2657,13 @@ static const AVCodecDescriptor codec_descriptors[] = {
>          .props     = AV_CODEC_PROP_LOSSY,
>      },
>      {
> +        .id        = AV_CODEC_ID_CODEC2,
> +        .type      = AVMEDIA_TYPE_AUDIO,
> +        .name      = "codec2",

> +        .long_name = NULL_IF_CONFIG_SMALL("codec2"),

Not helpful.

> +        .props     = AV_CODEC_PROP_LOSSY,
> +    },
> +    {
>          .id        = AV_CODEC_ID_G723_1,
>          .type      = AVMEDIA_TYPE_AUDIO,
>          .name      = "g723_1",
> diff --git a/libavcodec/libcodec2.c b/libavcodec/libcodec2.c
> new file mode 100644
> index 0000000000..a7451cc881
> --- /dev/null
> +++ b/libavcodec/libcodec2.c
> @@ -0,0 +1,190 @@
> +/*
> + * codec2 encoder/decoder using libcodec2
> + * Copyright (c) 2017 Tomas Härdin
> + *
> + * 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 <codec2/codec2.h>
> +#include "avcodec.h"
> +#include "libavutil/opt.h"
> +#include "internal.h"
> +#include "codec2utils.h"
> +
> +typedef struct {
> +    const AVClass *class;
> +    struct CODEC2 *codec;
> +    char *mode;

> +} libcodec2_context;

Case not consistent with the rest of the code.

> +
> +static const AVOption options[] = {

> +    //not AV_OPT_FLAG_DECODING_PARAM since mode should come from the demuxer

??? If it comes from the raw demuxer, it is still a decoding param, is
it not?

> +    { "mode", "codec2 mode", offsetof(libcodec2_context, mode), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, AV_OPT_FLAG_AUDIO_PARAM | AV_OPT_FLAG_ENCODING_PARAM },

I think it would be better to make the mode option an integer and use
the CONST system to expose the valid values. Same below.

> +    { NULL },
> +};
> +

> +static const AVClass codec2_class = {
> +    .class_name = "libcodec2",
> +    .item_name  = av_default_item_name,
> +    .option     = options,
> +    .version    = LIBAVUTIL_VERSION_INT,
> +};

IIRC, you cannot use the same class for two components. Does "ffmpeg -h
full" still work?

> +
> +static av_cold int libcodec2_init_common(AVCodecContext *avctx)
> +{
> +    libcodec2_context *c2 = avctx->priv_data;
> +    int mode;
> +
> +    //take -mode if set
> +    if (c2->mode) {
> +        if ((mode = avpriv_codec2_mode_from_str(avctx, c2->mode)) < 0) {
> +            return mode;
> +        }
> +
> +        if (!(avctx->extradata = av_mallocz(sizeof(avpriv_codec2_header) + AV_INPUT_BUFFER_PADDING_SIZE))) {
> +            return AVERROR(ENOMEM);
> +        }
> +
> +        avctx->extradata_size = sizeof(avpriv_codec2_header);
> +        *((avpriv_codec2_header*)avctx->extradata) = avpriv_codec2_make_header(mode);
> +    } else {
> +        if (avctx->extradata_size != sizeof(avpriv_codec2_header)) {
> +            av_log(avctx, AV_LOG_ERROR, "must have exactly %zu bytes of extradata (got %i)\n",
> +                   sizeof(avpriv_codec2_header), avctx->extradata_size);

return AVERROR_INVALIDDATA?

> +        }
> +
> +        mode = ((avpriv_codec2_header*)avctx->extradata)->mode;
> +    }
> +
> +    if (!(c2->codec = codec2_create(mode))) {
> +        return AVERROR(ENOMEM);
> +    }
> +
> +    avctx->frame_size = codec2_samples_per_frame(c2->codec);
> +    avctx->block_align = (codec2_bits_per_frame(c2->codec) + 7) / 8;
> +    codec2_set_natural_or_gray(c2->codec, 1);
> +
> +    return 0;
> +}
> +
> +static av_cold int libcodec2_init_decoder(AVCodecContext *avctx)
> +{
> +    avctx->sample_rate      = 8000;
> +    avctx->channels         = 1;
> +    avctx->sample_fmt       = AV_SAMPLE_FMT_S16;
> +    avctx->channel_layout   = AV_CH_LAYOUT_MONO;
> +
> +    return libcodec2_init_common(avctx);
> +}
> +
> +static av_cold int libcodec2_init_encoder(AVCodecContext *avctx)
> +{
> +    //will need to be smarter once we get wideband support
> +    if (avctx->sample_rate != 8000 ||
> +        avctx->channels != 1 ||
> +        avctx->sample_fmt != AV_SAMPLE_FMT_S16) {
> +        av_log(avctx, AV_LOG_ERROR, "only 8 kHz 16-bit mono allowed\n");
> +        return AVERROR(EINVAL);
> +    }
> +
> +    return libcodec2_init_common(avctx);
> +}
> +
> +static av_cold int libcodec2_close(AVCodecContext *avctx)
> +{
> +    libcodec2_context *c2 = avctx->priv_data;
> +
> +    codec2_destroy(c2->codec);
> +    return 0;
> +}
> +
> +static int libcodec2_decode(AVCodecContext *avctx, void *data,
> +                            int *got_frame_ptr, AVPacket *pkt)
> +{
> +    libcodec2_context *c2 = avctx->priv_data;
> +    AVFrame *frame = data;
> +    int ret, nframes, i;
> +    int16_t *output;
> +
> +    nframes = pkt->size / avctx->block_align;
> +    frame->nb_samples = avctx->frame_size * nframes;
> +
> +    if ((ret = ff_get_buffer(avctx, frame, 0)) < 0) {
> +        return ret;
> +    }
> +
> +    output = (int16_t *)frame->data[0];
> +
> +    for (i = 0; i < nframes; i++) {

> +        codec2_decode(c2->codec, &output[i*avctx->frame_size], &pkt->data[i*avctx->block_align]);

I suggest:

    input  += avctx->frame_size;
    output += avctx->frame_size;

and drop the multiplication.

> +    }
> +
> +    if (nframes > 0) {
> +        *got_frame_ptr = 1;
> +    }
> +
> +    return nframes * avctx->block_align;
> +}
> +
> +static int libcodec2_encode(AVCodecContext *avctx, AVPacket *avpkt,
> +                            const AVFrame *frame, int *got_packet_ptr)
> +{
> +    libcodec2_context *c2 = avctx->priv_data;
> +    int16_t *samples = (int16_t *)frame->data[0];
> +    int ret;
> +
> +    if ((ret = ff_alloc_packet2(avctx, avpkt, avctx->block_align, 0)) < 0) {
> +        return ret;
> +    }
> +
> +    codec2_encode(c2->codec, avpkt->data, samples);
> +    *got_packet_ptr = 1;
> +
> +    return 0;
> +}
> +
> +AVCodec ff_libcodec2_decoder = {
> +    .name                   = "libcodec2",

> +    .long_name              = NULL_IF_CONFIG_SMALL("codec2 encoder/decoder using libcodec2"),

This is not an encoder.

> +    .type                   = AVMEDIA_TYPE_AUDIO,
> +    .id                     = AV_CODEC_ID_CODEC2,
> +    .priv_data_size         = sizeof(libcodec2_context),
> +    .init                   = libcodec2_init_decoder,
> +    .close                  = libcodec2_close,
> +    .decode                 = libcodec2_decode,
> +    .capabilities           = 0,
> +    .supported_samplerates  = (const int[]){ 8000, 0 },
> +    .sample_fmts            = (const enum AVSampleFormat[]) { AV_SAMPLE_FMT_S16, AV_SAMPLE_FMT_NONE },
> +    .channel_layouts        = (const uint64_t[]) { AV_CH_LAYOUT_MONO, 0 },
> +    .priv_class             = &codec2_class,
> +};
> +
> +AVCodec ff_libcodec2_encoder = {
> +    .name                   = "libcodec2",

> +    .long_name              = NULL_IF_CONFIG_SMALL("codec2 encoder/decoder using libcodec2"),

And this is not a decoder.

> +    .type                   = AVMEDIA_TYPE_AUDIO,
> +    .id                     = AV_CODEC_ID_CODEC2,
> +    .priv_data_size         = sizeof(libcodec2_context),
> +    .init                   = libcodec2_init_encoder,
> +    .close                  = libcodec2_close,
> +    .encode2                = libcodec2_encode,
> +    .capabilities           = 0,
> +    .supported_samplerates  = (const int[]){ 8000, 0 },
> +    .sample_fmts            = (const enum AVSampleFormat[]) { AV_SAMPLE_FMT_S16, AV_SAMPLE_FMT_NONE },
> +    .channel_layouts        = (const uint64_t[]) { AV_CH_LAYOUT_MONO, 0 },
> +    .priv_class             = &codec2_class,
> +};
> diff --git a/libavcodec/version.h b/libavcodec/version.h
> index 02c4f41800..7473000579 100644
> --- a/libavcodec/version.h
> +++ b/libavcodec/version.h
> @@ -28,7 +28,7 @@
>  #include "libavutil/version.h"
>  
>  #define LIBAVCODEC_VERSION_MAJOR  57
> -#define LIBAVCODEC_VERSION_MINOR 102
> +#define LIBAVCODEC_VERSION_MINOR 103
>  #define LIBAVCODEC_VERSION_MICRO 100
>  
>  #define LIBAVCODEC_VERSION_INT  AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
> diff --git a/libavformat/Makefile b/libavformat/Makefile
> index b0ef82cdd4..ff1a1412eb 100644
> --- a/libavformat/Makefile
> +++ b/libavformat/Makefile
> @@ -128,6 +128,10 @@ OBJS-$(CONFIG_CAVSVIDEO_MUXER)           += rawenc.o
>  OBJS-$(CONFIG_CDG_DEMUXER)               += cdg.o
>  OBJS-$(CONFIG_CDXL_DEMUXER)              += cdxl.o
>  OBJS-$(CONFIG_CINE_DEMUXER)              += cinedec.o
> +OBJS-$(CONFIG_CODEC2_DEMUXER)            += codec2.o rawdec.o
> +OBJS-$(CONFIG_CODEC2_MUXER)              += codec2.o rawenc.o
> +OBJS-$(CONFIG_CODEC2RAW_DEMUXER)         += codec2.o rawdec.o
> +OBJS-$(CONFIG_CODEC2RAW_MUXER)           += rawenc.o
>  OBJS-$(CONFIG_CONCAT_DEMUXER)            += concatdec.o
>  OBJS-$(CONFIG_CRC_MUXER)                 += crcenc.o
>  OBJS-$(CONFIG_DATA_DEMUXER)              += rawdec.o
> diff --git a/libavformat/allformats.c b/libavformat/allformats.c
> index 1ebc14231c..26f0f1eccd 100644
> --- a/libavformat/allformats.c
> +++ b/libavformat/allformats.c
> @@ -94,6 +94,8 @@ static void register_all(void)
>      REGISTER_DEMUXER (CDG,              cdg);
>      REGISTER_DEMUXER (CDXL,             cdxl);
>      REGISTER_DEMUXER (CINE,             cine);
> +    REGISTER_MUXDEMUX(CODEC2,           codec2);
> +    REGISTER_MUXDEMUX(CODEC2RAW,        codec2raw);
>      REGISTER_DEMUXER (CONCAT,           concat);
>      REGISTER_MUXER   (CRC,              crc);
>      REGISTER_MUXER   (DASH,             dash);
> diff --git a/libavformat/codec2.c b/libavformat/codec2.c
> new file mode 100644
> index 0000000000..ea9c22e501
> --- /dev/null
> +++ b/libavformat/codec2.c
> @@ -0,0 +1,244 @@
> +/*
> + * codec2 raw demuxer
> + * Copyright (c) 2017 Tomas Härdin
> + *
> + * 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 "libavcodec/codec2utils.h"
> +#include "avio_internal.h"
> +#include "avformat.h"
> +#include "internal.h"
> +#include "rawdec.h"
> +#include "rawenc.h"
> +#include "pcm.h"
> +
> +typedef struct {
> +    const AVClass *class;
> +    char *mode;
> +} codec2_context;
> +
> +static int codec2_probe(AVProbeData *p)
> +{
> +    if (p->buf_size < sizeof(avpriv_codec2_header)) {
> +        return 0;
> +    }
> +
> +    //file starts wih 0xC0DEC2

> +    if (p->buf[0] == 0xC0 && p->buf[1] == 0xDE && p->buf[2] == 0xC2) {
> +        return AVPROBE_SCORE_MAX;

As pointed by Carl Eugen, this is not reliable enough. The problem is
not misdetecting the version of files, the problem is detecting files
that are not codec2 as codec2. A lot of files can start with 192 222
194.

> +    }
> +
> +    return 0;
> +}
> +
> +
> +static int codec2raw_read_header_common(AVFormatContext *s, AVStream *st)
> +{
> +    int mode = ((avpriv_codec2_header*)st->codecpar->extradata)->mode;
> +
> +    //let decoder determine as many properties as possible (channels, sample format, channel layout etc)
> +    //we need to set sample_rate and bit_rate for duration and seeking to work properly
> +    st->codecpar->codec_type        = AVMEDIA_TYPE_AUDIO;
> +    st->codecpar->codec_id          = AV_CODEC_ID_CODEC2;
> +    st->codecpar->sample_rate       = 8000;
> +    st->codecpar->channels          = 1;
> +    st->codecpar->format            = AV_SAMPLE_FMT_S16;
> +    st->codecpar->channel_layout    = AV_CH_LAYOUT_MONO;
> +    st->codecpar->bit_rate          = avpriv_codec2_mode_bit_rate(s, mode);
> +    st->codecpar->frame_size        = avpriv_codec2_mode_frame_size(s, mode);
> +    st->codecpar->block_align       = avpriv_codec2_mode_block_align(s, mode);
> +
> +    avpriv_set_pts_info(st, 64, 1, st->codecpar->sample_rate);
> +

> +    //replicating estimate_timings_from_bit_rate() in utils.c to avoid warnings

Please elaborate.

> +    if (s->pb && st->codecpar->bit_rate > 0) {
> +        int64_t filesize = avio_size(s->pb);
> +        if (filesize > s->internal->data_offset) {
> +            filesize -= s->internal->data_offset;
> +            st->duration = av_rescale(8 * filesize,
> +                                      st->time_base.den,
> +                                      st->codecpar->bit_rate * (int64_t) st->time_base.num);
> +        }
> +    }
> +
> +    return 0;
> +}
> +
> +static int codec2_read_header(AVFormatContext *s)
> +{
> +    AVStream *st;
> +
> +    if (!(st = avformat_new_stream(s, NULL)) ||
> +        ff_alloc_extradata(st->codecpar, sizeof(avpriv_codec2_header))) {
> +        return AVERROR(ENOMEM);
> +    }
> +
> +    s->internal->data_offset = sizeof(avpriv_codec2_header);
> +    avio_read(s->pb, st->codecpar->extradata, sizeof(avpriv_codec2_header));
> +

> +    if (st->codecpar->extradata[0] != 0xC0 ||
> +        st->codecpar->extradata[1] != 0xDE ||
> +        st->codecpar->extradata[2] != 0xC2) {

Since you do it twice, you could make it a function.

> +        av_log(s, AV_LOG_ERROR, "not a .c2 file\n");
> +    }

I think setting the time base information is missing.

> +
> +    return codec2raw_read_header_common(s, st);
> +}
> +
> +//based off of ff_raw_read_partial_packet()
> +static int codec2_read_packet(AVFormatContext *s, AVPacket *pkt)
> +{
> +    AVStream *st = s->streams[0];
> +    int ret, size, n, block_align, frame_size;
> +
> +    block_align = st->codecpar->block_align;
> +    frame_size  = st->codecpar->frame_size;
> +

> +    //Read roughly 0.1 seconds worth of data.
> +    n = st->codecpar->bit_rate / ((int)(8/0.1) * block_align) + 1;

I do not like that block. Maybe make it an option?

> +    size = n * block_align;
> +
> +    if (av_new_packet(pkt, size) < 0)
> +        return AVERROR(ENOMEM);
> +
> +    //try to read desired number of bytes, recompute n from to actual number of bytes read
> +    pkt->pos= avio_tell(s->pb);
> +    pkt->stream_index = 0;
> +    ret = ffio_read_partial(s->pb, pkt->data, size);
> +    if (ret < 0) {
> +        av_packet_unref(pkt);
> +        return ret;
> +    }
> +    av_shrink_packet(pkt, ret);

av_get_packet()?

> +    n = ret / block_align;
> +
> +    //only set duration - compute_pkt_fields() and ff_pcm_read_seek() takes care of everything else
> +    //tested by spamming the seek functionality in ffplay
> +    pkt->duration = n * frame_size;
> +
> +    return ret;
> +}
> +
> +static int codec2_write_header(AVFormatContext *s)
> +{
> +    AVStream *st;
> +
> +    if (s->nb_streams != 1 || s->streams[0]->codecpar->codec_id != AV_CODEC_ID_CODEC2) {
> +        av_log(s, AV_LOG_ERROR, ".c2 files must have exactly one codec2 stream\n");
> +        return AVERROR(EINVAL);
> +    }
> +
> +    st = s->streams[0];
> +
> +    if (st->codecpar->extradata_size != sizeof(avpriv_codec2_header)) {

> +        av_log(s, AV_LOG_ERROR, ".c2 files require exactly %zu bytes of extradata (got %i)\n",

IIRC, the z modifier is broken on some proprietary system. I personally
do not care.

> +               sizeof(avpriv_codec2_header), st->codecpar->extradata_size);
> +        return AVERROR(EINVAL);
> +    }
> +
> +    avio_write(s->pb, st->codecpar->extradata, sizeof(avpriv_codec2_header));
> +
> +    return 0;
> +}
> +
> +static int codec2raw_read_header(AVFormatContext *s)
> +{
> +    codec2_context *c2 = s->priv_data;
> +    AVStream *st;
> +    int mode;
> +
> +    if (!(st = avformat_new_stream(s, NULL)) ||
> +        ff_alloc_extradata(st->codecpar, sizeof(avpriv_codec2_header))) {
> +        return AVERROR(ENOMEM);
> +    }
> +
> +    if ((mode = avpriv_codec2_mode_from_str(s, c2->mode)) < 0) {
> +        return mode;
> +    }
> +
> +    s->internal->data_offset = 0;
> +    *((avpriv_codec2_header*)st->codecpar->extradata) = avpriv_codec2_make_header(mode);
> +
> +    return codec2raw_read_header_common(s, st);
> +}
> +
> +static const AVOption options[] = {
> +    { "mode", "codec2 mode", offsetof(codec2_context, mode), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, AV_OPT_FLAG_DECODING_PARAM},
> +    { NULL },
> +};
> +
> +static const AVClass codec2_class = {
> +    .class_name = "codec2",
> +    .item_name  = av_default_item_name,
> +    .option     = options,
> +    .version    = LIBAVUTIL_VERSION_INT,
> +    .category   = AV_CLASS_CATEGORY_DEMUXER,
> +};
> +
> +static const AVClass codec2raw_class = {
> +    .class_name = "codec2raw",
> +    .item_name  = av_default_item_name,
> +    .option     = options,
> +    .version    = LIBAVUTIL_VERSION_INT,
> +    .category   = AV_CLASS_CATEGORY_DEMUXER,
> +};
> +
> +#if CONFIG_CODEC2_DEMUXER
> +AVInputFormat ff_codec2_demuxer = {
> +    .name           = "codec2",
> +    .long_name      = NULL_IF_CONFIG_SMALL("codec2 .c2 file"),
> +    .priv_data_size = sizeof(codec2_context),
> +    .extensions     = "c2",
> +    .read_probe     = codec2_probe,
> +    .read_header    = codec2_read_header,
> +    .read_packet    = codec2_read_packet,
> +    .read_seek      = ff_pcm_read_seek,
> +    .flags          = AVFMT_GENERIC_INDEX,
> +    .raw_codec_id   = AV_CODEC_ID_CODEC2,
> +    .priv_class     = &codec2_class,
> +};
> +#endif
> +
> +#if CONFIG_CODEC2_MUXER
> +AVOutputFormat ff_codec2_muxer = {
> +    .name           = "codec2",
> +    .long_name      = NULL_IF_CONFIG_SMALL("codec2 .c2 file"),
> +    .priv_data_size = sizeof(codec2_context),
> +    .extensions     = "c2",
> +    .audio_codec    = AV_CODEC_ID_CODEC2,
> +    .video_codec    = AV_CODEC_ID_NONE,
> +    .write_header   = codec2_write_header,
> +    .write_packet   = ff_raw_write_packet,
> +    .flags          = AVFMT_NOTIMESTAMPS,
> +    .priv_class     = &codec2_class,
> +};
> +#endif
> +
> +#if CONFIG_CODEC2RAW_DEMUXER
> +AVInputFormat ff_codec2raw_demuxer = {
> +    .name           = "codec2raw",
> +    .long_name      = NULL_IF_CONFIG_SMALL("raw codec2"),
> +    .priv_data_size = sizeof(codec2_context),
> +    .read_header    = codec2raw_read_header,
> +    .read_packet    = codec2_read_packet,
> +    .read_seek      = ff_pcm_read_seek,
> +    .flags          = AVFMT_GENERIC_INDEX,
> +    .raw_codec_id   = AV_CODEC_ID_CODEC2,
> +    .priv_class     = &codec2raw_class,
> +};
> +#endif
> diff --git a/libavformat/rawenc.c b/libavformat/rawenc.c
> index 26baa850e1..f622c02244 100644
> --- a/libavformat/rawenc.c
> +++ b/libavformat/rawenc.c
> @@ -104,6 +104,20 @@ AVOutputFormat ff_cavsvideo_muxer = {
>  };
>  #endif
>  
> +#if CONFIG_CODEC2RAW_MUXER
> +AVOutputFormat ff_codec2raw_muxer = {
> +    .name              = "codec2raw",
> +    .long_name         = NULL_IF_CONFIG_SMALL("raw codec2"),
> +    //.extensions        = "c2",
> +    .audio_codec       = AV_CODEC_ID_CODEC2,
> +    .video_codec       = AV_CODEC_ID_NONE,
> +    .write_header      = force_one_stream,
> +    .write_packet      = ff_raw_write_packet,
> +    .flags             = AVFMT_NOTIMESTAMPS,
> +};
> +#endif
> +
> +
>  #if CONFIG_DATA_MUXER
>  AVOutputFormat ff_data_muxer = {
>      .name              = "data",
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index 38d247c6cd..3558e700ac 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -898,6 +898,7 @@ static int determinable_frame_size(AVCodecContext *avctx)
>      case AV_CODEC_ID_MP1:
>      case AV_CODEC_ID_MP2:
>      case AV_CODEC_ID_MP3:
> +    case AV_CODEC_ID_CODEC2:
>          return 1;
>      }
>  
> diff --git a/libavformat/version.h b/libavformat/version.h
> index 48b81f2e48..a8cf4c158e 100644
> --- a/libavformat/version.h
> +++ b/libavformat/version.h
> @@ -32,7 +32,7 @@
>  // Major bumping may affect Ticket5467, 5421, 5451(compatibility with Chromium)
>  // Also please add any ticket numbers that you believe might be affected here
>  #define LIBAVFORMAT_VERSION_MAJOR  57
> -#define LIBAVFORMAT_VERSION_MINOR  76
> +#define LIBAVFORMAT_VERSION_MINOR  77
>  #define LIBAVFORMAT_VERSION_MICRO 100
>  
>  #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \
> -- 
> 2.13.3
> 




> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20170803/250b6fcc/attachment.sig>


More information about the ffmpeg-devel mailing list