ZXTune integration

[vitamin-caig]

Hello!

I'm the author of ZXTune you are using in your project. Are you interested in recommendations about its better usage and making changes in order to achieve best performance/stability and make upstream merging easy?

[arnaud-neny]

Hi! Yeah sure, I always want to make it better :) Btw, thank you for the amazing work you did on ZXTune.

[vitamin-caig]

Great!:)

At first glance:

0. Consider using clang-cl as a compiler instead of MSVS - it's crossplatform and supports all the modern C++ features. The only problem I've met is slooooooow HighlyTheoretical core compilation, but I've managed to speed it up
1. Instead of making hard changes in zxtune code, use #if 0 ... (optional #else ... ) #endif to reduce further conflicts while merge
2. You may create your own plugins_*.cpp files with required subset of plugins to register and include it to build instead of regular one (I use the same technique for android builds with reduced archive/compressor plugins set)
3. Do not use Binary::CreateContainer(data, size) - it copies input data, just implement Binary::Data interface over you blob data type and then call Binary::CreateContainer(Data::Ptr)
4. Do not call ZXTune::Service::Create for every ReplayZXTune::Load call - it's quite heavy, just make it singletone
5. Simplify ReplayZXTune::Render - no need to control loop (or I don't understand what it's for) and calculate channels count every chunk - moreover, seems like it's not the information anybody needs:)
6. No need to recreate renderer - it has Reset method, so also no need to keep Holder
7. Very strange code in ReplayZXTune::Init :)

[vitamin-caig]

8. Define NO_L10N and delete boost/locale dependency
9. Define NO_DEBUG_LOGS and may revert some changes in debug calls
10. Not sure you need platform and resource libraries

[arnaud-neny]

Yeah, Thanks for the info: I did a quick and dirty integration and never looked at improving it.

1. I will not change the compiler to clang (at least, yet), and that's why I had to do some changes in yourr code because you are using some non standard c++.
2. :thumbsup: (another lazy update to help me see if there are new plugins while merging)
3. :thumbsup:
4. :thumbsup:
5. to explain, the control loop is there to notify the player the replay has looped (it's working the same way as openmpt, returning 0 renderered samples means the song has looped or ended).
6. :thumbsup:
7. :D hook entries for rePlayer
8. :thumbsup:
9. :thumbsup:
10. I had to add these to build zxtune at the time. Maybe it could be gone with some defines.

[vitamin-caig]

0. AFAIK, the only non-standard feature used is template literals to support compile-time strings concatenation - it significantly reduced binary size, actual for android.
1. zxtune returns empty data if playback is finished and no looping allowed
2. I mean you may just do something like:

   std::set<std::string> extensions;
   for (const auto& plugin : ... ) {
   extensions.insert(Strings::ToLowerAscii(plugin->Id()));
   }
   const auto buffer = Strings::Join(extensions, ";"_sv) + ";szx;zip...";
   g_replayPlugin.extensions = strdup(buffer.c_str());

[arnaud-neny]

Thanks for the suggestions, it reduced the repo and the ouput runtime size.

[vitamin-caig]

Do not make ms_service static class variable:

// ReplayZXTune.cpp
namespace {
ZXTune::Service& GetService() {
  static const auto instance = ZXTune::Service::Create(Parameters::Container::Create());
  return *instance;
}

...
GetService().DetectModules(...)

This will postpone service creation till first replay usage. The same technique may be used for another singletons.