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.

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

AE: drop null sink #13169

Merged
merged 1 commit into from Dec 11, 2017

Conversation

Projects
None yet
4 participants
@FernetMenta
Copy link
Member

FernetMenta commented Dec 11, 2017

see title

@FernetMenta FernetMenta added the Audio label Dec 11, 2017

@FernetMenta FernetMenta merged commit 66747ba into xbmc:master Dec 11, 2017

1 check failed

default Sorry, building this PR failed. Please check the logs for the errors.
Details

@FernetMenta FernetMenta deleted the FernetMenta:nullsink branch Dec 11, 2017

@Rechi Rechi added this to the L 18.0-alpha1 milestone Dec 11, 2017

@@ -20,8 +20,7 @@ set(SOURCES AEResampleFactory.cpp
Utils/AELimiter.cpp
Utils/AEPackIEC61937.cpp
Utils/AEStreamInfo.cpp
Utils/AEUtil.cpp

This comment has been minimized.

@fritsch

fritsch Dec 13, 2017

Member

This broke all sinks by accident, cause now no system.h was included anymore

This comment has been minimized.

@fritsch

fritsch Dec 13, 2017

Member

the one below this comment (obviously) -> therefore all the ifdefs ended up negative

This comment has been minimized.

@FernetMenta

FernetMenta Dec 13, 2017

Member

interesting, why did jenkins builds not fail?

This comment has been minimized.

@lrusak

lrusak Dec 13, 2017

Contributor

because it the function was still defined but it was the wrong function I guess

This comment has been minimized.

@FernetMenta

FernetMenta Dec 13, 2017

Member

what function? there is obviously something wrong if it build without this file

This comment has been minimized.

@fritsch

fritsch Dec 13, 2017

Member

AESinkNULL.cpp included system.h which then was available for all the others. Now as it is not there anymore all the ifdefs in the OptionalReg.cpp file evaluate to false and no sink is registered anymore.

This comment has been minimized.

@FernetMenta

FernetMenta Dec 13, 2017

Member

if AEUtil is only used by sinks, we should move it to sinks folder

This comment has been minimized.

@fritsch

fritsch Dec 13, 2017

Member

Not the case:

xbmc/Application.cpp:#include "cores/AudioEngine/Utils/AEUtil.h"
xbmc/GUIInfoManager.cpp:#include "cores/AudioEngine/Utils/AEUtil.h"
xbmc/addons/AudioDecoder.cpp:#include "cores/AudioEngine/Utils/AEUtil.h"
xbmc/cores/AudioEngine/Encoders/AEEncoderFFmpeg.cpp:#include "cores/AudioEngine/Utils/AEUtil.h"
xbmc/cores/AudioEngine/Engines/ActiveAE/ActiveAE.cpp:#include "cores/AudioEngine/Utils/AEUtil.h"
xbmc/cores/AudioEngine/Engines/ActiveAE/ActiveAEBuffer.cpp:#include "cores/AudioEngine/Utils/AEUtil.h"
xbmc/cores/AudioEngine/Engines/ActiveAE/ActiveAEResampleFFMPEG.cpp:#include "cores/AudioEngine/Utils/AEUtil.h"
xbmc/cores/AudioEngine/Engines/ActiveAE/ActiveAEResamplePi.cpp:#include "cores/AudioEngine/Utils/AEUtil.h"
xbmc/cores/AudioEngine/Engines/ActiveAE/ActiveAESink.cpp:#include "cores/AudioEngine/Utils/AEUtil.h"
xbmc/cores/AudioEngine/Engines/ActiveAE/ActiveAEStream.cpp:#include "cores/AudioEngine/Utils/AEUtil.h"
xbmc/cores/AudioEngine/Engines/ActiveAE/AudioDSPAddons/ActiveAEDSP.cpp:#include "cores/AudioEngine/Utils/AEUtil.h"
xbmc/cores/AudioEngine/Engines/ActiveAE/AudioDSPAddons/ActiveAEDSPProcess.cpp:#include "cores/AudioEngine/Utils/AEUtil.h"
xbmc/cores/AudioEngine/Interfaces/AESink.h:#include "cores/AudioEngine/Utils/AEUtil.h"
xbmc/cores/AudioEngine/Sinks/AESinkALSA.cpp:#include "cores/AudioEngine/Utils/AEUtil.h"
xbmc/cores/AudioEngine/Sinks/AESinkAUDIOTRACK.cpp:#include "cores/AudioEngine/Utils/AEUtil.h"
xbmc/cores/AudioEngine/Sinks/AESinkAUDIOTRACK.h:#include "cores/AudioEngine/Utils/AEUtil.h"
xbmc/cores/AudioEngine/Sinks/AESinkDARWINIOS.mm:#include "cores/AudioEngine/Utils/AEUtil.h"
xbmc/cores/AudioEngine/Sinks/AESinkDirectSound.cpp:#include "cores/AudioEngine/Utils/AEUtil.h"
xbmc/cores/AudioEngine/Sinks/AESinkOSS.cpp:#include "cores/AudioEngine/Utils/AEUtil.h"
xbmc/cores/AudioEngine/Sinks/AESinkPULSE.h:#include "cores/AudioEngine/Utils/AEUtil.h"
xbmc/cores/AudioEngine/Sinks/AESinkPi.cpp:#include "cores/AudioEngine/Utils/AEUtil.h"
xbmc/cores/AudioEngine/Sinks/AESinkSNDIO.cpp:#include "cores/AudioEngine/Utils/AEUtil.h"
xbmc/cores/AudioEngine/Sinks/AESinkWASAPI.cpp:#include "cores/AudioEngine/Utils/AEUtil.h"
xbmc/cores/AudioEngine/Sinks/AESinkXAudio.cpp:#include "cores/AudioEngine/Utils/AEUtil.h"
xbmc/cores/AudioEngine/Sinks/osx/AEDeviceEnumerationOSX.cpp:#include "cores/AudioEngine/Utils/AEUtil.h"
xbmc/cores/AudioEngine/Sinks/test/TestAESinkDARWINOSX.cpp:#include "cores/AudioEngine/Utils/AEUtil.h"
xbmc/cores/AudioEngine/Sinks/windows/AESinkFactoryWin.h:#include "cores/AudioEngine/Utils/AEUtil.h"
xbmc/cores/AudioEngine/Utils/AEDeviceInfo.cpp:#include "AEUtil.h"
xbmc/cores/AudioEngine/Utils/AEUtil.cpp:#include "AEUtil.h"
xbmc/cores/RetroPlayer/RetroPlayerAudio.cpp:#include "cores/AudioEngine/Utils/AEUtil.h"
xbmc/cores/VideoPlayer/DVDCodecs/Audio/DVDAudioCodec.h:#include "cores/AudioEngine/Utils/AEUtil.h"
xbmc/cores/VideoPlayer/DVDCodecs/Audio/DVDAudioCodecFFmpeg.cpp:#include "cores/AudioEngine/Utils/AEUtil.h"
xbmc/cores/VideoPlayer/VideoPlayerAudio.cpp:#include "cores/AudioEngine/Utils/AEUtil.h"
xbmc/cores/omxplayer/OMXAudio.h:#include "cores/AudioEngine/Utils/AEUtil.h"
xbmc/cores/omxplayer/OMXAudioCodecOMX.cpp:#include "cores/AudioEngine/Utils/AEUtil.h"
xbmc/cores/paplayer/PAPlayer.cpp:#include "cores/AudioEngine/Utils/AEUtil.h"
xbmc/cores/paplayer/VideoPlayerCodec.cpp:#include "cores/AudioEngine/Utils/AEUtil.h"
xbmc/settings/dialogs/GUIDialogAudioDSPSettings.cpp:#include "cores/AudioEngine/Utils/AEUtil.h"
xbmc/video/dialogs/GUIDialogAudioSubtitleSettings.cpp:#include "cores/AudioEngine/Utils/AEUtil.h"

AEUtils is perfectly fine :-) Again: AESinkNULL included system.h and made it available for everyone in scope of AE - nothing to do with AEUtils.

This comment has been minimized.

@FernetMenta

FernetMenta Dec 13, 2017

Member

The header with its definition might be used by other components but the body seems to be only used by sinks.

AEUtils is perfectly fine :-)

definitely not as long there is crap like CAESpinSection in this file

This comment has been minimized.

@fritsch

fritsch Dec 13, 2017

Member

Fernet :-) I will add "in the above regard" next time :-) I am also not sure if AEUtils affects "world peace" in a certain way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment