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.

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

[WIP] AudioDSP fixes #9949

Closed
wants to merge 3 commits into from
Closed

Conversation

AchimTuran
Copy link
Member

@FernetMenta @popcornmix @MilhouseVH @AlwinEsch @fritsch
These are fixes to prevent Kodi crashing when adsp is enabled on RPi and probably Android (it's currently not tested from my side). I don't know why this has worked before on Windows, Linux and OS X because the speaker layout and buffer sizes could be wrong in some cases.
I already tested these fixes on Windows and it's fine.

But currently I stock, all video frames are dropped and I don't know why. Maybe adsp does something wrong with the picture time stamp. So it would be nice if @AlwinEsch, @FernetMenta or @fritsch could have a look at it.

@@ -436,9 +436,10 @@ bool CActiveAEBufferPoolResample::ResampleBuffers(int64_t timestamp)
if (!m_dspSample)
m_dspSample = m_dspBuffer->GetFreeBuffer();

m_dspSample->timestamp = in->timestamp;

This comment was marked as spam.

This comment was marked as spam.

@AchimTuran
Copy link
Member Author

jenkins build this please

@mithunkundu1983
Copy link

How to build kodi android on Travis

@popcornmix
Copy link
Member

While this fixes the crash on Pi, it doesn't make it work.
Just enabling ADSP (settings on default) causes the audio to go high pitched and scratchy.

mediainfo of test file: http://paste.ubuntu.com/17288193/
Debug log: http://paste.ubuntu.com/17290011/
Sample file: https://dl.dropboxusercontent.com/u/3669512/temp/Prometheus.Sample.mkv

@popcornmix
Copy link
Member

Also after playing the file then disabling ADSP I get a crash with
*** Error in /opt/xbmc-dbg/arm-linux-gnueabihf/lib/kodi/kodi.bin': free(): invalid pointer: 0x72612f67 ***`

@AchimTuran
Copy link
Member Author

@popcornmix

While this fixes the crash on Pi, it doesn't make it work.

I didn't say that the enitre problem is sovled with these fixes. Currently there is a problem with synchronization and it produces scratchy audio output. But the crashes on my Pi2 are gone.

Also after playing the file then disabling ADSP I get a crash with
*** Error in `/opt/xbmc-dbg/arm-linux-gnueabihf/lib/kodi/kodi.bin': free(): invalid pointer: 0x72612f67 ***

Could you create a backlog please?

@AchimTuran AchimTuran added the WIP PR that is still being worked on label Jun 13, 2016
@AchimTuran AchimTuran changed the title AudioDSP fixes [WIP] AudioDSP fixes Jun 13, 2016
@AchimTuran
Copy link
Member Author

Sorry I forgot to add the WIP label and title 😅

@popcornmix
Copy link
Member

Pretty sure the high pitched / scratchy audio is due to interpreting buffers with incorrect number of channels or format or planar.
I'll try to get a backtrace when I'm back on that branch. However for me just starting kodi and disabling ADSP will crash every time (I eventually had to disable the ADSP addons in add-on manager before I could disable in system/audio settings without crashing).

@popcornmix
Copy link
Member

This time I just clicked enable then disable on ADSP in system audio settings. This was crash:

Program terminated with signal SIGSEGV, Segmentation fault.
#0  std::_Rb_tree<ISubSettings*, ISubSettings*, std::_Identity<ISubSettings*>, std::less<ISubSettings*>, std::allocator<ISubSettings*> >::_M_erase (this=this@entry=0x5d8b758, __x=0xe1c120d0)
    at /home/dc4/tools/arm-bcm2708/arm-rpi-4.9.3-linux-gnueabihf/arm-linux-gnueabihf/include/c++/4.9.3/bits/stl_tree.h:1245
1245    /home/dc4/tools/arm-bcm2708/arm-rpi-4.9.3-linux-gnueabihf/arm-linux-gnueabihf/include/c++/4.9.3/bits/stl_tree.h: No such file or directory.
(gdb) bt
#0  std::_Rb_tree<ISubSettings*, ISubSettings*, std::_Identity<ISubSettings*>, std::less<ISubSettings*>, std::allocator<ISubSettings*> >::_M_erase (this=this@entry=0x5d8b758, __x=0xe1c120d0)
    at /home/dc4/tools/arm-bcm2708/arm-rpi-4.9.3-linux-gnueabihf/arm-linux-gnueabihf/include/c++/4.9.3/bits/stl_tree.h:1245
#1  0x00c8ff04 in std::_Rb_tree<ISubSettings*, ISubSettings*, std::_Identity<ISubSettings*>, std::less<ISubSettings*>, std::allocator<ISubSettings*> >::_M_erase (this=this@entry=0x5d8b758, __x=
    0x687e61ec <TSettingsElement<float>::set_Setting(float&)>) at /home/dc4/tools/arm-bcm2708/arm-rpi-4.9.3-linux-gnueabihf/arm-linux-gnueabihf/include/c++/4.9.3/bits/stl_tree.h:1245
#2  0x00c8ff04 in std::_Rb_tree<ISubSettings*, ISubSettings*, std::_Identity<ISubSettings*>, std::less<ISubSettings*>, std::allocator<ISubSettings*> >::_M_erase (this=this@entry=0x5d8b758, 
    __x=0x6880a348 <vtable for TSettingsElement<float>+8>) at /home/dc4/tools/arm-bcm2708/arm-rpi-4.9.3-linux-gnueabihf/arm-linux-gnueabihf/include/c++/4.9.3/bits/stl_tree.h:1245
#3  0x00c8f2d8 in clear (this=0x5d8b758) at /home/dc4/tools/arm-bcm2708/arm-rpi-4.9.3-linux-gnueabihf/arm-linux-gnueabihf/include/c++/4.9.3/bits/stl_tree.h:908
#4  clear (this=0x5d8b758) at /home/dc4/tools/arm-bcm2708/arm-rpi-4.9.3-linux-gnueabihf/arm-linux-gnueabihf/include/c++/4.9.3/bits/stl_set.h:670
#5  CSettingsManager::~CSettingsManager (this=0x5d8b6e0, __in_chrg=<optimized out>) at SettingsManager.cpp:42
#6  0x687d8dc0 in CBiquadFiltersSettings::~CBiquadFiltersSettings (this=0x6880b444 <CBiquadFiltersSettings::Get()::sSettings>, __in_chrg=<optimized out>)
    at /home/dc4/xbmc_holder/xbmc/tools/depends/target/binary-addons/arm-linux-gnueabihf/build/adsp.biquad.filters/src/BiquadFiltersSettings.cpp:52
#7  0x76173eec in __cxa_finalize (d=0x6880ab64) at cxa_finalize.c:56
#8  0x687d7cac in __do_global_dtors_aux () from /opt/xbmc-dbg/arm-linux-gnueabihf/lib/kodi/addons/adsp.biquad.filters/adsp.biquad.filters.so.0.0.2
#9  0x76fbd2a8 in _dl_close_worker (map=map@entry=0x5094e40) at dl-close.c:272
#10 0x76fbdebc in _dl_close (_map=0x5094e40) at dl-close.c:773
#11 0x764f7cbc in dlclose_doit (handle=<optimized out>) at dlclose.c:35
#12 0x76fb7e94 in _dl_catch_error (objname=0x76fb7e94 <_dl_catch_error+112>, errstring=0x76fcd56c, mallocedp=0x4290ba4, operate=0x4290ba0, args=0x5094e40) at dl-error.c:187
#13 0x764f82a8 in _dlerror_run (operate=0x764f7ca0 <dlclose_doit>, args=0x5094e40) at dlerror.c:163
#14 0x764f7cf0 in __dlclose (handle=<optimized out>) at dlclose.c:46
#15 0x0188c4c4 in SoLoader::Unload (this=0x5e479f8) at SoLoader.cpp:82
#16 0x0188b23c in DllLoaderContainer::ReleaseModule (pDll=@0x45360a8: 0x5e479f8) at DllLoaderContainer.cpp:218
#17 0x00d98a10 in CSectionLoader::UnloadDLL (dllname="/opt/xbmc-dbg/arm-linux-gnueabihf/lib/kodi/addons/adsp.biquad.filters/adsp.biquad.filters.so.0.0.2") at SectionLoader.cpp:97
#18 0x00d33604 in DllDynamic::Unload (this=0x58ca628) at DynamicDll.cpp:67
#19 0x00875b68 in ADDON::CAddonDll<DllAudioDSP, AudioDSP, AE_DSP_PROPERTIES>::Destroy (this=this@entry=0x54eba2c) at /home/dc4/xbmc_holder/xbmc/xbmc/addons/AddonDll.h:325
#20 0x008721d4 in ActiveAE::CActiveAEDSPAddon::Destroy (this=this@entry=0x54eba2c) at DSPAddons/ActiveAEDSPAddon.cpp:171
#21 0x00873f24 in ActiveAE::CActiveAEDSPAddon::~CActiveAEDSPAddon (this=0x54eba2c, __in_chrg=<optimized out>) at DSPAddons/ActiveAEDSPAddon.cpp:46
#22 0x0086a3cc in _M_release (this=0x54eba20) at /home/dc4/tools/arm-bcm2708/arm-rpi-4.9.3-linux-gnueabihf/arm-linux-gnueabihf/include/c++/4.9.3/bits/shared_ptr_base.h:149
#23 ~__shared_count (this=0x684f20c4, __in_chrg=<optimized out>) at /home/dc4/tools/arm-bcm2708/arm-rpi-4.9.3-linux-gnueabihf/arm-linux-gnueabihf/include/c++/4.9.3/bits/shared_ptr_base.h:666
#24 ~__shared_ptr (this=0x684f20c0, __in_chrg=<optimized out>) at /home/dc4/tools/arm-bcm2708/arm-rpi-4.9.3-linux-gnueabihf/arm-linux-gnueabihf/include/c++/4.9.3/bits/shared_ptr_base.h:914
#25 ~shared_ptr (this=0x684f20c0, __in_chrg=<optimized out>) at /home/dc4/tools/arm-bcm2708/arm-rpi-4.9.3-linux-gnueabihf/arm-linux-gnueabihf/include/c++/4.9.3/bits/shared_ptr.h:93
#26 ~pair (this=0x684f20b8, __in_chrg=<optimized out>) at /home/dc4/tools/arm-bcm2708/arm-rpi-4.9.3-linux-gnueabihf/arm-linux-gnueabihf/include/c++/4.9.3/bits/stl_pair.h:96
#27 _Destroy<std::pair<std::shared_ptr<ActiveAE::CActiveAEDSPMode>, std::shared_ptr<ActiveAE::CActiveAEDSPAddon> > > (__pointer=0x684f20b8)
    at /home/dc4/tools/arm-bcm2708/arm-rpi-4.9.3-linux-gnueabihf/arm-linux-gnueabihf/include/c++/4.9.3/bits/stl_construct.h:93
#28 __destroy<std::pair<std::shared_ptr<ActiveAE::CActiveAEDSPMode>, std::shared_ptr<ActiveAE::CActiveAEDSPAddon> >*> (__last=<optimized out>, __first=0x684f20b8)
    at /home/dc4/tools/arm-bcm2708/arm-rpi-4.9.3-linux-gnueabihf/arm-linux-gnueabihf/include/c++/4.9.3/bits/stl_construct.h:103
#29 _Destroy<std::pair<std::shared_ptr<ActiveAE::CActiveAEDSPMode>, std::shared_ptr<ActiveAE::CActiveAEDSPAddon> >*> (__last=<optimized out>, __first=<optimized out>)
    at /home/dc4/tools/arm-bcm2708/arm-rpi-4.9.3-linux-gnueabihf/arm-linux-gnueabihf/include/c++/4.9.3/bits/stl_construct.h:126
#30 _Destroy<std::pair<std::shared_ptr<ActiveAE::CActiveAEDSPMode>, std::shared_ptr<ActiveAE::CActiveAEDSPAddon> >*, std::pair<std::shared_ptr<ActiveAE::CActiveAEDSPMode>, std::shared_ptr<ActiveAE::CActiveAEDSPAddon> > > (__last=0x684f20d8, __first=0x684f20b8) at /home/dc4/tools/arm-bcm2708/arm-rpi-4.9.3-linux-gnueabihf/arm-linux-gnueabihf/include/c++/4.9.3/bits/stl_construct.h:151
#31 _M_erase_at_end (this=<optimized out>, __pos=0x684f20b8) at /home/dc4/tools/arm-bcm2708/arm-rpi-4.9.3-linux-gnueabihf/arm-linux-gnueabihf/include/c++/4.9.3/bits/stl_vector.h:1438
#32 clear (this=0x42b5b90) at /home/dc4/tools/arm-bcm2708/arm-rpi-4.9.3-linux-gnueabihf/arm-linux-gnueabihf/include/c++/4.9.3/bits/stl_vector.h:1212
#33 ActiveAE::CActiveAEDSP::Cleanup (this=this@entry=0x42b5a98) at DSPAddons/ActiveAEDSP.cpp:190
#34 0x0086cb90 in ActiveAE::CActiveAEDSP::Deactivate (this=0x42b5a98) at DSPAddons/ActiveAEDSP.cpp:172
#35 0x00d15cb8 in CApplication::OnApplicationMessage (this=0x41f9318, pMsg=0x58ab0d8) at Application.cpp:2420
#36 0x00ac95a4 in KODI::MESSAGING::CApplicationMessenger::ProcessMessage (this=this@entry=0x2dbd6c0 <KODI::MESSAGING::CApplicationMessenger::GetInstance()::appMessenger>, pMsg=pMsg@entry=0x58ab0d8)
    at ApplicationMessenger.cpp:232
#37 0x00ac9894 in KODI::MESSAGING::CApplicationMessenger::ProcessMessages (this=0x2dbd6c0 <KODI::MESSAGING::CApplicationMessenger::GetInstance()::appMessenger>) at ApplicationMessenger.cpp:205
#38 0x00d19288 in CApplication::Process (this=0x41f9318) at Application.cpp:4437
#39 0x00dbbb08 in CXBApplicationEx::Run (this=0x41f9318) at XBApplicationEx.cpp:102
#40 0x00ac7808 in XBMC_Run (renderGUI=<optimized out>) at xbmc.cpp:106
#41 0x0041d2e4 in main (argc=5, argv=0x7eaef544) at main.cpp:79

@AchimTuran
Copy link
Member Author

@popcornmix
Thanks that is very helpful. You have actived adsp.biqaud.filters, which has an issue during saving settings:
CBiquadFiltersSettings::~CBiquadFiltersSettings

The crash is not related to the AudioDSP system and is created inside an add-on. If you disable adsp.biquad.filters the crash should go away. Otherwise if you have the time please create another backlog.

@popcornmix
Copy link
Member

Okay, with biquad disabled in settings I no longer get the crash when disabling nor the distorted audio.
Audio sounds normal with either "Audio DSP Basic Processor" or "Free Surround Processor".
However I'm unable to adjust the settings. From system/audio settings menu it says you must adjust from OSD when playing video. However the "Audio DSP..." OSD setting has no effect.

@AchimTuran
Copy link
Member Author

AchimTuran commented Jun 13, 2016

@popcornmix

From system/audio settings menu it says you must adjust from OSD when playing video. However the "Audio DSP..." OSD setting has no effect.

I know that this is confusing for the users and I try to fix this in my new branch. Please let us focus on syncing audio with video. This is really broken on my RPi2. <-- Forget about that!

Edit:
Try to start playing a video- or music-file and the you can set settings from adsp.basic or adsp.freesurround.

@AchimTuran
Copy link
Member Author

@FernetMenta, @AlwinEsch, @popcornmix, @fritsch, @MilhouseVH
I started to dive very deeo into AudioDSP and found a lot of issues. So far I think this PR will not cover all required changes. I guess I have to create several PRs for the changes.

Latest changes can be followed in my repository in this branch.

Consequently this PR will be closed.

@AchimTuran AchimTuran closed this Jun 17, 2016
@@ -233,7 +233,7 @@ bool CActiveAEBufferPoolResample::Create(unsigned int totaltime, bool remap, boo
if (m_processor->GetChannelLayout().Count() > 2) /* Disable upmix for CActiveAEResample if DSP layout > 2.0, becomes perfomed by DSP */
upmix = false;

m_dspBuffer = new CActiveAEBufferPool(m_inputFormat); /* Get dsp processing buffer class, based on dsp output format */
m_dspBuffer = new CActiveAEBufferPool(m_format); /* Get dsp processing buffer class, based on dsp output format */

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@AchimTuran
Copy link
Member Author

Jenkins seems to be happy about my changes.

So far only XCode is missing because I don't own a Mac. Is there someone who can do the required changes. I moved the entire source code of AudioDSP from xbmc/cores/AudioEngine to xbmc/cores/AudioEngine/Engines/ActiveAE/AudioDSPAddons/.

@Memphiz
Copy link
Member

Memphiz commented Jun 18, 2016

i am on it ... why is this pr closed?

@Memphiz
Copy link
Member

Memphiz commented Jun 18, 2016

@AchimTuran something is wrong - the branch from this PR has not the mentioned changes - nothing to fix for me ...

@AchimTuran
Copy link
Member Author

@Memphiz sorry I forget to set the reference to PR #9994.

I closed this PR, because AudioDSP does have so many issues that it is not possible to create one PR for it.

I created a new branch in my fork and will do further development from there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: ADSP Platform: Android Platform: RPi Type: Fix non-breaking change which fixes an issue v17 Krypton WIP PR that is still being worked on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants