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

Drop QtMultimedia and QtMultimediaWidgets deps #551

Merged
merged 1 commit into from Aug 10, 2020

Conversation

ferdnyc
Copy link
Contributor

@ferdnyc ferdnyc commented Aug 10, 2020

Turns out, we don't actually use any of the classes in QtMultimedia (or its Widgets), so this PR drops them as dependencies.

This also prevents both Qt5Network and Qt5OpenGL getting pulled in as dependencies, and reduces our Qt dependency fingerprint significantly to just the three modules (now) listed in the CMakeLists.txt: QtCore, QtGui, and QtWidgets.

Shared libs being what they are, it only reduces the final size of libopenshot.so by about 33Kb, but the size isn't the point. (Besides, not loading four big Qt modules could give runtime a teeny tiny bit more pep, theoretically.)

# Before
$ ldd libopenshot.so.0.2.5 |& grep -i qt
	libQt5MultimediaWidgets.so.5 => /lib64/libQt5MultimediaWidgets.so.5 (0x00007f8911972000)
	libQt5Widgets.so.5 => /lib64/libQt5Widgets.so.5 (0x00007f89112cc000)
	libQt5Multimedia.so.5 => /lib64/libQt5Multimedia.so.5 (0x00007f89111e2000)
	libQt5Gui.so.5 => /lib64/libQt5Gui.so.5 (0x00007f8910c79000)
	libQt5Network.so.5 => /lib64/libQt5Network.so.5 (0x00007f8910af0000)
	libQt5Core.so.5 => /lib64/libQt5Core.so.5 (0x00007f89105d3000)
	libQt5OpenGL.so.5 => /lib64/libQt5OpenGL.so.5 (0x00007f890dfa7000)

# After
$ ldd libopenshot.so.0.2.5 |& grep -i qt
	libQt5Widgets.so.5 => /lib64/libQt5Widgets.so.5 (0x00007f8429fff000)
	libQt5Gui.so.5 => /lib64/libQt5Gui.so.5 (0x00007f842785c000)
	libQt5Core.so.5 => /lib64/libQt5Core.so.5 (0x00007f8427341000)

@ferdnyc ferdnyc added the build Issues related to compiling or installing libopenshot and its dependencies label Aug 10, 2020
@codecov
Copy link

codecov bot commented Aug 10, 2020

Codecov Report

Merging #551 into develop will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop     #551   +/-   ##
========================================
  Coverage    48.77%   48.77%           
========================================
  Files          129      129           
  Lines        10036    10036           
========================================
  Hits          4895     4895           
  Misses        5141     5141           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 729e349...44c29d5. Read the comment docs.

@ferdnyc
Copy link
Contributor Author

ferdnyc commented Aug 10, 2020

Committing, as these classes are undeniably not used in the code, so they serve no purpose in the build.

The only way this could cause problems is if we have some kind of implicit dependency on Qt5OpenGL or Qt5Network — and if so (it's highly unlikely), then good! Let's break things like that, so we find them now instead of it causing problems later. We can put anything here that's actually required back, but as a declared explicit dependency.

@ferdnyc ferdnyc merged commit 08550fa into OpenShot:develop Aug 10, 2020
@ferdnyc ferdnyc deleted the drop-qt-multimedia branch August 10, 2020 20:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues related to compiling or installing libopenshot and its dependencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant