[FFmpeg-devel] [PATCH V3 1/2] avcodec/vorbisenc: Add pre-echo detection

[Tyler Jones]

> > --- a/libavcodec/vorbisenc.c
> > +++ b/libavcodec/vorbisenc.c
> > @@ -33,6 +33,7 @@
> >  #include "mathops.h"
> >  #include "vorbis.h"
> >  #include "vorbis_enc_data.h"
> > +#include "vorbispsy.h"
> >  
> >  #include "audio_frame_queue.h"
> >  #include "libavfilter/bufferqueue.h"
> > @@ -136,6 +137,7 @@ typedef struct vorbis_enc_context {
> >      int64_t next_pts;
> >  
> >      AVFloatDSPContext *fdsp;
> > +    VorbisPsyContext *vpctx;
> 
> Why a pointer? I don't see the benefit. It means an unnecessary malloc
> and free call.

You're probably right. It's changed now.

> > @@ -1252,6 +1270,7 @@ static av_cold int vorbis_encode_close(AVCodecContext *avctx)
> >      ff_mdct_end(&venc->mdct[1]);
> >      ff_af_queue_close(&venc->afq);
> >      ff_bufqueue_discard_all(&venc->bufqueue);
> > +    ff_psy_vorbis_close(venc->vpctx);
> 
> You should pass a pointer to venc->vpctx instead, regardless of what you
> do with the comment above.

I'm not sure I understand what you mean. It is passing a pointer to a
VorbisPsyContext, please see the prototype:

    av_cold void ff_psy_vorbis_close(VorbisPsyContext *vpctx);    

> > +/**
> > + * Calculate the variance of a block of samples
> > + *
> > + * @param in     Array of input samples
> > + * @param length Number of input samples being analyzed
> > + * @return       The variance for the current block
> > + */
> > +static float variance(const float *in, int length)
> > +{
> > +    int i;
> > +    float mean = 0.0f, square_sum = 0.0f;
> > +
> > +    for (i = 0; i < length; i++) {
> > +        mean += in[i];
> > +        square_sum += in[i] * in[i];
> 
> Can't you use AVFloatDSPContext's scalarproduct_float for square_sum?
> The constrains are lax. 16 byte alignment for in and length a multiple
> of 4. You can pad the buffer if needed to achieve that.

You are correct, it is switched over now.

> > +    }
> > +
> > +    mean /= length;
> > +    return (square_sum - length * mean * mean) / (length - 1);
> > +}
> > +
> > +av_cold int ff_psy_vorbis_init(VorbisPsyContext *vpctx, int sample_rate,
> > +                               int channels, int blocks)
> > +{
> > +    int crit_freq;
> > +    float Q[2] = {.54, 1.31}; // Quality values for maximally flat cascaded filters
> 
> const float Q[2]

Fixed.

Thank you for catching these mistakes and providing suggestions. A new version
of this patch will be sent soon.

Thanks again,

Tyler Jones
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20170728/21968030/attachment.sig>