[PATCH] High quality AAC encoder on Mac OS X

[Ritsuka]

Well the choice here is to have an aac encoder that sucks on every platform or to have an additional one on mac os x that sucks a bit less.
And IMHO one platform is better than nothing.
I don't want to add this to turn the other platform in second class citizens, if there was a free encoder that we could use on Windows ( Apple's aac encoder on windows requires an QuickTime pro license) and Linux I would gladly add it. But there ain't any.
I am not saying to remove faac, but to add a choice, open-sources is all about choices. All the preset can still use faac and maintains cross-platform similar output, but if one wants to use a different aac encode on mac os x, he would not be constrained to faac.

And really, two #ifdef and an additional menuitem in the gui is not really something I would call complicate.

[jbrjake]

And really, two #ifdef and an additional menuitem in the gui is not really something I would call complicate.

I don't mean your implementation is complicated, but it complicates the code from the point of view of a maintainer.

* It can only be tested on one platform, so a number of developers will be out of the loop entirely and unable to even attempt to reproduce support issues.
* Apple can change how it works at any time.
* If that happened, and you weren't around, I don't think there's anyone else here who would know how to fix it immediately.

[Ritsuka]

1) 90% of the handbrake's developer own a mac. And it's not like we don't have platform specific problems already (ffmpeg on windows? libswscale on 64bit?)
2) I don't think apple is gonna break is own state of the art api used by their own applications and by every other professional audio app anytime soon. They first will have to mark it deprecated, and then remove 5 five years later. Even QuickDraw is still supported on Snow Leopard, Core Audio is here to stay.
3) I guess Van could fix it in 30 seconds

[jbrjake]

1) 90% of the handbrake's developer own a mac.

It's lower than that, actually. We have 8 active devs, only 6 have Macs. 75%.

And it's not like we don't have platform specific problems already (ffmpeg on windows? libswscale on 64bit?)

The existence of problems is not a rationale for adding the potential for more problems, imo.

2) I don't think apple is gonna break is own state of the art api used by their own applications and by every other professional audio app anytime soon. They first will have to mark it deprecated, and then remove 5 five years later. Even QuickDraw is still supported on Snow Leopard, Core Audio is here to stay.

I have zero faith in Apple sticking by any decision long-term.

3) I guess Van could fix it in 30 seconds

We put enough of a burden on Van already. Every second he spends on HB is one less second spent developing next-generation networking architectures. I think expecting him to fix problems in code he didn't write is taking him for granted.

And of the 6 Mac devs, that includes you (who in this hypothetical would be unavailable), Scott and Joe who are busy enough with their interfaces, and me, who knows nada about Core APIs.

[cvk_b]

As we all love cross-platform features parity in libhb, the truth is that after an year and half, there is no aac encoder on par with apple and nero ones yet (and not even par with lame and aoTuV).
So I think it would be a really nice addition to add apple's aac encoder as one of the usable encoder of HandBrake. At least on Mac OS X.

An updated patch:
- Does not replaces faac, but add ca_aac as a new aac encoder.
- Fixed a memory leak.
- Bit rate validation.
- The new encoder is enabled only on mac os x. So libhb still compiles without it on other platforms.

http://handbrake.fr/pastebin/pastebin.php?show=319

Code: Select all

** BUILD SUCCEEDED **

Love it!!! Thanks!

[Ritsuka]

Updated patch: http://handbrake.fr/pastebin/pastebin.php?show=323

[JohnAStebbins]

Just casting a vote here. I think platform specific additions like this should be evaluated case by case. We shouldn't have drastically different feature sets on the different platforms. But in a case like this where we are just allowing one platform to produce better quality audio, I'm for it. Us freetards probably won't know we're getting substandard audio anyway.

Although I would like to see platform specific changes be mostly isolated to a platform subdir to keep the core code easily distinguishable.

[cvk_b]

Updated patch: http://handbrake.fr/pastebin/pastebin.php?show=323

I just wanted to say thank you. I have been using this exclusively with the cli "-E ca_aac -B 224" and WOW! So much better with DPLII. This really makes me want to go back and redo some films. I think Jon has some valid arguments against this but they are far outweighed by the quality improvement this adds. And fwiw, if the win people somehow got nero going I'd be totally happy for them!

[eddyg]

My vote would be to include it on macosx, I'd certainly use it, but via a libhb plugin mechanism to keep it out of libhb.

[dynaflash]

My vote would be to include it on macosx, I'd certainly use it, but via a libhb plugin mechanism to keep it out of libhb.

I agree.

[cvk_b]

Code: Select all

./configure --enable-coreaudio

[eddyg]

Code: Select all

./configure --enable-coreaudio

