search

ebu / ear-production-suite

The EAR Production Suite is a set of VST® plugins and tools for producing immersive and personalizable audio content suitable for any Next Generation Audio codec. It is based on the Audio Definition Model (ITU-R BS.2076) and the ITU ADM Renderer (ITU-R BS.2127) and enables monitoring on any ITU-R BS.2051 loudspeaker configuration.
https://ear-production-suite.ebu.io/
GNU General Public License v3.0
102 stars 20 forks source link

Stack overflow if AdmPresetDefinitionsHelper first inited on NNG thread #277

Closed firthm01 closed 6 months ago

firthm01 commented 6 months ago

The NNG threads have limited stack, and if AdmPresetDefinitionsHelper::getSingleton() is first called via an NNG message, it will overflow. This is because construction of the singleton runs a bunch of libadm stuff to parse the common definitions. The crash was observed in the binaural monitoring plugin.

Call stack:

    EAR Binaural Monitoring.vst3!__chkstk() Line 109    
    EAR Binaural Monitoring.vst3!adm::xml::XmlParser::parse() Line 43
    EAR Binaural Monitoring.vst3!adm::getCommonDefinitions() Line 163   
    EAR Binaural Monitoring.vst3!adm::parseXml(std::basic_istream<char,std::char_traits<char>> & stream, adm::xml::ParserOptions options) Line 18
    EAR Binaural Monitoring.vst3!AdmPresetDefinitionsHelper::AdmPresetDefinitionsHelper() Line 54
    EAR Binaural Monitoring.vst3!AdmPresetDefinitionsHelper::getSingleton() Line 65
    EAR Binaural Monitoring.vst3!ear::plugin::BinauralMonitoringBackend::onSceneReceived(ear::plugin::proto::SceneStore store) Line 228
    [External Code] 
    EAR Binaural Monitoring.vst3!ear::plugin::communication::MonitoringMetadataReceiver::handleReceive(std::error_code ec, nng::Message message) Line 65
    [External Code] 
    EAR Binaural Monitoring.vst3!nng::detail::AsyncReadAction::operator()() Line 261
    [External Code] 
    EAR Binaural Monitoring.vst3!nng::AsyncIO::callback() Line 227

The attached project crashes reliably on windows - tested on 2 machines. REAPER v7 (and i think the other machine was v6.something). IIRC, the stack is bigger on MacOS so probably won't crash. bin mon crash project.zip

I see two solutions.

  1. Bump the threads stack size. We've done this elsewhere by calling the message processing it in yet another thread so we're not using NNGs thread. SceneGainsCalculator for normal LS monitoring plugins - https://github.com/ebu/ear-production-suite/blob/77184d2a17594ca671925e80f1c5e2676c2f5419/ear-production-suite-plugins/lib/src/scene_gains_calculator.cpp#L57-L58
  2. Always instantiate AdmPresetDefinitionsHelper on main thread on plugin startup.

Maybe it makes sense for all NNG message callbacks to launch in their own thread? This is bound to trip us up again.

Tagging @rsjbailey for suggestions.

firthm01 commented 6 months ago

MonitoringMetadataReceiver::handleReceive is the last common point between the Binaural and LS monitoring plugins branch off. Perhaps this is where we launch the thread and remove it from SceneGainsCalculator?