[FFmpeg-trac] #2686(avcodec:open): Native AAC encoder collapses at high bitrates on some samples

[FFmpeg]

#2686: Native AAC encoder collapses at high bitrates on some samples
-------------------------------------+-------------------------------------
             Reporter:  Kamedo2      |                    Owner:
                 Type:  defect       |                   Status:  open
             Priority:  normal       |                Component:  avcodec
              Version:  git-master   |               Resolution:
             Keywords:  aac          |               Blocked By:
  regression                         |  Reproduced by developer:  1
             Blocking:               |
Analyzed by developer:  0            |
-------------------------------------+-------------------------------------

Comment (by klaussfreire):

 Replying to [comment:292 cehoyos]:
 > I am assuming that this patch is meant to be applied to FFmpeg git
 master soon, the following is partly necessary, partly just a suggestion:
 > * The patch contains trailing white space, this cannot be pushed to our
 git repository, please remove it. {{{tools/patcheck}}} can help you
 finding such issues.

 Forgot about that. Will do. But this patch is only a POC, I'll re-do the
 changes (verbatim, but in steps) and post them as separate, progressive
 patches.

 > * Please either remove all printf's or make them av_log's.

 Of course.

 > * The function sqrf() is duplicated iiuc, please move it to a header (if
 the function is necessary).

 Ok.

 > * There are three or four blocks where you just reindent existing code.
 It makes reading your patch (in the future) easier if you don't reindent
 them right now in the same commit, just leave them where they are. I can
 do the reindent for you (or you can send a followup patch).

 I could do the patch with -w to make it not diff whitespace-only. Does
 that work equally well? (there's a lot of code that really needs
 reindenting or it becomes unreadable). I could post both (with and without
 -w).

 > * And finally (purely optional):
 > Using the following
 > {{{
 > if (condition) {
 >     do1;
 > } else {
 >     do2;
 > }
 > }}}
 > instead of
 > {{{
 > if (condition)
 >     do1;
 > else
 >     do2;
 > }}}
 > has the advantage that future changes are smaller and easier to read
 (this point is of course up to you, it is your code).

 Surely. Easy enough to change.


 Replying to [comment:294 Timothy_Gu]:
 > @cehoyos: this patch makes ANMR worse than default twoloop, even it is
 theoretically better and takes more time. While Claudio expresses interest
 to work on ANMR later, I don't think committing a patch that makes
 something worse than they should be is a good idea.

 What I could do, is try to get rid of a common hole-avoidance bug that
 makes all the other coders much worse. It shouldn't be hard. Other than
 that, I think only updating the doc is pertinent to thi patch set.

 > Other than that, @klaussfreire, if the patch was to be applied to
 master, you could split this patch to at least two patches. The change of
 default to M/S encoding should also be documented somehow.

 Of course. Bugfixes first, improvements next. I need to update my A/B
 testing script, so I can run all patches through it - especially bugfixes,
 where automated A/B testing does work.

--
Ticket URL: <https://trac.ffmpeg.org/ticket/2686#comment:297>
FFmpeg <https://ffmpeg.org>
FFmpeg issue tracker