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

Add org.videolan.VLC.Plugin.pause_click #3843

Closed

Conversation

nurupo
Copy link

@nurupo nurupo commented Jan 23, 2023

Please confirm your submission meets all the criteria

  • I have read the App Requirements and App Maintenance pages.
  • My pull request follows the instructions at App Submission.
  • I am using only the minimal set of permissions. (If not, please explain each non-standard permission.)
  • All assets referenced in the manifest are redistributable by any party. If not, the unredistributable parts are using an extra-data source type.
  • I am an upstream contributor to the project. If not, I contacted upstream developers about submitting their software to Flathub. Link:
  • I own the domain used in the application ID or the domain has a policy for delegating subdomains (e.g. GitHub, SourceForge).
  • Any additional patches or files have been submitted to the upstream projects concerned. (If not, explain why.)

Hi, I'm the author of the Pause Click VLC plugin. Some users expressed their desire to be able to use the plugin with the flatpak version of VLC, so I have put some effort into making a flatpak for the plugin and adding it to Flathub.

I have tested that the flatpak builds successfully locally and, upon installation, the plugin appears in the VLC, fully functioning. The appdata file was validated with org.freedesktop.appstream-glib validate and I have also added x-checker-data to the json, which hopefully works as I didn't test it.

I'm new to Flatpak so please tell me if I'm doing something wrong or missing something.

The addon uses id of org.videolan.VLC.Plugin.pause_click. I'm not a VLC developer, so I have no relation with the domain. I wanted to use the repository URL for the application id, i.e. com.github.nurupo.vlc-pause-click-plugin, but it seems like due to the way addons work in Flatpak this is not possible. The VLC flatpak exposes org.videolan.VLC.Plugin as the extension point, so it seems like the addon must use org.videolan.VLC.Plugin. for its id, using other ids didn't work for me.

The plugin requires libvlccore.so ABI-compatible with the version of VLC the plugin is going to be used, libvlc and libvlccore headers, and pkg-config file vlc-plugin.pc to be present in order for the plugin to get built. All of these are generated during the org.videolan.VLC build, but are not present in the resulting VLC flatpak. Ideally I want to build the plugin against the VLC version the user would be using it with, but since the VLC sdk is not provided by the org.videolan.VLC flatpak, I have to resort to building a very stripped-down but ABI-compatible version of VLC myself, in the addon manifest. It would be nice if org.videolan.VLC was modified to provide org.videolan.VLC.SDK sdk extension, so that I could add it to my sdk-extensions and not have to build VLC in my addon manifest.

By the way, is there some guide on how to maintain the repository once it's created? From the looks of what other VLC plugins do, it looks like I need to create a new branch/<version> for each version of org.videolan.VLC.Plugin I want to support, and whenever I release a new version of vlc-pause-click-plugin, I need to update the json in all of these branches to use than new version?

@nurupo
Copy link
Author

nurupo commented Jan 23, 2023

bot, build org.videolan.VLC.Plugin.pause_click

@flathubbot
Copy link

Queued test build for org.videolan.VLC.Plugin.pause_click.

@flathubbot
Copy link

Started test build 18948

@flathubbot
Copy link

Build 18948 failed

@nurupo
Copy link
Author

nurupo commented Jan 23, 2023

zgrep "<icon type="remote">" builddir//share/app-info/xmls/org.videolan.VLC.Plugin.pause_click.xml.gz || test -f builddir//share/app-info/icons/flatpak/128x128/org.videolan.VLC.Plugin.pause_click.png

It's an addon, it produces a libpause_click_plugin.so that VLC loads when started, it doesn't need an icon or a desktop file or anything like that. Other VLC plugins don't have an icon either: org.videolan.VLC.Plugin.makemkv.json, org.videolan.VLC.Plugin.bdj.json.

I guess an icon might be nice for displaying it in Flathub, but I don't have any icon for the plugin to begin with.

@hfiguiere
Copy link
Contributor

Other add-ons add skip-icon-check to the flathub.json like that:

https://github.com/flathub/org.freedesktop.LinuxAudio.Plugins.GuitarML/blob/branch/22.08/flathub.json#L2

@nurupo
Copy link
Author

nurupo commented Jan 23, 2023

Thanks for the tip!

@nurupo
Copy link
Author

nurupo commented Jan 23, 2023

bot, build org.videolan.VLC.Plugin.pause_click

@flathubbot
Copy link

Queued test build for org.videolan.VLC.Plugin.pause_click.

@flathubbot
Copy link

Started test build 18949

@flathubbot
Copy link

Build 18949 successful
To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/1574/org.videolan.VLC.Plugin.pause_click.flatpakref

Some users might not read the Flathub page for the plugin which contains
the instructions and be furstrated as to why the plugin doesn't work.
While printing this on every run is noisy, I think the benefits
overweight the annoyance.
@nurupo nurupo force-pushed the org.videolan.VLC.Plugin.pause_click branch from a8a3be6 to a817dd3 Compare January 23, 2023 22:09
@nurupo
Copy link
Author

nurupo commented Jan 23, 2023

Cleaned up the PR a bit, squashing a few commits together.

bot, build org.videolan.VLC.Plugin.pause_click

@flathubbot
Copy link

Queued test build for org.videolan.VLC.Plugin.pause_click.

@flathubbot
Copy link

Started test build 19063

@flathubbot
Copy link

Build 19063 successful
To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/1694/org.videolan.VLC.Plugin.pause_click.flatpakref

@nurupo
Copy link
Author

nurupo commented Jan 24, 2023

VLC flatpak accepts extensions only of specific version:

