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

Design flaw. Qt dependency in the shared lib that in not used for UI itself. #235

Open
SuslikV opened this issue May 12, 2019 · 5 comments

Comments

@SuslikV
Copy link
Contributor

SuslikV commented May 12, 2019

libopenshot library uses Qt for internal stuff, that makes it harder to build UI later (that itself could use Qt). The https://github.com/OpenShot/openshot-qt project is not linked directly to libopenshot itself too (just linked as dependent library, not as part of the code). It uses PyQt dependency that should be build using exactly the same options as were used during libopenshot compiling.

So, it makes impossible to use pre-compiled dependencies when building the application UI. It makes use of the shared library is very limited (the libopenshot will loose its compatibility as soon as you update your PyQt, for example, and thus it should be explicitly recompiled).

Just some of my thoughts about the project and its future. Multi-platform solutions is harder to build and maintain, so design flaw at the root will produce number of issues later.

@ferdnyc
Copy link
Contributor

ferdnyc commented Jun 1, 2019

PREFACE: (Just wait 'til you see how long this is! I outdid myself, in wasting everyone's time with my bloviating. ...I apologize in advance to anyone foolish enough to continue, and TL;DR: "...Why?")

libopenshot library uses Qt for internal stuff,

Well, define "internal". libopenshot uses Qt data structures and classes, yes, but they're not particularly internal. All of the image (pixel/bitmap) data is stored as QImages. It uses QColor for all of its color values. Qt is not merely a widget library for GUI development, it's a comprehensive framework for developing C++ (among other languages) applications. libopenshot could theoretically be implemented without Qt (well, except the parts that are literally Qt widget implementations, like src/Qt/VideoRendererWidget.cpp), but you'd have to do a lot more reinventing of the wheel.

that makes it harder to build UI later (that itself could use Qt).

There's also an argument to be made that using Qt within libopenshot makes it easier to build GUI code on top of the library, because (if your GUI is implemented in Qt) you can just directly access and pass around data using the exact same classes and data structures as in the library itself.

If you wanted to build a non Qt GUI, well then that's fine. But libopenshot will still be linked with Qt, and you'll still need access to Qt classes in your external codebase, because Qt is a dependency of libopenshot.

It'd be great if Qt were less monolithic, and libopenshot could be dependent on just the things it needs from Qt. But that's just not how it's set up. Which feels like a design flaw of Qt, more than anything else... no? 😉

Kidding aside, though... you'll have to sell me on how libopenshot's Qt dependency makes it harder to build a GUI for a libopenshot application, because I'm not seeing it.

It uses PyQt dependency that should be build using exactly the same options as were used during libopenshot compiling.

So, it makes impossible to use pre-compiled dependencies when building the application UI. It makes use of the shared library is very limited (the libopenshot will loose its compatibility as soon as you update your PyQt, for example, and thus it should be explicitly recompiled).

Now, that's definitely not true... or I'm misunderstanding you. What do you mean by "update PyQt"? We've updated the RPM Fusion PyQt packages at various times — it hasn't been an issue. They're built against the same version of Qt as the previous ones (and therefore the same version as libopenshot), so I wouldn't expect there to be any issues. And when we update Qt itself, all of its dependents have to be recompiled — including both PyQt and libopenshot — but that's fairly standard.

In terms of PyQt causing issues with libopenshot, though... well, the relationship between PyQt and Qt itself is not dissimilar from the relationship between libopenshot's Python bindings and Qt: they're both generated wrappers (using SIP for PyQt5, vs. SWIG for openshot) around a C++ library, exporting its APIs as a Python module. Because they occupy adjacent positions in the API stack, as long as they're sitting on a common foundation (the underlying Qt libraries) things should be fine. (I mentioned using the same version of Qt, earlier... well, that just seems like common sense, and I don't see why you ever wouldn't, so I'm not clear what the concern is there.) But I'm curious to hear what sort of real, observable problem the current design is causing; right now this feels like jumping at shadows.

