[FFmpeg-devel] [PATCH] AAC encoder: improve SF range utilization

Michael Niedermayer michaelni at gmx.at
Wed Dec 2 02:47:46 CET 2015


On Tue, Dec 01, 2015 at 03:35:40AM -0300, Claudio Freire wrote:
> On Mon, Nov 30, 2015 at 1:04 PM, Hendrik Leppkes <h.leppkes at gmail.com> wrote:
> > On Mon, Nov 30, 2015 at 4:50 PM, Claudio Freire <klaussfreire at gmail.com> wrote:
> >> On Mon, Nov 30, 2015 at 12:27 PM, Rostislav Pehlivanov
> >> <atomnuker at gmail.com> wrote:
> >>> On Sun, 2015-11-29 at 16:54 -0300, Claudio Freire wrote:
> >>>> Before pushing this, I'd like some feedback,
> >>>> especially about
> >>>> the implementation of point 3. I'm not sure the AAC encoder
> >>>> setting the cutoff in the encoder context like this is legal or
> >>>> desirable.
> >>> I think setting the cutoff is necessary. You can't avoid holes and yet
> >>> keep a full bandwidth as you decrease bits unless you like the sound of massive quantization errors.
> >>
> >> My point was more about whether a codec should write into the context
> >> struct like that or not. Basically an API technicality that's unclear
> >> to me.
> >>
> >
> > It seems slightly odd to write into that variable, since thats the
> > cutoff the user requests.
> > Maybe you can use/write into a private variable instead? ac3enc seems
> > to use a private variable for similar purposes.
> 
> Attached patch does that.
> 
> I'm far more comfortable with this implementation, so if all agree, I'll push.
> 
> On Mon, Nov 30, 2015 at 2:20 PM, Rostislav Pehlivanov
> <atomnuker at gmail.com> wrote:
> > On Mon, 2015-11-30 at 12:50 -0300, Claudio Freire wrote:
> >> Also I don't see how a static var would help or even be correct here.
> >> Perhaps you meant something else?
> > static uint8_t cond1 = param1 && param2;
> > static uint8_t cond2 = param3 && !param4;
> > ...etc
> > return cond1 && cond2;
> 
> I didn't do that since it would not short circuit some terms that
> ought to be short-circuited.
> 
> But I did split lines and it does read better. In fact, maybe that was
> all that was needed?

>  libavcodec/aaccoder.c         |   60 ++++++++++++------
>  libavcodec/aaccoder_twoloop.h |  136 ++++++++++++++++++++++++++++--------------
>  libavcodec/aacenc.c           |    2 
>  libavcodec/aacenc_is.c        |   11 ++-
>  libavcodec/aacenc_utils.h     |   63 +++++++++++++++++++
>  libavcodec/aacpsy.c           |   20 ++++--
>  libavcodec/psymodel.c         |    1 
>  libavcodec/psymodel.h         |    1 
>  tests/fate/aac.mak            |   18 ++---
>  9 files changed, 231 insertions(+), 81 deletions(-)
> 404ff616bb267619be431a7c45afa4db82070b65  0001-AAC-encoder-improve-SF-range-utilization.patch

some stuff in mips will need to be updated by someone or disabled
after this is pushed:
CC      libavcodec/aacpsy.o
/ffmpeg/libavcodec/aacpsy.c: In function ‘psy_3gpp_analyze_channel’:
/ffmpeg/libavcodec/aacpsy.c:666: error: too many arguments to function ‘calc_thr_3gpp_mips’
make: *** [libavcodec/aacpsy.o] Error 1
make: Target `all' not remade because of errors.

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

During times of universal deceit, telling the truth becomes a
revolutionary act. -- George Orwell
-------------- 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/20151202/e7a73e87/attachment.sig>


More information about the ffmpeg-devel mailing list