[FFmpeg-devel] [PATCH] opus_celt: rename structures to better names and reorganize them

[Rostislav Pehlivanov]

On 11 November 2016 at 02:25, James Almer <jamrial at gmail.com> wrote:

> On 11/10/2016 1:17 PM, Rostislav Pehlivanov wrote:
> > This is meant to be applied on top of my previous patch which
> > split PVQ into celt_pvq.c and made opus_celt.h
> >
> > Essentially nothing has been changed other than renaming CeltFrame
> > to CeltBlock (CeltFrame had absolutely nothing at all to do with
> > a frame) and CeltContext to CeltFrame.
> > 3 variables have been put in CeltFrame as they make more sense
> > there rather than being passed around as arguments.
> > The coefficients have been moved to the CeltBlock structure
> > (why the hell were they in CeltContext and not in CeltFrame??).
> >
> > Now the encoder would be able to use the exact context the decoder
> > uses (plus a couple of extra fields in there).
> >
> > FATE passes, no slowdowns, etc.
> >
> > Signed-off-by: Rostislav Pehlivanov <atomnuker at gmail.com>
> > ---
> >  libavcodec/opus.h      |  14 +-
> >  libavcodec/opus_celt.c | 719 +++++++++++++++++++++++++-----
> -------------------
> >  libavcodec/opus_celt.h |  82 +++---
> >  libavcodec/opus_pvq.c  |  50 ++--
> >  libavcodec/opus_pvq.h  |   8 +-
> >  libavcodec/opus_rc.c   |  12 +-
> >  libavcodec/opus_rc.h   |   6 +-
> >  libavcodec/opusdec.c   |   1 +
> >  8 files changed, 449 insertions(+), 443 deletions(-)
> >
> > diff --git a/libavcodec/opus.h b/libavcodec/opus.h
> > index be04249..e8d13df 100644
> > --- a/libavcodec/opus.h
> > +++ b/libavcodec/opus.h
> > @@ -75,7 +75,7 @@ enum OpusBandwidth {
> >
> >  typedef struct SilkContext SilkContext;
> >
> > -typedef struct CeltContext CeltContext;
> > +typedef struct CeltFrame CeltFrame;
>
> [...]
>
> > diff --git a/libavcodec/opus_celt.h b/libavcodec/opus_celt.h
> > index 8a15c8d..5bce8fc 100644
> > --- a/libavcodec/opus_celt.h
> > +++ b/libavcodec/opus_celt.h
> > @@ -37,7 +37,16 @@ enum CeltSpread {
> >      CELT_SPREAD_AGGRESSIVE
> >  };
> >
> > -typedef struct CeltFrame {
> > +enum CeltBlockSize {
> > +    CELT_BLOCK_120,
> > +    CELT_BLOCK_240,
> > +    CELT_BLOCK_480,
> > +    CELT_BLOCK_960,
> > +
> > +    CELT_BLOCK_NB
> > +};
> > +
> > +typedef struct CeltBlock {
> >      float energy[CELT_MAX_BANDS];
> >      float prev_energy[2][CELT_MAX_BANDS];
> >
> > @@ -45,50 +54,46 @@ typedef struct CeltFrame {
> >
> >      /* buffer for mdct output + postfilter */
> >      DECLARE_ALIGNED(32, float, buf)[2048];
> > +    DECLARE_ALIGNED(32, float, coeffs)[CELT_MAX_FRAME_SIZE];
> >
> >      /* postfilter parameters */
> > -    int pf_period_new;
> > +    int   pf_period_new;
> >      float pf_gains_new[3];
> > -    int pf_period;
> > +    int   pf_period;
> >      float pf_gains[3];
> > -    int pf_period_old;
> > +    int   pf_period_old;
> >      float pf_gains_old[3];
> >
> > -    float deemph_coeff;
> > -} CeltFrame;
> > +    float emph_coeff;
> > +} CeltBlock;
> >
> > -struct CeltContext {
> > +typedef struct CeltFrame {
>
> You're typedefing CeltFrame again here. Keep it the same as CeltContext
> used to be.
>

Fixed locally


>
> >      // constant values that do not change during context lifetime
> > -    AVCodecContext    *avctx;
> > -    IMDCT15Context    *imdct[4];
> > -    AVFloatDSPContext  *dsp;
> > +    AVCodecContext      *avctx;
> > +    IMDCT15Context      *imdct[4];
> > +    AVFloatDSPContext   *dsp;
>
> Leave cosmetics for another patch, so actual changes are easier to review.
>
>
Cosmetics is what the entire patch is about. As I said, the only changes
apart from cosmetics is moving the coefficients to the CeltBlock structure
and adding 2 new variables. I'll split the patch though.