OpenShot / openshot-qt

OpenShot Video Editor is an award-winning free and open-source video editor for Linux, Mac, and Windows, and is dedicated to delivering high quality video editing and animation solutions to the world.
http://www.openshot.org
Other
4.31k stars 533 forks source link

OpenShot-qt git master no longer compatible with libopenshot-0.1.9 #1371

Closed ferdnyc closed 6 years ago

ferdnyc commented 6 years ago

System Details:

Issue Description and steps to reproduce:

Attempting to test the current repo code from the source tree (using src/launch.py to start), I discovered that I was unable to import any files, always getting the "...is not a valid video, audio, or image file." error dialog.

Bisecting the source revealed commit 3accac8 as the culprit. After realizing that libopenshot had a corresponding commit OpenShot/libopenshot@f2b0f3a0f4 I built libopenshot from git and ran openshot-qt against that version. Importing was restored.

So, it looks like libopenshot needs to be rolled into a 0.1.10 release, and the minimum version updated in openshot-qt, because openshot-qt is no longer compatible with libopenshot-0.1.9 following commit 3accac8.

peanutbutterandcrackers commented 6 years ago

@ferdnyc - Do you think this addresses these (and future similar) issues? o.O

ferdnyc commented 6 years ago

With my test process, arguably maybe. You could argue that it was my error, as I should've been testing against the latest libopenshot sources.*

But in the general sense, no. Because src/classes/info.py still contains:

MINIMUM_LIBOPENSHOT_VERSION = "0.1.9"

And that's no longer true. That's the issue I'm reporting here. The libopenshot version needs to be incremented both in its development tree and, correspondingly, in that file.

That has to be done before the next OpenShot release. If OpenShot 2.4.2 claims to be compatible with libopenshot 0.1.9, then things will break for endusers just like they did for me. The version bump could be be done immediately after the previous release (the same way src/classes/info.py contains VERSION="2.4.1-dev1", and has since the 2.4.1 release), but if libopenshot doesn't release every time openshot-qt does (and it doesn't) then that wouldn't necessarily be desirable. But if it's not done automatically, then it's necessary once there's an incompatible API breakage, which has occurred as of the changes referenced above.


* ­– OpenShot makes testing against the devel tree rather difficult, though, by asking that the user install the library in order to use it. Endusers do not ever get to install software ad-hoc on my system, not even if they're me. That document instructs the user to run "make install" in the library trees, which will fail on any Linux system. And "sudo make install" (which would install in /usr/local/) is absolutely prohibited on any system I manage. I ended up rolling a new RPM package of the latest libopenshot and installing it in place of the official libopenshot-0.1.9, which worked in this case but risked breaking my real OpenShot install. A way of running OpenShot from the development tree, using the corresponding development libraries without installing them would be invaluable for testing. (Whether that's using something like LIBOPENSHOT_SRC=../libopenshot python3 src/launch.py to launch OpenShot, or if it means pulling the libopenshot and libopenshot-audio trees in as Git submodules of openshot-qt and having src/launch.py figure out the rest.)

peanutbutterandcrackers commented 6 years ago

I will bring this to Mr. Thomas's attention, ASAP. :)

DylanC commented 6 years ago

@ferdnyc - Has this been resolved by now?

jonoomph commented 6 years ago

I will bump this version on our next release, which should be any day now. This is an interesting problem though, because during development, sometimes we will break ABI, and of course, the solution is to always compile all 3 projects from source during development (libopenshot-audio, libopenshot, and openshot-qt), however, in many cases, some people use our PPAs for libopenshot-audio and libopenshot, and those can be a few hours delayed... or recently, there was a breakage in the PPA for a few days. Regardless, even if I had bumped the libopenshot version right after the last release, you would have still experience a breakage after this commit, if you didn't recompile libopenshot. So, like I said, it's an interesting problem, and I think during active development we shouldn't need to bump versions on every ABI breakages... but the result will be, occasional disagreements between libopenshot and openshot-qt on developers computers who haven't re-compiled the latest libopenshot. :+1:

ferdnyc commented 6 years ago

during development, sometimes we will break ABI, and of course, the solution is to always compile all 3 projects from source during development (libopenshot-audio, libopenshot, and openshot-qt)

Fair enough, but the bigger issue is that libopenshot and libopenshot-audio don't merely need to be compiled from source, but installed on the system — there's no way currently for launch.py to run OpenShot using libraries from a location other than the system default (e.g. from their own development trees), is there?