(In terms of "It makes use of the shared library is very limited", there are existing users of libopenshot other than openshot-qt. I don't know any of the details, but I know @DylanC has a project that's dependent on libopenshot, but doesn't (AFAIK) make use of the Python bindings or PyQt.)

The https://github.com/OpenShot/openshot-qt project is not linked directly to libopenshot itself too (just linked as dependent library, not as part of the code).

Do you mean "linked" as in ELF/DLL shared library linking, or "linked" in the more general sense of them being structurally connected to each other? If it's the former... well, as Python code, openshot-qt isn't linked with anything. its dependencies are Python modules. Some of those are linked with various libraries; PyQt is one, libopenshot is another. Both provide Python modules with binary shared lib dependencies.

In terms of the latter... Look, the structure of the OpenShot project(s) — the organization of both the code itself and the development effort — is, IMHO, abysmal.

  • The libraries' CMake build definitions are a steaming pile of crap, much of which appears to have been written by someone with a very poor understanding of how CMake works and is meant to be used. As a result, they're structured disjointedly and bass-ackwards pretty much top-to-bottom.
  • And then the decision, in the openshot-qt project, to eschew CMake in favor of a hand-cobbled-together build based (loosely) around Python distutils and cx_Freeze, makes things even worse. There's no convenient way to run openshot-qt against an uninstalled libopenshot.so / openshot.py, which immensely complicates development and testing.
  • Beyond that, attempts to address those issues are hamstrung by the need for compatibility with the IMPOSSIBLY out-of-date builder environments in use on the builder hosts @ the project's Gitlab instance. The PRIVATE builders, that outside contributors can't even examine, access log files from, or really know WTF configuration is even in use.
  • @jonoomph 's insistence on building on a 5-year-old Ubuntu release, with no upgrades even to the build tools, absolutely infuriates me. I completely walked away from the project entirely at one point, over that exact issue. It means we're stuck having to write CMake build scripts compatible all the way back to CMake 3.1, which is just self-sabotage. And it's just completely unnecessary; there's no reason a newer version of CMake can't be installed on even 5-year-old shitty Ubuntu.

I could rant about this stuff all day long, but what's the point? It's not my project. I'm not even involved — mostly because, when I did see collaborator invite emails drop into my inbox a couple of months ago, I was already frustrated enough that I decided I didn't want to be more "officially" involved. I'd have to be an idiot to volunteer for that kind of stress. I'll chase bugs and write code for "fun", but you have to pay me to sign up for Additional Bullshit.

...But none of those issues are flaws in the project design. They're implementation issues, or organizational failings, or however else you want to frame them. They are things that, in theory, should be surmountable. (Though part of my frustration is the perceived lack of interest in addressing them, from anyone inside the project. Instead the development cycle still looks like #222: I submitted a PR to enhance the handling of Python install paths for the SWIG bindings. Three weeks later, it was merged. It broke those mystery Gitlab builders, in ways I couldn't possibly have predicted because I HAVE NO IDEA HOW THEY'RE SET UP, so it was panic-reverted 20 minutes later via #227. Like 5 hours after that, I submitted #229, with a fix to the issue that led to the revert of #222. Exactly one month minus one day later, #229 remains open, unmerged, and ignored.)

I don't even know what my point is here, actually. I kind of just lapsed into venting. You pick the one you like better, it's either:

  1. The project design may not be ideal, but the issues are mostly solvable with incremental changes; the problems people experience aren't inherent, they're just signs that more work needs to be put into the structure of the project trees and codebase, especially in terms of supporting developers who wish to examine, build, test, hack on, and contribute changes to what's supposedly an open-source development effort. (There are a still a bunch of /home/jonathan/... paths in all three projects' checked-in files, for instance. Which isn't a design flaw, it's just sloppy development.) But the acknowledged issues don't point to any unsolvable flaws, nor are there any obvious "right ways" to do the things the project needs to do but isn't doing, that are somehow precluded by its basic design. Not that I'm aware of, anyway.

  2. In a development effort where...

    • A PR with a one-line change can sit unacknowledged for weeks at a time
    • There's often zero feedback on submissions right up until the point where they get merged
    • Merged changes are subject to being insta-reverted if problems are encountered that are beyond the submitter's control, or even visibility

    what possible hope is there of making sweeping changes to the fundamental design of the project itself?

One of those sounds like it's probably the point I'm getting at; like I said, pick whichever you like better. I'm pretty sure they're both at least partially correct.

Just some of my thoughts about the project and its future. Multi-platform solutions is harder to build and maintain, so design flaw at the root will produce number of issues later.

I mean, the window for that sort of concern feels like it closed a long time ago. The project(s)'s code dates back to 2014, any issues that were going to come up "later" pretty much have to be here by now, right? It's already way past "later".

And that's part of it, too. It's hard to argue with success — it's just real easy to change the definition of success. "The openshot-qt/libopenshot/libopenshot-audio code is a joy to work on and incredibly easy to for a new developer to get involved with" is how every developer would like to define success, but that's because we're self-centered and expect our priorities to be the only priorities. But that's clearly not the primary concern of the project, and who are we to say it should be?

So I guess my question is still: what's this REALLY about? What's the actual issue you're having? It may be solvable, and (Occam's Razor) is unlikely to be the result of a fundamental design flaw in the project.

Your OP "complaints" (I use that word loosely and non-accusatively) are all very vague and theoretical, which to be blunt just isn't very compelling. Real, concrete problems would be more convincing. As things stand, this just kind of feels like #83, where the argument was that libopenshot is unnecessary and C++ is the devil, because it makes it hard for skript kiddies to hack on OpenShot. So why not just reimplement the entire project in pure Python, it sounds so simple and convenient and why worry about performance, it'll probably be fine! ...Or something like that. Who the fuck knows? That conversation got so stupid I just checked out completely.

There are plenty of times I've gotten frustrated with something in OpenShot — in some piece of code, in the build configs, in the basically-nonexistent facilities for debugging — and just felt like the only sane move would be to tear the whole thing out and start over. (I even kinda succeeded in doing that, once, with the translation-file loading. See OpenShot/openshot-qt#1795. Spoiler: It failed to bring me inner peace, and in fact ultimately resulted in significant inner not-peace. Not an experience I would wish to do over again.) Point is, I absolutely get the feeling of, "Oh, man, if I were writing this code, I'd have done things totally different!" I can 100% understand that argument.

...But I also realize that if I actually were writing the code, there probably wouldn't be any OpenShot. The code trees for openshot-qt+libopenshot+libopenshot-audio combined probably represent more lines of code than I've written, total, in my entire LIFE. And, certainly, I don't actually have a concrete plan for how I'd re-write the code, other than "not the way it's written now". I'd do it... some other way! A better way, obviously, duh! I'm sure I could probably figure out what that is, eventually. *shrug*

Not sure what my point is there, either. Other than, "it's easier to write code than read it", and throwing away the whole thing and starting over (to fix "design flaws") is "the single worst strategic mistake that any software company can make". Don't take my word for it, take Joel Spolsky's.

@SuslikV
Copy link
Contributor Author

SuslikV commented Jun 1, 2019

The spear of my word hit you too hard, man. Sorry, for this.

The issues already here: I've seen 3 types of Qt libraries that can be linked to libopenshot without issues and you'll be unable to load any function in openshot-qt because it will use the different ones (Windows builds). One of the recent changes in the code (OpenShot/openshot-qt#2742) made Preferences window not working (version of the libopenshot should be updated first). The last one is because you don't remember (and shouldn't) what was changed in the shared library.

Solution - _submodule_s of git and recursive cloning when obtaining code for building. Or your own functions to handle all differences between platforms and wiping out Qt from the libopenshot.

Edit: other disadvantage of using Qt is filtering functions of transform limited to bilinear.

@stale
Copy link

stale bot commented Mar 12, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale This issue has not had any activity in 90 days :( label Mar 12, 2020
@SuslikV
Copy link
Contributor Author

SuslikV commented Mar 13, 2020

Until bilinear scale issue wouldn't be solved - this issue will be opened. This is only Qt usage limitation in the project.

@stale stale bot removed the stale This issue has not had any activity in 90 days :( label Mar 13, 2020
@stale
Copy link

stale bot commented Jun 11, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale This issue has not had any activity in 90 days :( label Jun 11, 2020
@ctsdownloads ctsdownloads added enhancement and removed stale This issue has not had any activity in 90 days :( labels Jun 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants