[FFmpeg-devel] [PATCH 1/2] avcodec/alacdec: split off decorrelate_stereo and append_extra_bits as alacdsp

[Michael Niedermayer]

On Sun, Oct 04, 2015 at 01:31:27AM -0300, James Almer wrote:
> On 10/4/2015 12:44 AM, James Almer wrote:
> > Signed-off-by: James Almer <jamrial at gmail.com>
> > ---
> >  libavcodec/Makefile  |  2 +-
> >  libavcodec/alac.c    | 65 +++++++++++++++++++---------------------------------
> >  libavcodec/alacdsp.c | 57 +++++++++++++++++++++++++++++++++++++++++++++
> >  libavcodec/alacdsp.h | 35 ++++++++++++++++++++++++++++
> >  4 files changed, 116 insertions(+), 43 deletions(-)
> >  create mode 100644 libavcodec/alacdsp.c
> >  create mode 100644 libavcodec/alacdsp.h
> > 
> > diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> > index b15e431..153c3f8 100644
> > --- a/libavcodec/Makefile
> > +++ b/libavcodec/Makefile
> > @@ -143,7 +143,7 @@ OBJS-$(CONFIG_AC3_ENCODER)             += ac3enc_float.o ac3enc.o ac3tab.o \
> >                                            ac3.o kbdwin.o
> >  OBJS-$(CONFIG_AC3_FIXED_ENCODER)       += ac3enc_fixed.o ac3enc.o ac3tab.o ac3.o
> >  OBJS-$(CONFIG_AIC_DECODER)             += aic.o
> > -OBJS-$(CONFIG_ALAC_DECODER)            += alac.o alac_data.o
> > +OBJS-$(CONFIG_ALAC_DECODER)            += alac.o alac_data.o alacdsp.o
> >  OBJS-$(CONFIG_ALAC_ENCODER)            += alacenc.o alac_data.o
> >  OBJS-$(CONFIG_ALIAS_PIX_DECODER)       += aliaspixdec.o
> >  OBJS-$(CONFIG_ALIAS_PIX_ENCODER)       += aliaspixenc.o
> > diff --git a/libavcodec/alac.c b/libavcodec/alac.c
> > index 827c0db..b767438 100644
> > --- a/libavcodec/alac.c
> > +++ b/libavcodec/alac.c
> > @@ -57,6 +57,7 @@
> >  #include "unary.h"
> >  #include "mathops.h"
> >  #include "alac_data.h"
> > +#include "alacdsp.h"
> >  
> >  #define ALAC_EXTRADATA_SIZE 36
> >  
> > @@ -81,6 +82,8 @@ typedef struct ALACContext {
> >  
> >      int direct_output;
> >      int extra_bit_bug;
> > +
> > +    ALACDSPContext dsp;
> >  } ALACContext;
> >  
> >  static inline unsigned int decode_scalar(GetBitContext *gb, int k, int bps)
> > @@ -230,35 +233,6 @@ static void lpc_prediction(int32_t *error_buffer, int32_t *buffer_out,
> >      }
> >  }
> >  
> > -static void decorrelate_stereo(int32_t *buffer[2], int nb_samples,
> > -                               int decorr_shift, int decorr_left_weight)
> > -{
> > -    int i;
> > -
> > -    for (i = 0; i < nb_samples; i++) {
> > -        int32_t a, b;
> > -
> > -        a = buffer[0][i];
> > -        b = buffer[1][i];
> > -
> > -        a -= (b * decorr_left_weight) >> decorr_shift;
> > -        b += a;
> > -
> > -        buffer[0][i] = b;
> > -        buffer[1][i] = a;
> > -    }
> > -}
> > -
> > -static void append_extra_bits(int32_t *buffer[2], int32_t *extra_bits_buffer[2],
> > -                              int extra_bits, int channels, int nb_samples)
> > -{
> > -    int i, ch;
> > -
> > -    for (ch = 0; ch < channels; ch++)
> > -        for (i = 0; i < nb_samples; i++)
> > -            buffer[ch][i] = (buffer[ch][i] << extra_bits) | extra_bits_buffer[ch][i];
> > -}
> > -
> >  static int decode_element(AVCodecContext *avctx, AVFrame *frame, int ch_index,
> >                            int channels)
> >  {
> > @@ -389,19 +363,24 @@ static int decode_element(AVCodecContext *avctx, AVFrame *frame, int ch_index,
> >          decorr_left_weight = 0;
> >      }
> >  
> > -    if (alac->extra_bits && alac->extra_bit_bug) {
> > -        append_extra_bits(alac->output_samples_buffer, alac->extra_bits_buffer,
> > -                          alac->extra_bits, channels, alac->nb_samples);
> > -    }
> > +    if (channels == 2) {
> > +        if (alac->extra_bits && alac->extra_bit_bug) {
> > +            alac->dsp.append_extra_bits_stereo(alac->output_samples_buffer, alac->extra_bits_buffer,
> > +                                               alac->extra_bits, channels, alac->nb_samples);
> > +        }
> >  
> > -    if (channels == 2 && decorr_left_weight) {
> > -        decorrelate_stereo(alac->output_samples_buffer, alac->nb_samples,
> > -                           decorr_shift, decorr_left_weight);
> > -    }
> > +        if (decorr_left_weight) {
> 
> So while i was writing a checkasm unit i started looking at the possible
> values for this, and found out that apparently it's either 0 or 1.
> Since the function below is not called when it's 0, wouldn't that mean
> it's a pointless argument?
> 
> I'll have to redo the asm function if that's the case, and change the
> prototype and boilerplate c version.

why would it only be 0 or 1 ?
(it sure could be in actual files but the syntax allows 8bit values)

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I do not agree with what you have to say, but I'll defend to the death your
right to say it. -- Voltaire
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20151004/446af7a0/attachment.sig>