Giter VIP home page Giter VIP logo

Comments (26)

MoonSweep avatar MoonSweep commented on August 26, 2024

Ping ?

from pipewire-module-xrdp.

elboulangero avatar elboulangero commented on August 26, 2024

Already done:

What needs to happen next:

  • @matt335672 Do you think it's a good idea to tag a release? That would be a good signal for distros that the repo is ready to be packaged.
  • if there's a tag, then I'd follow up with the Utopia team in Debian, so that the maintainers of the pipewire package in Debian can review my package and fix what needs be

Alternatively: there was some discussions to include this module straight in pipewire, cf. neutrinolabs/xrdp#2023 (comment). Pipewire upstream was willing to accept the module, however the author @Hiero32 never followed up, as far as I know.

Is there still a plan to include this module upstream, is anyone aware of anything?

from pipewire-module-xrdp.

matt335672 avatar matt335672 commented on August 26, 2024

They're all good points @elboulangero.

Arguments for upstreaming

  1. A lot of the maintenance of the module will now be done by the pipewire team. For example, a lot has changed in the module-pipe-tunnel.c module since @Hiero32 split this off. We should probably adopt some of these changes here.
  2. Supporting multiple pipewire versions might be messy if the module isn't delivered with Pipewire.

Arguments against upstreaming

  1. The pipewire team are unlikely to test any of this module unless we can also provide some kind of test harness. This will require maintenance, so we may not save a lot of time against 1) above anyway.
  2. Any changes will require us to update two projects now instead of one.
  3. Unlike Pulseaudio, Pipewire supports out-of-tree modules anyway, so building this is a walk in the park compared to the previous situation.

Personally I'm leaning towards not upstreaming, but that's just my view. @metalefty, @jsorg71, @Nexarian, @Hiero32 - do you have views on this? Please add to either section in the list above.

@elboulangero - regardless of the outcome of the above discussion, would you like a release of the current module? If so, I'll look at addressing some of cosmetic issues. We have at least one spelling mistake (conect_xrdp_socket()), and a clash of spaces and tabs. For the record, Pipewire sources use tabs for tabbing and xrdp sources use spaces. This module currently uses both! We should probably use tabs here.

from pipewire-module-xrdp.

MoonSweep avatar MoonSweep commented on August 26, 2024

Hi,

I'm not part of the project so this is only my humble opinion, but I would find it more logical if it was included in XRDP distribution, rather than PipeWire.

This way, maintainers of XRDP packages in distributions would only need to add PipeWire headers as build depends, and the module could be built automatically. Then, it can be shipped in a separate binary package depending on PipeWire, as not to make the whole XRDP package depend on PipeWire.

On the other hand, maintenance from the PipeWire team would of course be greatly useful, but as said above, it would be difficult for them to test it, as it would need a ready to use XRDP server, which is completely out of scope for their project.

from pipewire-module-xrdp.

elboulangero avatar elboulangero commented on August 26, 2024

@matt335672 Thank you for summarizing the pros and cons of upstreaming vs separate module.

@elboulangero - regardless of the outcome of the above discussion, would you like a release of the current module?

Yep, if that's Ok with you, I'd be happy with that. Then I can kickstart an attempt to get this into Debian proper, as I have a bit of bandwidth these days.

@MoonSweep Apparently, in terms of build depends, there are not dependency on xrdp, so it's not shocking it it becomes part of pipewire. It will not make pipewire depend on xrdp at build time. And I suppose that, even if it's part of pipewire, it will still be under the form of a module, so it can be shipped as a separate package if needed.

Cheers

from pipewire-module-xrdp.

matt335672 avatar matt335672 commented on August 26, 2024

Well, this version works and people are using it. I see no reason to mess about with getting the spacing consistent at this stage, or to fix spellings. We can do that for the next release, or for getting the module into pipewire itself.

@metalefty - would you like to make a release so this is signed with your key? I'm happy to make one, but using the same signing key as xrdp seems to make more sense to me.

from pipewire-module-xrdp.

metalefty avatar metalefty commented on August 26, 2024

I would rather like to keep maintaining by us because it is easier to build out-of-tree modules compared to PulseAudio as you mentioned, however, I personally happy to push this to upstream if PipeWire team is willing to maintain.

I will make a release soon with sign.

from pipewire-module-xrdp.

metalefty avatar metalefty commented on August 26, 2024

I made a release. The tarball is not added to the release because it does not include submodules, unlike xrdp main source. pulseaudio-module-xrdp also does not have a tarball other than generated by GitHub. Let me know if it is needed.