That would be a huge help to the development process, if it were possible — or, even better, if it was done by default when running from the development tree. (As I suggested above, libopenshot and libopenshot-audio would probably have to be more tightly coupled with the OpenShot sources in that case. For instance, by pulling them in as Git submodules.)

But even just a manual method of being able to run, say,

python3 src/launch.py LIBOPENSHOT_DIR=../libopenshot/build/ \
LIBOPENSHOT_AUDIO_DIR=../libopenshot-audio/build/

would go a long way towards making the testing process for development code smoother.

I think during active development we shouldn't need to bump versions on every ABI breakages Oh, I don't think every one, just the first one following each release.

If the above were possible (running OpenShot against non-installed libraries), but possibly even if not, I think it would make sense to push the library versions immediately on the first API breakage that makes the previous library release incompatible. That way, the version checking that OpenShot already performs would be able to inform development users of any incompatibilities, should they try to run OpenShot's development code against that release.

The problem isn't that the API has changed between releases. That's completely to be expected. It's that even the current Git HEAD for openshot-qt claims compatibility with libopenshot version 0.1.9 — and that's been flat-out false since 3accac8.

I mean, I suppose the other option would be to insist that development users always test using self-contained packages, instead of running against installed system libs at all. I believe @N3WWN has had success building AppImages, so that might be an option on Linux. Building OpenShot under Windows is a whole separate can of worms, of course, but that's sort of beyond this conversation.

musteresel commented 4 years ago

there's no way currently for launch.py to run OpenShot using libraries from a location other than the system default (e.g. from their own development trees), is there?

Of course there is: PYTHONPATH

PYTHONPATH=../libopenshot/build/src/bindings/python:$PYTHONPATH python3 src/launch.py

A bit more complex is the site module, which is what NixOS/the Nix package manager uses to achieve the same. That being said Nix (the package manger) is a really comfortable way to have multiple installations or automatically built different versions. Example:

with import (builtins.fetchGit {
  name = "nixpkgs-for-openshot-development";
  url = https://github.com/nixos/nixpkgs/;
  rev = "d7bbcdf274eaa3de1f813f391b64626b2f87bc49";
}) {};
# This above is only necessary if you run on an older nixpkgs .. like I am atm. Otherwise it's just:
# with import <nixpkgs> {};

rec {
  # libopenshot-audio in nixpkgs is old, does not yet set version in its header! need to use it 
  # from source. Could also use src = "https://...github...repo".
  los-audio = pkgs.libsForQt5.libopenshot-audio.overrideAttrs (old: {
    src = ./libopenshot-audio;
    name = "libopenshot-audio-dev";
  });
  # libopenshot which uses libopenshot-audio defined above.
  libopenshot = pkgs.libsForQt5.libopenshot.override {
    libopenshot-audio = los-audio;
  };
  # function to create a libopenshot package ... with the given revision (a commit hash) 
  # on the given branch (or tag). Default is to checkout tip of develop.
  los =
  { revision ? ""
  , branch ? "develop"}: libopenshot.overrideAttrs (old: {
    src = builtins.fetchGit {
      name = "libopenshot";
      url = ./libopenshot; # where I keep my repo; could also use https://github-repo-of-libopenshot
    } // lib.optionalAttrs (revision != "") {
      rev = revision;
    } // lib.optionalAttrs (branch != "") {
      ref = branch;
    };
    name = "libopenshot-dev"
      + lib.optionalString (revision != "") "-${revision}"
      + lib.optionalString (branch != "") "-${branch}";
    dontStrip = true;
    doCheck = true;
    cmakeFlags = [
     # Setting cmake flags
      "-DENABLE_RUBY=OFF"
      "-DUNITTEST++_INCLUDE_DIR=${unittest-cpp}/include/UnitTest++"
    ];
  });
}

With the above in libos.nix I run e.g. nix-build libos.nix -A los --arg revision '"d539e468"' to build and test revision https://github.com/OpenShot/libopenshot/commit/d539e468ca8934c7d72a20e817295c8af665b84a

Same works of course for building openshot with different libopenshot versions. Using an array one can also build several in parallel. Or use different versions for the dependencies (zeromq, python, ...). Lot's of possibilities :)

ferdnyc commented 4 years ago

there's no way currently for launch.py to run OpenShot using libraries from a location other than the system default (e.g. from their own development trees), is there?

Of course there is: PYTHONPATH

I know that now, I wrote that over 1.5 years ago.

Though, the points about building the project being a chore still stand. I've yet to succeed in actually building OpenShot from the ground up on Windows, and while it's mostly for lack of trying, the fact is that I've given up every time because it's simply not WORTH the effort.

