On 11 November 2016 at 02:25, James Almer <jamr...@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 <atomnu...@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.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