https://github.com/neutrinolabs/pipewire-module-xrdp/releases/tag/v0.1

from pipewire-module-xrdp.

matt335672 avatar matt335672 commented on August 26, 2024

That looks fine to me - thanks.

@elboulangero - is that enough for you to be getting on with?

from pipewire-module-xrdp.

elboulangero avatar elboulangero commented on August 26, 2024

All I need is a tag, and I see a tag v0.1. Enough for me! I'll keep you updated. Thanks!

from pipewire-module-xrdp.

elboulangero avatar elboulangero commented on August 26, 2024

from pipewire-module-xrdp.

MoonSweep avatar MoonSweep commented on August 26, 2024

Hi, sorry to insist, but...

In #1037111, your update states that "this module will not be included in pipewire upstream, and it will be maintained by neutrinolabs (which is the upstream for xrdp already)".

In that case, wouldn't it be much simpler to include the module in the xrdp source tree, instead of creating a new package ?

Are the maintainers of xrdp against this and want to keep the two projects as separate upstreams ?

Regards,

--
Raphaël

from pipewire-module-xrdp.

matt335672 avatar matt335672 commented on August 26, 2024

Hi @MoonSweep

I think 'simpler' may depend on your perspective. From ours, two separate projects are simpler, particularly as this module is effectively an interface between pipewire audio and xrdp. We only control one side of that arrangement.

I note that in the past, Debian have packaged two tarballs from us in the same Debian package - that might be an option here if you want to make things easier for the users:-

https://snapshot.debian.org/package/xrdp/0.9.1-1/

Thoughts?

from pipewire-module-xrdp.

MoonSweep avatar MoonSweep commented on August 26, 2024

Hi @MoonSweep

Hi @matt335672

I think 'simpler' may depend on your perspective. From ours, two separate projects are simpler, particularly as this module is effectively an interface between pipewire audio and xrdp. We only control one side of that arrangement.

I don't quite understand why it's simpler, but I won't insist on that point. It's just that, even if it doesn't need xrdp sources to compile, it would be useless without it, so I see it more like an xrdp module than a pipewire module. Hence my logic, including it with xrdp seems more natural to me.

I note that in the past, Debian have packaged two tarballs from us in the same Debian package - that might be an option here if you want to make things easier for the users:-
Thoughts?

Not simpler for the users, but for the maintainers (both are maintained by a team, Debian Remote) . Those two tarballs are the xrdp server itself, and the "virtual" drivers to start an X server from it (because IIUC it can also be used as a relay to a vnc or another rdp server).

Adding a new tarball wouldn't be easier for them, on the contrary, since they would need - I guess - to add new steps in debian/rules to compile the module along the rest.

But if this work is already done upstream, and xrdp's Makefile already compiles the pipewire module by itself if it detects that pipewire's headers are installed, the additional work would just be to add a new build-dependency and the new binary package (i.e. only modify debian/control and add one .install file, and only once, when the new upstream version including the pipewire module is released).