I've been slowly, slowly, making it more worth it, by bringing our build system into at least the 20th century, but it's such incredibly slow going, and there are still so many hardcoded paths in the repos' build tooling that point to specific files in locations that are only valid on the Gitlab builders that generate the official builds. (Heck, until last year there were literal /home/jonathan/.. paths scattered everywhere — I think I've gotten rid of nearly all of those, except in the translation master files.)

ferdnyc commented 4 years ago

At the time I originally opened this bug, the only version checking in any part of the OpenShot ecosystem was OpenShot's MINIMUM_LIBOPENSHOT_VERSION gating, which at the time compared the string value stored there to openshot.GetVersion().ToString(), a representation of the library version structure that could only store a three-level integer version.

Since then, I've:

  1. Moved version management for libopenshot and libopenshot-audio into their CMake configs, so that the version headers are generated during the build instead of having to be manually updated.
  2. Added a canonical string representation of libopenshot's (and -audio's) extended version info, so that they can express the same extended -dev# versioning openshot-qt uses.
  3. Extended FindOpenShotAudio.cmake in the libopenshot build with version importing and checking, so that CMakeLists.txt can use find_package(OpenShotAudio VERSION <num>) to ensure the library it's linking is at least a minimum compatible version.

All of which is to say, it's now possible to ensure version compatibility all the way up and down the stack, and to make preemptive checks for any API breaks at either build or launch time instead of letting them manifest as runtime errors.

...But we still DON'T. Not for developers trying to work on the code, I mean.

In released versions, sure, the versions all get bumped right before release and everything will be properly gated. (Though we're always just one forgotten step away from putting out a release that doesn't include all of the proper checks.) Still releases are generally not a problem. My (continuing) issue is with how things are handled between releases.

After the release of OpenShot M.N.O / libopenshot X.Y.Z. (sometimes way after, because we forget), the development trees are updated to be version M.N.O-dev1 / X.Y.Z-dev1. But the version checks still use M.N.O / X.Y.Z. Which means that if you have a released version of libopenshot installed on your system, the development code will still claim to be compatible with it even after the API has changed so that it's NOT.

Which means that if you forget to set PYTHONPATH when running OpenShot from the development tree, or if you make a TYPO in your PYTHONPATH, or if you point it to a path that hasn't been fully built yet, Python will simply ignore the supplied PYTHONPATH and instead pick up the release version of the openshot module (linked to the release libopenshot.so) from the location it's installed to. And if the API has been broken since the release, you'll end up with a malfunctioning or crashing OpenShot because it failed to detect that it's running with an incompatible library version.

We couldn't easily do much about that before, but we CAN now. The development OpenShot code could list libopenshot X.Y.Z-dev1 as the minimum version after an API break, for starters. (At least the FIRST time it happens, so that incompatible release versions are no longer able to be used. But it wouldn't even be a bad idea to increment the dev counter every time an API-breaking change is committed. It's not like it affects the release numbering.)

ferdnyc commented 4 years ago

It's also questionable how, or even whether, a mechanism like PYTHONPATH can be used on Windows, when the Python module in question has compiled library components. PYTHONPATH on Linux relies on RPATH, and more than that on CMake's intelligent management of binary RPATHs in build trees, to link all of the components together without having to install them. But Windows AFAIK does not have a mechanism like RPATH for connecting binary objects to their dependencies scattered elsewhere around the system. So, even running OpenShot with libopenshot.dll from a build tree (if you succeed in building libopenshot in the first place) is significantly more difficult on Windows. @mkarg is currently struggling through those issues in OpenShot/libopenshot#359, like many who've come before.

And like everyone who came before, figuring most of it out as they go. We have some documentation on the process of building OpenShot for deployment. (Though I believe what we have is still incomplete or contains inaccuracies, particularly on the Windows and MacOS fronts.) But we don't have any information available on how to develop OpenShot and its components.

I suppose at this point that's on me, really. I've certainly complained about the situation enough that I have no excuse for not trying to do something to improve it. I've been focusing mostly on the build tooling recently, enhancing and modernizing the toolchain so that building and packaging the application or (especially) the libraries can hopefully be a little easier (and a lot more comprehensible/predictable) for anyone who's interested in doing that for themselves. But the two aren't mutually exclusive, and better developer docs are probably a more urgent priority at this point.

Even the basics of using PYTHONPATH to run from a build tree is never so much as mentioned anywhere in any of our documentation or resources. Anyone looking to develop OpenShot who wants to know how they can test their code has to hope that someone thinks to mention it to them, or they happen to stumble across a conversation like this one and discover it in passing.