On Sat, Jul 20, 2013 at 12:17:18AM +0200, Diego Biurrun wrote:
> On Fri, Jul 19, 2013 at 09:22:46AM +0200, Kostya Shishkov wrote:
> > ---
> > It's good enough for the only file I care about but maybe it's useful for
> > the
> > others too.
>
> That wouldn't chance to be a movie with vampires, priests and strippers?
No chance - it's a classic Soviet comedy instead.
> What's missing?
Most of the codebooks for the different modes, hacks^Wspecial cases for some
modes (like 8 kHz 6 kpbs), maybe something else.
> > The source can benefit from heavy diegoing (because it's mostly TwinVQ
> > decoder
> > bits that were tweaked to be close to the reference decoder, or the ones
> > that
> > had to be duplicated because of the different bitstream reader).
>
> You don't know the magic uncrustify incantation?
>
> > And eternal shame on Anton for not writing this decoder.
>
> shame, shame, shame!
>
> > --- a/configure
> > +++ b/configure
> > @@ -1600,6 +1600,7 @@ lagarith_decoder_select="dsputil"
> > ljpeg_encoder_select="aandcttables mpegvideoenc"
> > loco_decoder_select="golomb"
> > mdec_decoder_select="dsputil error_resilience mpegvideo"
> > +metasound_decoder_select="mdct lsp sinewin"
>
> order
copied from twinvq_decoder_select
and judging from the other entries hardly anyone cared about the order
> > --- /dev/null
> > +++ b/libavcodec/metasound.c
> > @@ -0,0 +1,480 @@
> > +/*
> > + * VoxWare MetaSound decoder
> > + * Copyright (c) 2013 Konstantin Shishkov
> > + * based on
> > + * TwinVQ decoder
>
> nit: merge the two lines
>
> > + int bitstream_second_part = (i >=
> > tctx->bits_main_spec_change[ftype]);
>
> pointless ()
>
> > + tab0 = cb0 + tmp0*cb_len;
> > + tab1 = cb1 + tmp1*cb_len;
> > +
> > + for (j = 0; j < length; j++)
> > + out[tctx->permut[ftype][pos+j]] = sign0*tab0[j] +
> > sign1*tab1[j];
>
> spaces around operators
>
> > +/**
> > + * Sum to data a periodic peak of a given period, width and shape.
> > + *
> > + * @param period the period of the peak divised by 400.0
> > + */
> > +static void add_peak(float period, int width, const float *shape,
> > + float ppc_gain, float *speech, int len)
>
> How much sense does it make to have this as (incomplete) Doxygen comment?
> It would seem to me that file-internal stuff is best served by plain C
> comments.
it's almost all copied from TwinVQ decoder so it will improve in the next
iteration (when twinvq.c is improved).
> > + int i, j;
> > +
> > + const float *shape_end = shape + len;
> > + int center;
>
> nit: int i, j, center; ?
>
> > + } else {
> > + min_period = (int)( ratio * 0.2 * 400 + 0.5) / 400.0;
> > + max_period = (int)(6 * ratio * 0.2 * 400 + 0.5) / 400.0;
>
> maybe:
>
> min_period = (int)(ratio * 0.2 * 400 + 0.5) / 400.0;
> max_period = (int)(ratio * 0.2 * 400 * 6 + 0.5) / 400.0;
>
> > + period_range = max_period - min_period;
> > + period = min_period + period_coef * period_range / ((1 <<
> > mtab->ppc_period_bit) - 1);
>
> long line
>
> > + switch (isampf) {
> > + case 8: some_mult = 2.0; break;
> > + case 11: some_mult = 3.0; break;
> > + case 16: some_mult = 3.0; break;
> > + case 22: some_mult = ibps == 32 ? 2.0 : 4.0; break;
> > + case 44: some_mult = 8.0; break;
> > + default: some_mult = 4.0;
>
> I suggest breaking these lines.
>
> > + switch ((avctx->channels << 16) + (isampf << 8) + ibps) {
> > + case (1 << 16) + ( 8 << 8) + 8: tctx->mtab = &metasound_mode0808;
> > break;
> > + case (1 << 16) + (16 << 8) + 16: tctx->mtab = &metasound_mode1616;
> > break;
> > + case (1 << 16) + (44 << 8) + 32: tctx->mtab = &metasound_mode4432;
> > break;
> > + case (2 << 16) + (44 << 8) + 48: tctx->mtab = &metasound_mode4448s;
> > break;
>
> I suggest breaking these lines.
>
> > + default:
> > + av_log(avctx, AV_LOG_ERROR, "This version does not support %d kHz
> > - %d kbit/s/ch mode.\n", isampf, ibps);
>
> long line
>
> > +AVCodec ff_voxware_decoder = {
> > + .name = "metasound",
> > + .type = AVMEDIA_TYPE_AUDIO,
> > + .id = AV_CODEC_ID_VOXWARE,
> > + .priv_data_size = sizeof(TwinContext),
> > + .init = metasound_decode_init,
> > + .close = twinvq_decode_close,
> > + .decode = metasound_decode_frame,
> > + .capabilities = CODEC_CAP_DR1,
> > + .long_name = NULL_IF_CONFIG_SMALL("VoxWare MetaSound"),
> > + .sample_fmts = (const enum AVSampleFormat[]) { AV_SAMPLE_FMT_FLTP,
> > + AV_SAMPLE_FMT_NONE },
>
> Please move long_name after name.
>
> > diff --git a/libavcodec/metasound_cb0808.c b/libavcodec/metasound_cb0808.c
> > new file mode 100644
> > index 0000000..c6a4bb0
> > --- /dev/null
> > +++ b/libavcodec/metasound_cb0808.c
> > @@ -0,0 +1,856 @@
> > +
> > +#include <stdint.h>
> > +
> > +static const int16_t cb0808l0[] = {
> > + 164, -3637, -3563, -243, -123, -47, -87, -32,
> > + 62, 129, -2, 131, -36, -202, -197, 37,
>
> 4 spaces indentation?
next you'll mention formatting as yet another thing not present here
> > --- /dev/null
> > +++ b/libavcodec/metasound_cb1616.c
> > @@ -0,0 +1,620 @@
> > +
> > +#include <stdint.h>
> > +
> > +static const int16_t cb1616l0[] = {
> > + -15, -7707, 115, 30, -36, -27, -22, -43, 2, 5, 31, -1,
>
> ditto
>
> > --- /dev/null
> > +++ b/libavcodec/metasound_cb4432.c
> > @@ -0,0 +1,1096 @@
> > +
> > +static const int16_t cb4432l0[] = {
> > + -3764, -227, 184, -258, -1713, 122, 410, -32,
>
> same
>
> > --- /dev/null
> > +++ b/libavcodec/metasound_cb4448s.c
> > @@ -0,0 +1,706 @@
> > +
> > +static const int16_t cb4448sl0[] = {
> > + -3850, -1289, -449, -36, -1178, -1175, 705, -97, 37,
>
> ...
>
> > --- /dev/null
> > +++ b/libavcodec/metasound_data.c
> > @@ -0,0 +1,113 @@
> > +
> > +#include "metasound_data.h"
> > +
> > +#include "metasound_data_fcb.c"
> > +#include "metasound_data_lsp.c"
> > +#include "metasound_data_shape.c"
>
> Maybe add stdint.h
>
> > +static const uint16_t bark_tab_l8_512[] = {
> > + 4, 5, 4, 5, 4, 5, 5, 5, 5, 6, 6, 6, 6, 8, 7, 9, 9, 11, 11, 14, 15,
> > + 17, 20, 24, 28, 34, 41, 51, 64, 83,
> > +};
>
> ..
>
> > +#include "metasound_cb0808.c"
> > +#include "metasound_cb1616.c"
> > +#include "metasound_cb4432.c"
> > +#include "metasound_cb4448s.c"
>
> What's the sense of separating these?
It's easier to deal with them this way - there are too many codebooks
after all and I've added only some of them so far (4 out of 18 modes).
> > --- /dev/null
> > +++ b/libavcodec/metasound_data.h
> > @@ -0,0 +1,51 @@
> > +
> > +#ifndef AVCODEC_METASOUND_DATA_H
> > +#define AVCODEC_METASOUND_DATA_H
> > +
> > +#include <stddef.h>
> > +#include <stdint.h>
> > +
> > +#include "twinvq_common.h"
> > +
> > +typedef ModeTab MetasoundModeTab;
> > +
> > +extern const MetasoundModeTab metasound_mode0806;
> > +extern const MetasoundModeTab metasound_mode0806s;
> > +extern const MetasoundModeTab metasound_mode0808;
> > +extern const MetasoundModeTab metasound_mode0808s;
> > +extern const MetasoundModeTab metasound_mode1110;
> > +extern const MetasoundModeTab metasound_mode1110s;
> > +extern const MetasoundModeTab metasound_mode1616;
> > +extern const MetasoundModeTab metasound_mode1616s;
> > +extern const MetasoundModeTab metasound_mode2224;
> > +extern const MetasoundModeTab metasound_mode2224s;
> > +extern const MetasoundModeTab metasound_mode2232;
> > +extern const MetasoundModeTab metasound_mode2232s;
> > +extern const MetasoundModeTab metasound_mode4432;
> > +extern const MetasoundModeTab metasound_mode4432s;
> > +extern const MetasoundModeTab metasound_mode4440;
> > +extern const MetasoundModeTab metasound_mode4440s;
> > +extern const MetasoundModeTab metasound_mode4448;
> > +extern const MetasoundModeTab metasound_mode4448s;
>
> ff_ prefix
>
> > --- /dev/null
> > +++ b/libavcodec/metasound_data_fcb.c
> > @@ -0,0 +1,590 @@
> > +
> > +static const int16_t fcb8l[] = {
> > + -1239, -1310, -1240, -1146, -1337, 1303, -482, 2215,
>
> ..
>
> > --- /dev/null
> > +++ b/libavcodec/metasound_data_lsp.c
> > @@ -0,0 +1,475 @@
> > +
> > +static const float lsp8[] = {
> > + 0.27020001, 0.50959998, 0.6437, 0.76719999, 0.96390003, 1.0696, 1.2625,
> > 1.5789,
>
> ..
>
> > --- /dev/null
> > +++ b/libavcodec/metasound_data_shape.c
> > @@ -0,0 +1,540 @@
> > +
> > +static const int16_t shape8[] = {
> > + 2765, 1262, 6624, 867, 688, 1884, 3245, 1248,
>
> ..
>
> Diego
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