You could even do that for them if you think that it wouldn't be fair to add work for them without asking (I admit I didn't think about this until now), or even join the team if you like (since you're already a maintainer IIUC), it would still be much less work for you than creating a new package, having it included in Debian, and maintaining it on the long term.

Sorry if I'm not clear, English is not my native language ^^'

from pipewire-module-xrdp.

matt335672 avatar matt335672 commented on August 26, 2024

I think you're plenty clear enough @MoonSweep - it's probably me that's not being clear. My apologies.

The xorgxrdp package contains the Xorg drivers that allow us to use Xorg directly from an RDP session. It's quite closely coupled with xrdp and is useless without it.

For xrdp-0.9.1, Debian decided to package the two tarballs from us as a single Debian package. The reason was, that, for this version Debian decided to make the xorgxrdp backend the default rather than the older VNC backend.

For 0.9.6-1, Debian decided to make these two packages separate (Changelogs here and here). That's a decision that Debian Remote took without consulting us. We're absolutely fine with that. You would expect Debian Remote to not only grasp the finer details around Debian packaging, but to understand the needs of their user community better than upstream (i.e. xrdp, and other projects).

Other projects provide a single tarball which is split into several Debian packages. This doesn't apply to xrdp, but does (for example) to pulseaudio.

In other words, there's no real tie between the number of tarballs we deliver, and how the various distributions (not only Debian) present that software to their users. That's a decision for them, and not for us.

I don't think our decision regarding our own project deliverables makes it any easier or harder for the distro packagers. They're competent engineers, and providing we stick to standard packaging and configuration tools they can cope easily with anything we throw over the wall to them.

Other distros have made use of xrdp and xorgxrdp being separate deliverables. Arch currently has two packages, xorgxrdp and xorgxrdp-glamor where xorgxrdp is built with different switches. Personally I'd question the wisdom of that, but I'm not an Arch packager!

Given the above background, at the moment it seemed sensible to me to deliver the pipewire audio driver in the same way that xorgxrdp is delivered. The two are both 'interface packages'. They are similar in that:-

  • Neither is useful on its own
  • If the interface on the other side changes (i.e. Xorg or pipewire), it's a lot easier for us to change the interface package to support the new interface, or even to create a new interface package.

from pipewire-module-xrdp.

MoonSweep avatar MoonSweep commented on August 26, 2024

I understand your point. I won't make any more comments on this.

from pipewire-module-xrdp.

elboulangero avatar elboulangero commented on August 26, 2024

Hello, I opened 3 MR so far.

!7 and #10 are mostly about tidying up.

!8 is really needed for inclusion in Debian though, thanks to this change we can safely install the module on a system where pipewire is not installed, and in this case it won't do anything (the xdg autostart script will just exit early).

@metalefty If it's all good for you, I'd be happy if you could merge those 3 PRs, tag a new version, and then I'll be good to proceed with Debian packaging, and upload the package in Debian. Thanks thanks thanks!

from pipewire-module-xrdp.

elboulangero avatar elboulangero commented on August 26, 2024

I don't think our decision regarding our own project deliverables makes it any easier or harder for the distro packagers. They're competent engineers, and providing we stick to standard packaging and configuration tools they can cope easily with anything we throw over the wall to them.

Fully agree with @metalefty here! Upstream knows best how to handle their stuff, and whether it's better to split in different Git reps, or ship it all in a single Git repo. Downstreams (eg. Linux distros in this case) will adjust accordingly, and will then know best how it should be distributed (eg. one package, or split into several packages) so that it fits in the distro as a whole.

In this case I'm pretty happy that it's a separate Git repo. It's all about pipewire, therefore it will be maintained within the Debian team that already maintains the pipewire and wireplumber packages. If ever there are changes in the pipewire lib that requires a rebuild and/or adjustments in the module, they will be able to help with it, without having to deal with the whole xrdp repo. I think it's just perfect as it is :)

from pipewire-module-xrdp.

metalefty avatar metalefty commented on August 26, 2024

Hi @elboulangero,

I also do package maintenance in FreeBSD project so I think understand the manners of both sides. It is very grateful to submit useful changes from downstream to upstream. Unfortunately, PipeWire is not yet my area, so I won't be able to review your changes from corner to corner. However, your changes look nothing weird, we're happy to accept them.

Your contribution is definitely helpful, thanks!

from pipewire-module-xrdp.

metalefty avatar metalefty commented on August 26, 2024

Merged all PRs and created a new tag.

from pipewire-module-xrdp.

elboulangero avatar elboulangero commented on August 26, 2024

Fully agree with @metalefty here!

Actually I quoted a part from @matt335672 in my comment above, sorry for the confusion.

@metalefty Thanks for merging! I updated the Debian packaging, and now waiting for one last round of feedback from Dylan, before uploading the package to Debian. Almost there!

from pipewire-module-xrdp.

matt335672 avatar matt335672 commented on August 26, 2024

Fine by me @elboulangero!

Thanks for the effort you're putting into this. Having a pre-built module to do this work will help enormously with user issues.

from pipewire-module-xrdp.

elboulangero avatar elboulangero commented on August 26, 2024

The package pipewire-module-xrdp is now in Debian's NEW queue: https://ftp-master.debian.org/new.html

It means that it's uploaded, and waiting from review from the FTP masters, before it's accepted in Debian unstable. This step can take an unpredictable amount of time. I'll ping again here when the package is accepted.

from pipewire-module-xrdp.

MoonSweep avatar MoonSweep commented on August 26, 2024

The package pipewire-module-xrdp is now in Debian's NEW queue: https://ftp-master.debian.org/new.html

Great news ! Thanks for all your efforts :)

from pipewire-module-xrdp.

elboulangero avatar elboulangero commented on August 26, 2024

And now in Debian unstable: https://tracker.debian.org/pkg/pipewire-module-xrdp

We can close this issue, Thanks everyone for following up until the end!

from pipewire-module-xrdp.

matt335672 avatar matt335672 commented on August 26, 2024

I think the thanks are mostly due to you @elboulangero !

I'll close this now then.

from pipewire-module-xrdp.

Related Issues (7)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.