[FFmpeg-devel] [Patch] AAC encoder improvements

[Michael Niedermayer]

On Sat, May 04, 2013 at 06:44:59PM -0300, Claudio Freire wrote:
> On Sat, May 4, 2013 at 6:03 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> > like derek suggested, can you split this into 1 patch/commit per
> > issue ?
> > (see "git reset HEAD^ ; git add -p ; git commit" if you dont know how
> > to do that easily)
> 
> 
> I'm rather new to git, being more of a mercurial/SVN guy myself. I
> still find it rather confusing.
> 
> I'll give add -p a chance next time, now I already did the split
> manually editing hunks (I know, horrible, but I've got some experience
> manually editing hunks and I got used to it).
> 
> Anyway, attached.

>  aacpsy.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> b1956657d80ebc85c732ce1331e87b116c2455d9  0001-aac-psy-rounding.patch
> From ae41dbfb351ea4b14c1e8b3e24656da38ffd14c3 Mon Sep 17 00:00:00 2001
> From: Claudio Freire <klaussfreire at gmail.com>
> Date: Sat, 4 May 2013 18:35:49 -0300
> Subject: [PATCH 1/5] AAC encoder: Fixed a rounding bug in psy's channel bitrate computation.

applied


[...]

> @@ -814,12 +819,12 @@ static void search_for_quantizers_twoloop(AVCodecContext *avctx,
>                  }
>              }
>              if (tbits > destbits) {
> -                for (i = 0; i < 128; i++)
> -                    if (sce->sf_idx[i] < 218 - qstep)
> +                for (i = 0; i < 128; i++) 
> +                    if (sce->sf_idx[i] < 218 - qstep)

that looks unintended


>                          sce->sf_idx[i] += qstep;
> -            } else {
> -                for (i = 0; i < 128; i++)
> -                    if (sce->sf_idx[i] > 60 - qstep)
> +            } else if (tbits < destbits) {
> +                for (i = 0; i < 128; i++) 
> +                    if (sce->sf_idx[i] > 60 + qstep) 
>                          sce->sf_idx[i] -= qstep;
>              }
>              qstep >>= 1;
> -- 
> 1.7.3.4
> 

>  aaccoder.c |    4 ++--
>  aacenc.c   |    9 +++++++++
>  2 files changed, 11 insertions(+), 2 deletions(-)
> 4418865c669267d4c773f7e7d0d20cd2cfe116b8  0005-aac-jointstereo.patch
> From cf7ebf247ffca9cd2517f52b99b6922cbf3c1e3b Mon Sep 17 00:00:00 2001
> From: Claudio Freire <klaussfreire at gmail.com>
> Date: Sat, 4 May 2013 18:39:15 -0300
> Subject: [PATCH 5/5] Several improvements to the AAC encoder:
>    * After MS mode search, psy and quantization must be re-done, or the
>      resulting quantization nosie will be ridiculously wrong.
>    * MS cost estimation should use avg thresholds for mid channel (avg
>      signal would result in avg thresholds per psy model), changed side
>      channel to use min threshold. Side thresholds aren't estimable in
>      any way other than recalculation (TODO), so min in the most
>      conservative estimate short of a re-application of psy.
>      Seems to work fine enough like this.
> 
> ---
>  libavcodec/aaccoder.c |    4 ++--
>  libavcodec/aacenc.c   |    9 +++++++++
>  2 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/libavcodec/aaccoder.c b/libavcodec/aaccoder.c
> index 86be276..e0285bb 100644
> --- a/libavcodec/aaccoder.c
> +++ b/libavcodec/aaccoder.c
> @@ -1072,7 +1072,7 @@ static void search_for_ms(AACEncContext *s, ChannelElement *cpe,
>                      FFPsyBand *band0 = &s->psy.ch[s->cur_channel+0].psy_bands[(w+w2)*16+g];
>                      FFPsyBand *band1 = &s->psy.ch[s->cur_channel+1].psy_bands[(w+w2)*16+g];
>                      float minthr = FFMIN(band0->threshold, band1->threshold);
> -                    float maxthr = FFMAX(band0->threshold, band1->threshold);
> +                    float avgthr = 0.5f*(band0->threshold + band1->threshold);
>                      for (i = 0; i < sce0->ics.swb_sizes[g]; i++) {
>                          M[i] = (sce0->coeffs[start+w2*128+i]
>                                + sce1->coeffs[start+w2*128+i]) * 0.5;
> @@ -1100,7 +1100,7 @@ static void search_for_ms(AACEncContext *s, ChannelElement *cpe,
>                                                  sce0->ics.swb_sizes[g],
>                                                  sce0->sf_idx[(w+w2)*16+g],
>                                                  sce0->band_type[(w+w2)*16+g],
> -                                                lambda / maxthr, INFINITY, NULL);
> +                                                lambda / avgthr, INFINITY, NULL);
>                      dist2 += quantize_band_cost(s, S,
>                                                  S34,
>                                                  sce1->ics.swb_sizes[g],
> diff --git a/libavcodec/aacenc.c b/libavcodec/aacenc.c
> index 80dd3d8..aa93c90 100644
> --- a/libavcodec/aacenc.c
> +++ b/libavcodec/aacenc.c
> @@ -621,6 +621,15 @@ static int aac_encode_frame(AVCodecContext *avctx, AVPacket *avpkt,
>                  }
>              }
>              adjust_frame_information(cpe, chans);
> +            if (cpe->ms_mode) {
> +                /* Re-evaluate psy model and quantization selection based on
> +                   MS-transformed channels */
> +                s->psy.model->analyze(&s->psy, start_ch, coeffs, wi);
> +                for (ch = 0; ch < chans; ch++) {
> +                    s->cur_channel = start_ch * 2 + ch;
> +                    s->coder->search_for_quantizers(avctx, s, &cpe->ch[ch], s->lambda);
> +                }
> +            }

shouldnt this and the previous hunks be in seperate patches or is
there some dependance ?

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

While the State exists there can be no freedom; when there is freedom there
will be no State. -- Vladimir Lenin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130505/7e4736fe/attachment.asc>