Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fdk-aac encoder + libshout-idjc + vcpkg Windows dependencies integration #3615

Merged
merged 58 commits into from
Feb 22, 2021
Merged

fdk-aac encoder + libshout-idjc + vcpkg Windows dependencies integration #3615

merged 58 commits into from
Feb 22, 2021

Conversation

Be-ing
Copy link
Contributor

@Be-ing Be-ing commented Feb 4, 2021

This is a rebased combination of #3543 and #3594 which enables AAC encoding on Windows, macOS, and Linux.

Stéphane L and others added 30 commits January 8, 2021 23:51
@daschuer
fdk-aac encoder
029b405
@daschuer
Renamed libshout folder to libshout-idjc
c844c85
@daschuer
update to libshout-idjc-2.4.1
4ec8ceb
@daschuer
fix loading of libfdk-aac if no symlink without a version is provided
ff9335e
@daschuer
fix line break
77c557a
@daschuer
fix include files
ee26074
@daschuer
Enable internal libshout-idjc for all targets
dcf9895
@daschuer
clazy fixes
cf3188f
@daschuer
Fix indent
afe65d2
@daschuer
Don't use config.h configure by cmake instead, taken from https://git…
b628f61 
…hub.com/Holzhaus/icecast-libshout
@daschuer
fix os.h
ad5a511
@daschuer
Remove unsused strings.h
20ba8ca
@daschuer
replace illegal void* arythmetric
d7530f5
@daschuer
Try loading libfdk-aac.so.2 first
d11fdc8
@daschuer
Avoid possible string truncation.
85450b5
@daschuer
Avoid extra buffer for sample conversion
d346048
@daschuer
link libshout-idjc against pthread
282e838
@daschuer
Link against pthread on windows
8819241
@daschuer
Fix file extensions during recording.
1387cbc
@daschuer
Merge remote-tracking branch 'upstream/2.3' into fdkaac-encoder3
597b79f
@daschuer
Fix linking to system installed libshout-idjc
be0b0d2
@daschuer
Merge remote-tracking branch 'upstream/2.3' into fdkaac-encoder3
82ec841
@daschuer
Re-enabled Quality slider for AAC recording
9557eee
@daschuer
Avoid log spam when probing fdkaac libraries
5843994
@daschuer
Check return value from aacEncoder_SetParam
3408a2b
@daschuer
Check for SBR support
7fd6657
@daschuer
Replace QString::null
3c75744
@daschuer
Improve error handling when initalize the encoders for recording or b…
171fd9d 
…roadcasting
@daschuer
Prevent double message box in case of recording issues.
3968d81
@daschuer
Fix second pop up in case of encoder issues during broadcasting.
0fd0429
@Be-ing
Copy link
Contributor Author

Be-ing commented Feb 17, 2021

Now with TagLib 1.12 on Windows and macOS

uklotzde
uklotzde reviewed Feb 17, 2021
View reviewed changes
Copy link
Contributor

@uklotzde uklotzde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one minor remark where I think an additional comment would help.

The complexity and sheer amount of code is not caused by this PR that just tries to tie everything together. Even if it might (or almost certainly will) cause unforeseeable issues that we missed during the review, I expect those will be fixable. We urgently need some real world tests of this code.

First I was skeptical about switching to the idjc for. But now I consider it the lesser evil compared to the original libshout.

I trust you on the statement that the dynamic loading works on all platforms.

CMakeLists.txt Show resolved Hide resolved
CMakeLists.txt Show resolved Hide resolved
CMakeLists.txt
elseif(FDK_AAC_LIBRARY)
message(STATUS "Found fdk-aac: ${FDK_AAC_LIBRARY}")
else()
message(STATUS "Could NOT find fdk-aac")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am also against repeating the inner workings of the find algorithm in colloquial language, maybe even with platform-dependent wording. This message will only ever be seen by developers and is sufficient.

@Be-ing
Copy link
Contributor Author

Be-ing commented Feb 18, 2021

Merge?

uklotzde
uklotzde approved these changes Feb 18, 2021
View reviewed changes
Copy link
Contributor

@uklotzde uklotzde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Holzhaus @daschuer Do you agree?

I would like to merge this as a baseline and resolve any open issues that might occur later. I expect that reviewing individual improvements would be easier and hopefully less controversial afterwards.

@Be-ing
Copy link
Contributor Author

Be-ing commented Feb 18, 2021

The longer this sits open, the more will pile on top.

@Be-ing
Copy link
Contributor Author

Be-ing commented Feb 21, 2021

ping

@Be-ing
Copy link
Contributor Author

Be-ing commented Feb 22, 2021

Is someone going to press merge or are we going to delay this release indefinitely?

@Holzhaus Holzhaus merged commit 79471f1 into mixxxdj:2.3 Feb 22, 2021
@daschuer daschuer mentioned this pull request Feb 22, 2021
Merged
@Be-ing Be-ing deleted the fdk-aac_rebase branch February 24, 2021 18:20
@Be-ing Be-ing mentioned this pull request Feb 25, 2021
Closed
7 tasks
@Be-ing Be-ing mentioned this pull request May 9, 2021
Closed
@daschuer daschuer mentioned this pull request Jun 10, 2021
Merged
@Be-ing Be-ing mentioned this pull request Jun 28, 2021
Merged
This was referenced Aug 22, 2022
Closed
Closed
Closed
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JoergAtGithub JoergAtGithub JoergAtGithub left review comments

@daschuer daschuer daschuer left review comments

@uklotzde uklotzde uklotzde approved these changes

@Holzhaus Holzhaus Awaiting requested review from Holzhaus

Assignees
No one assigned
Labels
build code quality library ui
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@Be-ing @daschuer @uklotzde @JoergAtGithub @JosepMaJAZ @Holzhaus