Bug 374670

Created attachment 103405 [details]
aacfix

Here is an initial patch. I think it could be greatly improved.

Still allow use of libfaac if that's all that's available. It's not the best quality AAC encoder, but it's better than nothing.

Check ffmpeg version. If ffmpeg version < 3.x:
- Possibly prefer libfdk_aac if it's available, since ffmpeg's aac encoder was still considered experimental
If ffmpeg > 3.x:
- Don't use 'strict -2' since it's not necessary. I don't think it hurts anything to leave it there though.

Add an HE-AAC(v1/v2) encoder. I guess it would be easier to make that a completely separate transcoding option. libfdk_aac would have to be available, or else the option for HE-AAC should be unavailable (grayed out in the list).

Thank you for this patch, can you please submit it to reviewboard.kde.org, and subscribe the Amarok group to it?

(In reply to Myriam Schweingruber from comment #2)
> Thank you for this patch, can you please submit it to reviewboard.kde.org,
> and subscribe the Amarok group to it?

I tried, but my patches are not in the right format and I really didn't feel like creating yet another account. It's too many hoops for me to jump through for supporting a format I don't use for a music player I don't use. Feel free to use the ideas/code. I don't demand any sort of credit or attribution for anything here.

Created attachment 103641 [details]
Poor Attempt at Initial HE-AAC Support

Okay, it seems that this is not getting resolved on its own. How might *I* submit that patch to the review board? How does that work?

As described in comment #2. You need an account on identity.kde.org, then submit the patch to reviewboard.kde.org and subscribe the Amarok group to it. The websites are pretty elf-explaining

Sorry to have taken so long. I got locked out of the KDE reviewboard for some reason (account creation issues), and have been quite busy with schoolwork.

Unfortunately, I can't seem to submit the patch, with the error that I need a "git diff". How do I resolve this? I'd like to get this bug submitted, but I am still very new to this.

Thank you, and sorry to bother again.

How did you make this patch if you do not use git?
There are a huge load of HOWTOs available online, and there is also a quite comprehensive guide on our KDE wikis for this: https://community.kde.org/Get_Involved/development#Submitting_your_first_patch

I was the discoverer of the problem, but the other poster, DTL135, was the person to create the fix. He did not want to convert the patch format, however. I am guessing the format is with the diff command, but whatever it is, it is not git. Anyway, I am uncertain of the format and I can't submit it. I was told I could apply it with the patch command, however.

I am guessing I can maybe git clone the repo, patch it, and then... what? What git command creates a patch from recent changes like this? (It's probably idiotically simple, but git confuses me)

The command you are looking for is git diff :-)

Thanks. I managed to make a patch (and the patched program builds and runs!), unfortunately, it does not work. The problem is that the new patch for AAC searches this regex:  return ffmpegOutput.contains( QRegExp( "^ .EA....aac" ) );


Meanwhile (I assume it greps ffmpeg -codecs, but I don't know the exact command it filters output from) my search of the output ffmpeg -codecs | grep aac gives:

DEA.L. aac                  AAC (Advanced Audio Coding) (decoders: aac aac_fixed ) (encoders: aac libvo_aacenc )
 D.A.L. aac_latm             AAC LATM (Advanced Audio Coding LATM syntax).

The format for ffmpeg's output on this is DEV/A/ILS, with dots if one of the codes is not present. Since my ffmpeg has a decoder too, this breaks the regex.

I have to guess that the regex ONLY works if it matches perfectly, when it only really needs the A and E (Audio Codec, Encoder Present respectively). How do I rewrite the regex so that it still filters for AAC and EA, but not the rest (assuming the dots are searched verbatim and not separators)?

Can you please point me to a guide on how to write QRegexp or tell me what one to use? Thanks!

The second patch is broken too (the HE-AAC support one), but I am trying to contact the author...

QRegExp uses the standard syntax for regular expressions. The web is full of tutorials for those.

I found the guide, but it would seem that the dot character is supposed to match any character. This makes my assumption about why the regex is failing incorrect, and I need to do some more research...

Created attachment 105718 [details]
Git patch working for Amarok AAC transcoding now

I converted the patch to git, and after some reading on regex, got it working. To be honest, it may have been fine to start with, but I forgot to clear a cache before testing.

The patch is now in git format and works. DTL's patch seems to have been correct, but I had some issues testing it that are now resolved.

He-AAC format is still not working, and I don't know how to fix that one.

I am still working on this. I have recently gotten some spare time, so I am trying to run the tests for the submitted patch for regular AAC format encoding. 

(I do not understand this entirely, but I had thought that after submitting the patch, it would have gotten integrated into the source for Amarok. I am assuming that this is due to whatever weird version Ubuntu provides that I tested on, or due to my unability to run full testing.)

I have used git to copy the amarok repo to my computer, and have installed the dependencies with apt-get build-dep amarok. This seems to have left out a bunch of stuff for Mock and LibAV, and also nepomuk-core.

I think I have all the stuff for Mock and LibAV now, but nepomuk-core is not actually released for my version of Ubuntu. How do I get this package, or disable it from amarok if it is not strictly needed for testing transcoding?

/home/rdragonrydr/working_amarok/amarok/cmake/modules
Taglib found: -L/usr/lib/x86_64-linux-gnu -ltag
Taglib-Extras found: -L/usr/lib -ltag-extras
Found Qt-Version 4.8.7 (using /usr/bin/qmake)
Found X11: /usr/lib/x86_64-linux-gnu/libX11.so
Found KDE 4.12 include dir: /usr/include
Found KDE 4.12 library dir: /usr/lib
Found the KDE4 kconfig_compiler preprocessor: /usr/bin/kconfig_compiler
Found automoc4: /usr/bin/automoc4
Found liblastfm: /usr/include/lastfm, /usr/lib/x86_64-linux-gnu/liblastfm.so, version 1.0.9
Found libofa: /usr/include/ofa1, /usr/lib/x86_64-linux-gnu/libofa.so
Found MySQL: /usr/include/mysql, -L/usr/lib/x86_64-linux-gnu -lmysqlclient -lpthread -lz -lm -lrt -ldl
Found MySQL Embedded: /usr/include/mysql, -L/usr/lib/x86_64-linux-gnu -lmysqld -lpthread -lz -lm -lrt -lcrypt -ldl -laio -llz4 -lnuma -lpthread
	Include directory: /usr/include/gdk-pixbuf-2.0
Found MTP: /usr/lib/x86_64-linux-gnu/libmtp.so
Libgcrypt found: /usr/lib/x86_64-linux-gnu/libgcrypt.so
Found gmock and gtest but need to build both: /usr/include/gmock;/usr/src/gmock;GOOGLEMOCK_DEP_GTEST_SOURCES-NOTFOUND/gtest/include, GOOGLEMOCK_DEP_GTEST_SOURCES-NOTFOUND
Found X11: /usr/lib/x86_64-linux-gnu/libX11.so

-----------------------------------------------------------------------------
-- The following external packages were located on your system.
-- This installation will have the extra features provided by these packages.
-----------------------------------------------------------------------------
   * kdelibs - The toolkit Amarok uses to build
   * QtOpenGL - Required for the spectrum analyzer
   * mysqld - Embedded MySQL Libraries
   * mysql - MySQL Server Libraries
   * zlib - zlib
   * qca2 - Qt Cryptographic Architecture
   * QJson - Qt JSON Parser used for the Playdar Collection
   * liblastfm - Enable Last.Fm service, including scrobbling, song submissions, and suggested song dynamic playlists
   * ffmpeg - Libraries and tools for handling multimedia data
   * libofa - Enable MusicDNS service
   * libmygpo-qt - Enable gpodder.net service
   * libgpod - Support Apple iPod/iPad/iPhone audio devices
   * GDK-PixBuf - Support for artwork on iPod audio devices via GDK-PixBuf
   * libmtp - Enable Support for portable media devices that use the media transfer protocol
   * curl - cURL provides the necessary network libraries required by mp3tunes.
   * libxml2 - LibXML2 is an XML parser required by mp3tunes.
   * openssl or libgcrypt - OpenSSL or GNU Libgcrypt provides cryptographic functions required by mp3tunes.
   * loudmouth - Loudmouth is the communication backend needed by mp3tunes for syncing.
   * Qt4 Glib support - Qt4 must be compiled with glib support for mp3tunes
   * gobject - Required by libgpod and mp3tunes.
   * glib2 - Required by libgpod and mp3tunes
   * clamz - Optional requirement to download songs from the Amazon MP3 store. Highly recommended on Linux, as the official downloader from Amazon is quite broken on many systems.
   * Python - Required for generating the autocompletion file for the script console
   * gmock - Used in Amarok's tests.
   * Soprano - Soprano libraries required by Nepomuk Collection
   * taglib - Support for Audio metadata.
   * taglib - Additional support for Audio metadata of mod, s3m, it and xm files.
   * taglib - Additional support for Audio metadata of opus files.

-----------------------------------------------------------------------------
-- The following OPTIONAL packages could NOT be located on your system.
-- Consider installing them to enable more features from this software.
-----------------------------------------------------------------------------
   * nepomuk-core (4.9.0 or higher)  <http://kde.org/download/#v4.9>
     NepomukCore Libraries required by Nepomuk Collection

-----------------------------------------------------------------------------

Configuring incomplete, errors occurred!
See also "/home/rdragonrydr/working_amarok/build/CMakeFiles/CMakeOutput.log".
See also "/home/rdragonrydr/working_amarok/build/CMakeFiles/CMakeError.log".

Nepomuk is optional, and you do not need it. To understand the actual error, please review the complete cmake log file(s).

Created attachment 107274 [details]
Cmake log when encountering build error

I can't figure out what the error might be. I think everything was installed, but I had to somewhat guess, since apt-get build-dep does not seem to have a complete listing.

Can you tell what might be wrong?

Comment on attachment 107274 [details]
Cmake log when encountering build error

"CMake Error at tests/CMakeLists.txt:143 (add_subdirectory):
  add_subdirectory given source "GOOGLEMOCK_DEP_GTEST_SOURCES-NOTFOUND/gtest"
  which is not an existing directory."

not really hard to find, just search for the word "Error"

Thank you.

I have already installed google-mock and libgtest-dev, at very least. Would you know what package might provide the missing service?

It also lists them as present but in need of building?

-- Found gmock and gtest but need to build both: /usr/include/gmock;/usr/src/gmock;GOOGLEMOCK_DEP_GTEST_SOURCES-NOTFOUND/gtest/include, GOOGLEMOCK_DEP_GTEST_SOURCES-NOTFOUND

Thank you.

I have submitted the patch the reviewboard.kde.org, but I don't know what to do further. I am not a developer there, and don't really know much application coding. (I can write simple C programs, but don't know how to understand large projects like this one, aside from all the libraries and things used)

I have tested the patch; the AAC transcoding option now appears and I am fairly sure that I tested the output on my DSi to ensure that it works. The output file does play in a separate player on my computer.

I was unable to build the tests for the project, however. Perhaps this is why it has been on hold for the last 5.7 months. I installed gmock/gtest via my package manager, but have discovered that this only ships the source (and has been this way since Ubuntu 12 or 13!), NOT the binary that Amarok wants to build against.

How do I build gtest so that I can compile Amarok with tests enabled? I have to assume that this issue has since come up over at KDE with this change.
(I am guessing that "make test" will then run them?)

If that isn't the issue, how do I get the patch to move along?

I would try to get rid of the Ubuntu repo packages (libgtest-dev and google-mock) and use the upstream googletest:

cd <the directory you want to use>
git clone https://github.com/google/googletest.git
cd googletest/
mkdir build
cd build/
cmake -DCMAKE_INSTALL_PREFIX:PATH=/usr ../
make
sudo make install

Hopefully, Amarok will like this better.

I also hit an error with MySQL and something about -llz4 flag. I installed liblz4-dev and liblz4-tool and one of those took care of it.

I have followed the instructions in the two recent posts, the Github mock and the mySQL liblz4 ones.

I attempted to use the Github google mock rather than the built-in one, but I still got this error set (It seems to deal with libpthread):


Performing C++ SOURCE FILE Test COMPLEX_TAGLIB_FILENAME failed with the following output:
Change Dir: /home/rdragonrydr/working_amarok/build/CMakeFiles/CMakeTmp

Run Build Command:"/usr/bin/make" "cmTC_dc03d/fast"
/usr/bin/make -f CMakeFiles/cmTC_dc03d.dir/build.make CMakeFiles/cmTC_dc03d.dir/build
make[1]: Entering directory '/home/rdragonrydr/working_amarok/build/CMakeFiles/CMakeTmp'
Building CXX object CMakeFiles/cmTC_dc03d.dir/src.cxx.o
/usr/bin/c++    -I/usr/include/taglib  -DCOMPLEX_TAGLIB_FILENAME -I/usr/include/taglib   -o CMakeFiles/cmTC_dc03d.dir/src.cxx.o -c /home/rdragonrydr/working_amarok/build/CMakeFiles/CMakeTmp/src.cxx
/home/rdragonrydr/working_amarok/build/CMakeFiles/CMakeTmp/src.cxx: In function ‘int main()’:
/home/rdragonrydr/working_amarok/build/CMakeFiles/CMakeTmp/src.cxx:5:38: error: cannot convert ‘const wchar_t*’ to ‘TagLib::FileName {aka const char*}’ in initialization
   TagLib::FileName fileName2(L"wchar");
                                      ^
CMakeFiles/cmTC_dc03d.dir/build.make:65: recipe for target 'CMakeFiles/cmTC_dc03d.dir/src.cxx.o' failed
make[1]: *** [CMakeFiles/cmTC_dc03d.dir/src.cxx.o] Error 1
make[1]: Leaving directory '/home/rdragonrydr/working_amarok/build/CMakeFiles/CMakeTmp'
Makefile:126: recipe for target 'cmTC_dc03d/fast' failed
make: *** [cmTC_dc03d/fast] Error 2

Source file was:
#include <tfile.h>
	int main()
	{
		TagLib::FileName fileName1("char");
		TagLib::FileName fileName2(L"wchar");
		return 0;
	}
Determining if the Q_WS_WIN exist failed with the following output:
Change Dir: /home/rdragonrydr/working_amarok/build/CMakeFiles/CMakeTmp

Run Build Command:"/usr/bin/make" "cmTC_33044/fast"
/usr/bin/make -f CMakeFiles/cmTC_33044.dir/build.make CMakeFiles/cmTC_33044.dir/build
make[1]: Entering directory '/home/rdragonrydr/working_amarok/build/CMakeFiles/CMakeTmp'
Building CXX object CMakeFiles/cmTC_33044.dir/CheckSymbolExists.cxx.o
/usr/bin/c++    -I/usr/include/taglib -I/usr/include/qt4  -fmessage-length=0 -Wl,--as-needed -I/usr/include/taglib   -o CMakeFiles/cmTC_33044.dir/CheckSymbolExists.cxx.o -c /home/rdragonrydr/working_amarok/build/CMakeFiles/CMakeTmp/CheckSymbolExists.cxx
/home/rdragonrydr/working_amarok/build/CMakeFiles/CMakeTmp/CheckSymbolExists.cxx: In function ‘int main(int, char**)’:
/home/rdragonrydr/working_amarok/build/CMakeFiles/CMakeTmp/CheckSymbolExists.cxx:8:19: error: ‘Q_WS_WIN’ was not declared in this scope
   return ((int*)(&Q_WS_WIN))[argc];
                   ^
CMakeFiles/cmTC_33044.dir/build.make:65: recipe for target 'CMakeFiles/cmTC_33044.dir/CheckSymbolExists.cxx.o' failed
make[1]: *** [CMakeFiles/cmTC_33044.dir/CheckSymbolExists.cxx.o] Error 1
make[1]: Leaving directory '/home/rdragonrydr/working_amarok/build/CMakeFiles/CMakeTmp'
Makefile:126: recipe for target 'cmTC_33044/fast' failed
make: *** [cmTC_33044/fast] Error 2

File /home/rdragonrydr/working_amarok/build/CMakeFiles/CMakeTmp/CheckSymbolExists.cxx:
/* */
#include <QtCore/qglobal.h>

int main(int argc, char** argv)
{
  (void)argv;
#ifndef Q_WS_WIN
  return ((int*)(&Q_WS_WIN))[argc];
#else
  (void)argc;
  return 0;
#endif
}

Determining if the Q_WS_QWS exist failed with the following output:
Change Dir: /home/rdragonrydr/working_amarok/build/CMakeFiles/CMakeTmp

Run Build Command:"/usr/bin/make" "cmTC_55009/fast"
/usr/bin/make -f CMakeFiles/cmTC_55009.dir/build.make CMakeFiles/cmTC_55009.dir/build
make[1]: Entering directory '/home/rdragonrydr/working_amarok/build/CMakeFiles/CMakeTmp'
Building CXX object CMakeFiles/cmTC_55009.dir/CheckSymbolExists.cxx.o
/usr/bin/c++    -I/usr/include/taglib -I/usr/include/qt4  -fmessage-length=0 -Wl,--as-needed -I/usr/include/taglib   -o CMakeFiles/cmTC_55009.dir/CheckSymbolExists.cxx.o -c /home/rdragonrydr/working_amarok/build/CMakeFiles/CMakeTmp/CheckSymbolExists.cxx
/home/rdragonrydr/working_amarok/build/CMakeFiles/CMakeTmp/CheckSymbolExists.cxx: In function ‘int main(int, char**)’:
/home/rdragonrydr/working_amarok/build/CMakeFiles/CMakeTmp/CheckSymbolExists.cxx:8:19: error: ‘Q_WS_QWS’ was not declared in this scope
   return ((int*)(&Q_WS_QWS))[argc];
                   ^
CMakeFiles/cmTC_55009.dir/build.make:65: recipe for target 'CMakeFiles/cmTC_55009.dir/CheckSymbolExists.cxx.o' failed
make[1]: *** [CMakeFiles/cmTC_55009.dir/CheckSymbolExists.cxx.o] Error 1
make[1]: Leaving directory '/home/rdragonrydr/working_amarok/build/CMakeFiles/CMakeTmp'
Makefile:126: recipe for target 'cmTC_55009/fast' failed
make: *** [cmTC_55009/fast] Error 2

File /home/rdragonrydr/working_amarok/build/CMakeFiles/CMakeTmp/CheckSymbolExists.cxx:
/* */
#include <QtCore/qglobal.h>

int main(int argc, char** argv)
{
  (void)argv;
#ifndef Q_WS_QWS
  return ((int*)(&Q_WS_QWS))[argc];
#else
  (void)argc;
  return 0;
#endif
}

Determining if the Q_WS_MAC exist failed with the following output:
Change Dir: /home/rdragonrydr/working_amarok/build/CMakeFiles/CMakeTmp

Run Build Command:"/usr/bin/make" "cmTC_eb64a/fast"
/usr/bin/make -f CMakeFiles/cmTC_eb64a.dir/build.make CMakeFiles/cmTC_eb64a.dir/build
make[1]: Entering directory '/home/rdragonrydr/working_amarok/build/CMakeFiles/CMakeTmp'
Building CXX object CMakeFiles/cmTC_eb64a.dir/CheckSymbolExists.cxx.o
/usr/bin/c++    -I/usr/include/taglib -I/usr/include/qt4  -fmessage-length=0 -Wl,--as-needed -I/usr/include/taglib   -o CMakeFiles/cmTC_eb64a.dir/CheckSymbolExists.cxx.o -c /home/rdragonrydr/working_amarok/build/CMakeFiles/CMakeTmp/CheckSymbolExists.cxx
/home/rdragonrydr/working_amarok/build/CMakeFiles/CMakeTmp/CheckSymbolExists.cxx: In function ‘int main(int, char**)’:
/home/rdragonrydr/working_amarok/build/CMakeFiles/CMakeTmp/CheckSymbolExists.cxx:8:19: error: ‘Q_WS_MAC’ was not declared in this scope
   return ((int*)(&Q_WS_MAC))[argc];
                   ^
CMakeFiles/cmTC_eb64a.dir/build.make:65: recipe for target 'CMakeFiles/cmTC_eb64a.dir/CheckSymbolExists.cxx.o' failed
make[1]: *** [CMakeFiles/cmTC_eb64a.dir/CheckSymbolExists.cxx.o] Error 1
make[1]: Leaving directory '/home/rdragonrydr/working_amarok/build/CMakeFiles/CMakeTmp'
Makefile:126: recipe for target 'cmTC_eb64a/fast' failed
make: *** [cmTC_eb64a/fast] Error 2

File /home/rdragonrydr/working_amarok/build/CMakeFiles/CMakeTmp/CheckSymbolExists.cxx:
/* */
#include <QtCore/qglobal.h>

int main(int argc, char** argv)
{
  (void)argv;
#ifndef Q_WS_MAC
  return ((int*)(&Q_WS_MAC))[argc];
#else
  (void)argc;
  return 0;
#endif
}

Determining if the pthread_create exist failed with the following output:
Change Dir: /home/rdragonrydr/working_amarok/build/CMakeFiles/CMakeTmp

Run Build Command:"/usr/bin/make" "cmTC_804f1/fast"
/usr/bin/make -f CMakeFiles/cmTC_804f1.dir/build.make CMakeFiles/cmTC_804f1.dir/build
make[1]: Entering directory '/home/rdragonrydr/working_amarok/build/CMakeFiles/CMakeTmp'
Building C object CMakeFiles/cmTC_804f1.dir/CheckSymbolExists.c.o
/usr/bin/cc   -I/usr/include/taglib  -fmessage-length=0 -I/usr/include/taglib   -o CMakeFiles/cmTC_804f1.dir/CheckSymbolExists.c.o   -c /home/rdragonrydr/working_amarok/build/CMakeFiles/CMakeTmp/CheckSymbolExists.c
Linking C executable cmTC_804f1
/usr/bin/cmake -E cmake_link_script CMakeFiles/cmTC_804f1.dir/link.txt --verbose=1
/usr/bin/cc   -fmessage-length=0 -I/usr/include/taglib    CMakeFiles/cmTC_804f1.dir/CheckSymbolExists.c.o  -o cmTC_804f1 -rdynamic -L/usr/lib/x86_64-linux-gnu -ltag 
CMakeFiles/cmTC_804f1.dir/CheckSymbolExists.c.o: In function `main':
CheckSymbolExists.c:(.text+0x16): undefined reference to `pthread_create'
collect2: error: ld returned 1 exit status
CMakeFiles/cmTC_804f1.dir/build.make:97: recipe for target 'cmTC_804f1' failed
make[1]: *** [cmTC_804f1] Error 1
make[1]: Leaving directory '/home/rdragonrydr/working_amarok/build/CMakeFiles/CMakeTmp'
Makefile:126: recipe for target 'cmTC_804f1/fast' failed
make: *** [cmTC_804f1/fast] Error 2

File /home/rdragonrydr/working_amarok/build/CMakeFiles/CMakeTmp/CheckSymbolExists.c:
/* */
#include <pthread.h>

int main(int argc, char** argv)
{
  (void)argv;
#ifndef pthread_create
  return ((int*)(&pthread_create))[argc];
#else
  (void)argc;
  return 0;
#endif
}

Determining if the function pthread_create exists in the pthreads failed with the following output:
Change Dir: /home/rdragonrydr/working_amarok/build/CMakeFiles/CMakeTmp

Run Build Command:"/usr/bin/make" "cmTC_2313b/fast"
/usr/bin/make -f CMakeFiles/cmTC_2313b.dir/build.make CMakeFiles/cmTC_2313b.dir/build
make[1]: Entering directory '/home/rdragonrydr/working_amarok/build/CMakeFiles/CMakeTmp'
Building C object CMakeFiles/cmTC_2313b.dir/CheckFunctionExists.c.o
/usr/bin/cc    -fmessage-length=0 -DCHECK_FUNCTION_EXISTS=pthread_create -I/usr/include/taglib   -o CMakeFiles/cmTC_2313b.dir/CheckFunctionExists.c.o   -c /usr/share/cmake-3.5/Modules/CheckFunctionExists.c
Linking C executable cmTC_2313b
/usr/bin/cmake -E cmake_link_script CMakeFiles/cmTC_2313b.dir/link.txt --verbose=1
/usr/bin/cc   -fmessage-length=0 -DCHECK_FUNCTION_EXISTS=pthread_create -I/usr/include/taglib    CMakeFiles/cmTC_2313b.dir/CheckFunctionExists.c.o  -o cmTC_2313b -rdynamic -lpthreads -L/usr/lib/x86_64-linux-gnu -ltag 
/usr/bin/ld: cannot find -lpthreads
collect2: error: ld returned 1 exit status
CMakeFiles/cmTC_2313b.dir/build.make:97: recipe for target 'cmTC_2313b' failed
make[1]: *** [cmTC_2313b] Error 1
make[1]: Leaving directory '/home/rdragonrydr/working_amarok/build/CMakeFiles/CMakeTmp'
Makefile:126: recipe for target 'cmTC_2313b/fast' failed
make: *** [cmTC_2313b/fast] Error 2

I can integrate, test and commit your latest patch to fix AAC encoding. I have a question: what kind of encoder is the default "aac"? It would be nice to use libfdk_aac, but I know it's not always available, and not only for HE-AAC.

The primary patch is designed to still work with faac / libfaac (at least according to the person who wrote it), as far as I am aware. I am not certain, though, what encoder/decoder provides it, since all that FFMPEG reports is "aac".


(Meanwhile, I could not get the other, HE-FAAC patch to work for unknown reasons. Not for lack of Fdk-AAC, but because of some sort of issue with the patch itself. I am sorry I was unable to figure that one out.)

The patch you sent works, but it uses the -b:a parameter that means it's constant bitrate. I'm fine with this, I prefer constant bitrates as the documentation also says that VBR  is experimental, but then we should change the text. In addition, with the slider all on the left (8kb/s), the formula produced 0 instead of 8000.

Documentation of the encoder:
https://trac.ffmpeg.org/wiki/Encode/AAC

That's weird. I was not the one to write the original patch, so I am surprised to hear about that. I don't see a reason it should be 0bps, at least according to the code. Maybe 8k is too low?

I have learned more about how all this works ever since I posted this here, but I am still not very knowledgeable about how all of this works. The FFMPEG doc on that was helpful (and something I should have thought to look up. Oops).

I am still having cmake errors when trying to build with tests enabled, so I probably won't be much help there.

Did the HE-AAC patch work too (that's the one I could not get working)? I am rather less concerned about that one (I need regular AAC support), but I am curious.

Thank you for you work on integrating the changes. I look forward to having this working.

Yeah, the 0/8k thing is a mistake. Forgive me. Computer science classes were many years and many, many beers ago. Feel free to fix it, though I'm not sure why anyone would want AAC-LC files with such a low bitrate. My head hurts too much right now to think up an elegant solution. The lazy way would be to get rid of the 8k option and make it "Default", because ffmpeg will (sanely) default to 128k if you give it a 0 value for bitrate.

"What kind of encoder is the default "aac"?" 
It is LGPLv2. See: http://ffmpeg.org/index.html#aac_encoder_stable
If you meant something else by "what kind", please be more specific.

"The primary patch is designed to still work with faac / libfaac (at least according to the person who wrote it), as far as I am aware."
No, I said it would be nice if it did. But then, the only case where that would be useful is for people using really new versions of Amarok and really old versions of ffmpeg. In other words, not important enough for me to put coding effort into it.

Git commit 83c7357f0712aeeccbf083bc9462865f6acecf9e by Stefano Pettini.
Committed on 04/02/2018 at 22:24.
Pushed by spettini into branch 'master'.

Fixed transcoding to AAC format by switching to the latest 'aac' ffmpeg encoder
REVIEW: 130142

M  +25   -42   src/core/transcoding/formats/TranscodingAacFormat.cpp

https://commits.kde.org/amarok/83c7357f0712aeeccbf083bc9462865f6acecf9e