It's more the internal APIs I was thinking about. Trying to not pollute libhb with any knowledge whatsoever of the coreaudio abilities on MacOS X. So some sort of registration of an encoder, with what capabilities it supports. The UI's can use that info to make the encoder available. Configuration of adhoc abilities would be harder, given that all configuration would be from the UI to libhb and then out to the encoder. Probably would need some sort of abstract configuration mechanism using type/value tuples (e.g. [{"bitrate", number, 128000}, {"foo", number, 1}]) where libhb doesn't even know what that info means.

Cheers, Ed.

[tokyovigilante]

As we all love cross-platform features parity in libhb, the truth is that after an year and half, there is no aac encoder on par with apple and nero ones yet (and not even par with lame and aoTuV).
So I think it would be a really nice addition to add apple's aac encoder as one of the usable encoder of HandBrake. At least on Mac OS X.

An updated patch:
- Does not replaces faac, but add ca_aac as a new aac encoder.
- Fixed a memory leak.
- Bit rate validation.
- The new encoder is enabled only on mac os x. So libhb still compiles without it on other platforms.

http://handbrake.fr/pastebin/pastebin.php?show=319

Thanks, this is really nice.

[nightstrm]

Compiled last night... thanks for maintaining this patch. I think the plugin architecture suggested by eddyg earlier sounds like an interesting way to handle things like this.

[mithrandir]

The alternative, for those of us on the Windows platform, is to use the MKV container and encode our audio with LAME MP3. I understand that is not a palatable choice (since AVC/AAC are considered the standard) but at least we know what we are getting with LAME. LAME's CBR mode, while theoretically inferior to VBR AAC in terms of compression efficiency, is at least capable of maintaining high fidelity and stereo separation/imaging when used at higher bitrates (e.g. 192kbps). Of course, LAME VBR would be even better since the current encoder produces very impressive results at 130-150kbps but I understand using VBR can lead to audio sync issues. A shame.

What about Vorbis? Vorbis was designed with VBR in mind - it's bitrate-managed mode is more of a hack - so you may do better to stick with LAME. Vorbis is also not quite as well supported by players as ubiquitous MP3. (Of course players that can handle MKV can typically decode Vorbis). I'm using a Western Digital TV HD media player which can handle both.

[rpp3po]

Could somebody please post a link to a build? That's an awesome feature!

[s55]

There isn't a build. If you can't compile yourself, this is something you will probably be waiting for, for some time while the framework to allow such a feature to go in, gets done.

[Ritsuka]

Updated patch, hopefully I haven't included any useless change from my working copy.
http://handbrake.fr/pastebin/pastebin.php?show=394

[eddyg]

after going through the diff I think that there are only some small changes required before this could be committed.

The bits that I would like to see changed are:

1. Removed ifdefed registration of codec, may need a different registration mechanism, maybe distributed, I've got some ideas on that.

2. Remove ifdefed test.c code, replace with call to ask if codec is registered.

The rest of the changes are ok and don't really pollute the rest of hb in any real sense.

Cheers Ed

[nightstrm]

Any chance we could get an update... the patch no longer seems to apply to SVN head.

[refulgentis]

Any chance we could get an update... the patch no longer seems to apply to SVN head.

i cobbled this together:
http://pastebin.com/m1f339f98

[nightstrm]

Any chance we could get an update... the patch no longer seems to apply to SVN head.

i cobbled this together:
http://pastebin.com/m1f339f98

Thanks I'll give it a shot tonight after work.

[nightstrm]

Any chance we could get an update... the patch no longer seems to apply to SVN head.

i cobbled this together:
http://pastebin.com/m1f339f98

I'm having some trouble applying your updated patch against SVN head (2091). I get the message shown below; any thoughts?

MadRiver:HandBrake nateut$ patch -p0 < ca.patch
patch: **** Only garbage was found in the patch input.

[refulgentis]

Any chance we could get an update... the patch no longer seems to apply to SVN head.

i cobbled this together:
http://pastebin.com/m1f339f98

I'm having some trouble applying your updated patch against SVN head (2091). I get the message shown below; any thoughts?

MadRiver:HandBrake nateut$ patch -p0 < ca.patch
patch: **** Only garbage was found in the patch input.

no clue O_o its certainly not garbage, and when I hit the download link it is plain text so its not a rich text/plain text thing.

update: I patched successfully against 2091: hit download on pastebin, drag file to handbrake folder in finder. patch -p0 < (filename.txt), and it works successfuly. I'm trying to build now, will update again
update 2: i was messing around with xdot's alpha, and apparently it overwrote my math.h with gpac's math.h
i'll look at it again tomorrow, its 3:30 am and I have class in 6 hours
update 3: didn't overwrite, but placed a copy of GPAC's in /usr/local/include which precedes /usr/include in my path. the original patch I posted is fine if you already have encca_aac.c, but this patch will work against a clean checkout of r2091: http://pastebin.com/m41eb1b33

[Ritsuka]

http://handbrake.fr/pastebin/pastebin.php?show=439