https://github.com/flathub/org.videolan.VLC/blob/0e6abb5a01a1fe331a2916b10d9798c08a6ce22c/org.videolan.VLC.json#L25-L26

The version is used to enforce that the extension is built against the same VLC version, e.g. 3, optionally, KDE SDK, e.g. 5.15, and org.freedesktop.Sdk, e.g. 22.08. This all makes sense ABI compatibility wise, as these are the versions of things that are in the VLC flatpak container at run-time.

However, what I don't quite understand is: how does the update process work with the version requirement?

Here is a hypothetical example (which already happened a few times and will happen in the future):

VLC has only the stable branch. One day a stable VLC flatpak update would bump the require extension version, e.g. from 3-22.08 to 3-23.08. When a user updates their VLC to that new version, what happens to the installed extensions?

  • Do the 3-22.08 extensions just stop working and VLC launches extension-less?
  • What if at the time of VLC update an extension already had a 3-23.08 branch published on flathub, but the user had 3-22.08 installed?
    • Would updating VLC also result in the extensions being automatically updated to 3-23.08 branch, so that once VLC is updated all the extensions would continue working as expected?
    • Or perhaps updating VLC won't update any extensions, so none of them would get loaded and the user would be very frustrated as to why things broke with the new VLC update?
      • If so, would at least running flatpak update tell the user that there is a 3-23.08 branch now available for the installed extension and ask the user if they want to update to it?
      • Or running flatpak update won't tell the user anything and they would have to find it the hard way (or never find out at all!) that they should manually install the 3-23.08 branch of the extension in order to restore the previous functionality?
  • What if at the time of VLC update an extension doesn't have a 3-23.08 branch published on flathub, so the user has just 3-22.08 installed, but then, a month later, the extension published a 3-23.08 branch? Would the user notice this? Would the extension automatically update to the new branch?

I'm kind of fearing the worst outcome, that when a user updates to a VLC requiring new extension version, all installed extension would stop working, and the user wouldn't know how to fix this as flatpak won't auto-update extensions and won't tell the user that the user needs to install new branches of the extensions.

Would appreciate if you could explain what actually happens in such situation.

CC: @MatMaul

@barthalion
Copy link
Member

Or perhaps updating VLC won't update any extensions, so none of them would get loaded and the user would be very frustrated as to why things broke with the new VLC update?

That's what going to happen. It will remain broken until someone bumps it. Ideally, it should be the VLC maintainer. There's also non-zero chance the plugin will remain loadable but it depends on the code base and libraries changed in the newer runtime.

/merge @MatMaul

@barthalion
Copy link
Member

/merge:3-22.08 @MatMaul

@flathubbot
Copy link

A repository for this submission has been created: https://github.com/flathub/org.videolan.VLC.Plugin.pause_click

You will receive an invitation to be a collaborator which will grant you write access to the repository above. The invite can be also viewed here.

If you have never maintained an application before, common questions are answered in the app maintenance guide.

Thanks!

@flathubbot flathubbot closed this Jan 27, 2023
@MatMaul
Copy link

MatMaul commented Jan 27, 2023

Thanks for this plugin, and for taking the time to make a flatpak plugin, that is really appreciated.

I am usually trying to properly bump the plugins at the same time I bump the VLC plugin ABI, like I did for the 22.08 runtime. If I forget, feel free to ping me or send a PR.

Main trouble is that this use case is not well supported by flatpak tooling, and up to my knowledge the new branch of the plugins will not be automatically installed when I update the plugin version target in VLC manifest.
The user needs to install the plugins again, with the good branch. It should probably be handled at flatpak update level.

@hfiguiere
Copy link
Contributor

The user needs to install the plugins again, with the good branch. It should probably be handled at flatpak update level.

I filed this a while ago flatpak/flatpak#4208

@nurupo
Copy link
Author

nurupo commented Jan 27, 2023

@MatMaul Since ABI updates are coordinated -- all plugins gets bumped at the same time as VLC, why not just make plugins have a single branch then, e.g. stable? (or branch/stable; I think "branch/" prefix is required for extensions)

In fact, I don't see any compelling reason to do ABI+SDK branches, even if plugin updates were not coordinated with VLC. It seems like the single branch approach is better, has more positives, with no negatives:

  • With ABI+SDK branches, if a plugin is neglected and not updated in tandem with VLC, it simply won't get loaded per VLC manifest version requirement, i.e. VLC would run without the plugin. It also requires that the user manually installs a new branch of the plugin, which is very bad. (There is also no point in keeping older ABI+SDK plugin branches once VLC switches to the new SDK, as VLC itself has only stable branch that can be used only with the plugin of the latest ABI+SDK version - the older ABI+SDK plugins can't be used with the VLC any longer.)

  • With just a single stable branch, if a plugin is neglected and not updated in tandem with VLC, it simply will fail to load, i.e. VLC would run without the plugin -- so the negatives are the same as with ABI+SDK branches. However, it doesn't require that the user manually installs the new branch, the current branch will update with the new ABI+SDK support when ready. Also, it might be that a plugin built against 18.08, 19.08, 20.08, 21.08, etc. SDKs is still ABI-compatible with the newest 22.08 SDK, so the neglected plugin might still continue to work, which a positive too.

An I missing some reason as to why ABI+SDK branches approach is preferred?

@MatMaul
Copy link

MatMaul commented Mar 15, 2023

With just a single stable branch, if a plugin is neglected and not updated in tandem with VLC, it simply will fail to load I think it's more a theory than a fact, on the principle it may load and just randomly crash VLC I believe.

I am not completely dismissing a change to stable, but I don't really like it and it feels wrong. I am undecided :) . I would rather prefer flatpak to be fixed to properly handle the plugin upgrade use case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants