[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