Juce 5 changes

[ferdnyc]

Adjustments to build libopenshot against a libopenshot-audio built with the new Juce 5.4.3 changes from my companion PR OpenShot/libopenshot-audio#37.

• All header files (7) which contained #include "JuceLibraryCode/JuceHeader.h" were changed to simply #include "JuceHeader.h", which works in concert with the new include dir layout for libopenshot-audio.
• Savaged the cmake/Modules/FindOpenShotAudio.cmake discovery code to remove all of the recursive header-file discovery and LIBOPENSHOT_AUDIO_BASE_DIR noise.
  Now it simply looks for the file JuceHeader.h and considers that location to be the LIBOPENSHOT_AUDIO_INCLUDE_DIR, which it is. So much simpler it's not even funny.

(Note: DO NOT MERGE until after the corresponding changes to libopenshot-audio are committed. See OpenShot/libopenshot-audio#37 .)

[jonoomph]

Merged into https://github.com/OpenShot/libopenshot/tree/juce5

[jonoomph]

@ferdnyc Looks like only Mac fails on the libopenshot juce5 branch:

Mac Output: gitlab-mac-build-output.txt

[ferdnyc]

@jonoomph Mm, so I see, looks like the Mac has a Point class of its own, that Juce is bumping up against. Probably just needs a namespace shoved in somewhere.

Could I see the output of a successful Mac build of the develop branch, though? It'd be helpful when comparing the two branches if I could also compare the output. Thx.

Oh, a copy of /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.9.sdk/usr/include/MacTypes.h would help as well (though if I dig I could probably find that somewhere on the web).

[jonoomph]

@ferdnyc Yeah, agreed.

Successful Mac Build: gitlab-mac-build-output.txt
MacTypes.h (from build server): https://github.com/OpenShot/libopenshot/files/3025845/MacTypes.zip

[jonoomph]

Okay, I think I fixed the Mac build issue with the following hack in JuceHeader.h (in libopenshot-audio): OpenShot/libopenshot-audio@10835cb

[jonoomph]

Based on my research, many people recommend to never include CoreFoundation.h directly, because it tends to typedef a ton of things with no namespace. JUCE is included this (when building on a Mac), which is defining Point prior to OpenShot defining a Point. OpenShot does not prefix every usage with openshot::Point, thus we end in a conflict.

So, this hack to just undefs the Mac Point class after JUCE includes it. So.... not beautiful, but seems to work fine.

[jonoomph]

Linux and Mac builds now work correctly (play audio and play video). Windows build plays no audio, so I'm digging into that now. I've had this problem in the past though, so I'm going to try the same basic approach as before: OpenShot/libopenshot-audio@29b42e6

[ferdnyc]

So, this hack to just undefs the Mac Point class after JUCE includes it. So.... not beautiful, but seems to work fine.

Makes sense, that's basically what I proposed in #189 to work around the RSHIFT redefinition — #define Ruby's to the name RB_RSHIFT, let FFmpeg apply their #define RSHIFT, #define that one over to be FF_RSHIFT, and then either #undef RSHIFT or (in the Ruby bindings) restore their definition with #define RSHIFT RB_RSHIFT. It's ugly but as long as you keep all the balls in the air, could be worse than having to juggle some #defines.

And with MacTypes.h being a strict C header, namespaces probably aren't going to solve it, so we're kind of stuck in the same boat here. (As the isfinite() issue showed, namespace prefixing won't even necessarily work to overcome #define clashes. Still don't understand that, but...)

[jonoomph]

Here is the output of openshot-audio-test-sound.exe (on Windows 10):

Old Version of JUCE (working / plays sound)

Begin init
Playing sound now
... 1
... 2
... 3
... 4
... 5
before device loop
Speakers (Realtek High Definition Audio) (Windows Audio)
Speakers (Realtek High Definition Audio) (Windows Audio (Exclusive Mode))
Primary Sound Driver (DirectSound)
Speakers (Realtek High Definition Audio) (DirectSound)
after device loop

New Version of JUCE (broken / no sound)

Begin init
Playing sound now
... 1
... 2
... 3
... 4
... 5
before device loop
Speakers (Realtek High Definition Audio) (Windows Audio)
Primary Sound Driver (DirectSound)
Speakers (Realtek High Definition Audio) (DirectSound)
after device loop

[jonoomph]

Perhaps I'll try and enable JUCE_WASAPI_EXCLUSIVE

[jonoomph]

Okay, now the output of openshot-audio-test-sound.exe is the same for the broken and working version, but still no audio is playing on Windows. Hmmm... I'll keep digging.

[ferdnyc]

@jonoomph
You could try switching to using AudioDeviceManager::initialiseWithDefaultDevices() instead — it only requires two args, numInputChannelsNeeded and numOutputChannelsNeeded, and "Resets everything to a default device setup, clearing any stored settings." which might give you a cleaner setup than AudioDeviceManager::initialise()? (If nothing else, you don't have to pass in all those other args you don't need.)

[ferdnyc]

There's also Juce's Tutorial: The AudioDeviceManager class — which is probably packaged in the release zip file as a ready-to-build project. Building and running that on Windows may give you an idea of whether it's a Juce issue, a configuration problem, or something in the libopenshot-audio code.

ETA: OK, well, it isn't in the zip file, but it is downloadable on the tutorial page.

[jonoomph]

After some additional debugging, it appears that there is no audio device open, and regardless of how I initialise (or initialiseWithDefaultDevices) the deviceManager still has no audio device open (and thus no sound is played). However, someone else who tested the juce5 build from yesterday, mentioned they did successfully play audio on Win 7 (32-bit). So, perhaps this is a bit more nuanced.

[ferdnyc]

Hmm. Could be that when multiple outputs are available, instead of opening a default choice on initialization, no device is chosen. Which also could be a change in Win10, compared to Win7.

(Perhaps Win10 is less aggressive about forcing a default when no selection is made? My Linux desktop technically has three audio outputs, and while some autodetection is possible — e.g. I don't have an HDMI cable connected, so that one's out — it's still a bit complex. Fortunately PulseAudio does have a systemwide "default" flag, because otherwise it'd be a real pain for software to correctly divine that I don't use the built-in sound card, so sound needs to be routed to the USB audio device instead.)

One solution might be a "Default Audio Output Device" selector, to go with the "Default Audio Sample Rate" and "Default Audio Channels" in Preferences > Profiles. It'd be nice if that could be avoided, of course, but it'd give users a way to set the output device if/when one doesn't get selected automatically.

[ferdnyc]

@jonoomph
Check out this info in the OBS Studio Wiki regarding Windows 10 audio routing. They're straight C++ Qt, no PyQt5 or Juce... but the entire thrust of that info seems to be that it doesn't matter what's going on inside the app — a lot more is handled by Win10 directly.

[jonoomph]

Got it working on Windows 10 (which reaffirms my dislike of Windows)! Apparently, JUCE was failing to initiate a WASAPI connection because my default playback device in Windows was 24-bit 48 kHz, and the associated microphone was 16-bit 48 kHz. Same sample rate, but different bit rates. Once those matched in Windows, audio played back just fine.

[jonoomph]

So, no more changes are required for libopenshot-audio for Windows 10. However, JUCE does return an error string during the audioDeviceManager.initialise() call (if it can not open an audio device). This would have made it very easy to debug had I seen this error. So, I'm going to experiment with ways to bubble this up to the user (or at the very least, log it).

[jonoomph]

I've added in a GetError() method on QPlayer, so we can access an error (if any is encountered during the initialise() method), and it will now bubble up in openshot-qt during launch. Not perfect, but much more communicative than before. 😋

[ferdnyc]

Sweet! Yeah, that's something that would be good to sort out more generally, e.g. for calls to FFmpeg library functions which can also fail... silently, as things currently stand, unless debugging mode is enabled. (I have an Issue open somewhere about that.)

That's kind of hilarious that it wouldn't open an output device because your input device is set to a different bit depth — when you're explicitly not even asking for any input channels!!

https://github.com/OpenShot/libopenshot-audio/blob/fbfac789c2cea4e8e85e8a60efcb61fbe1919d2c/src/Main.cpp#L48-L54

	// Initialize audio devices
	cout << "Begin init" << endl;
	AudioDeviceManager deviceManager;
	deviceManager.initialise (	0, /* number of input channels */
					2, /* number of output channels */
					0, /* no XML settings.. */
					true  /* select default device on failure */);

I can't even decide if that's Windows or JUCE being stupid. I guess there's room to blame both. It sort of feels like, while that situation should be handled better by Windows, since it's not the JUCE code could try to work around it instead of just erroring out.

[ferdnyc]

Aargh! I keep forgetting those don't work cross-repo. (Also, Github, it sucks that those don't work cross-repo. At least for repos with the same owner!)

[ferdnyc]

@jonoomph
So it sounds like libopenshot and libopenshot-audio are both in pretty good shape to merge the juce5 branches to develop at some point? (When they're tested and ready, of course.)

Once that happens, what are your thoughts on releasing the changes?

Easiest from my perspective (as Fedora maintainer) would be new releases of the libraries alone, without OpenShot. Then I could package OpenShot 2.4.4 for Fedora 30+ with dependencies on new libopenshot-0.2.4 and libopenshot-audio-0.1.9 (or -0.2.0?) packages built from those releases. But an OpenShot 2.4.5 release would mean I have to do package updates across the board even if there are no changes, so I'd prefer to avoid that. #BecauseLazy

If you wanted to hold off on new library releases for now, maybe address other issues or whatever, that's not a problem either. In the interim I can build F30 library packages from the repos' git HEAD. There's no policy requiring we package only release versions, it's just preferred as the path of least resistance under normal circumstances. (The GCC 9 breakage would of course constitute abnormal circumstances.)

(Oh, and at the risk of being a pest, any word on the cmake3 thing for Gitlab-CI?)

[jonoomph]

I'm waiting on the PPA to update before merging this into develop, since Travis will keep failing until the PPA is updated with the new libopenshot-audio.

[jonoomph]

This has been merged into develop with #209. I had to fix some Travis CI stuff, since trusty is no longer supported on our PPA. Travis is now on xenial.

[jonoomph]

@ferdnyc Regarding a new release, I would suggest building against HEAD if you are looking for the easiest and quickest solution. I think the earliest a new release would be possible is a couple weeks away, for the same reason: #BecauseLazy, haha

[ferdnyc]

@jonoomph Works for me! Fedora 30's still almost a month away, anyway. Whoops! Nope, more than a month away. Slipped another week.