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

 gstreamer1: update to 1.18.2 #26411

Closed
wants to merge 10 commits into from
Closed

 gstreamer1: update to 1.18.2 #26411

wants to merge 10 commits into from

Conversation

ndowens
Copy link
Contributor

@ndowens ndowens commented Nov 15, 2020

gst1-python version 1.18.1 doesn't support python2 anymore so can not update it

If someone can test update please let me know if update works fine.

For me, no matter gst version I have installed I can't not get audio out from an app that depends on gstreamer

@ndowens ndowens marked this pull request as draft November 15, 2020 20:28
@ndowens ndowens marked this pull request as ready for review November 15, 2020 20:53
srcpkgs/gst-libav/template Show resolved Hide resolved
srcpkgs/gst-plugins-bad1/template Show resolved Hide resolved
srcpkgs/gst-omx/patches/0001-ARM-build.patch Outdated Show resolved Hide resolved
@not-chicken not-chicken mentioned this pull request Nov 16, 2020
@ericonr
Copy link
Member

ericonr commented Nov 16, 2020

Please try removing python from hostmakedepends and use python3 instead.

Your commit message still doesn't say that you removed the build option from one of the templates.

@ndowens
Copy link
Contributor Author

ndowens commented Dec 17, 2020

Please try removing python from hostmakedepends and use python3 instead.

Your commit message still doesn't say that you removed the build option from one of the templates.

The one I see I removed option from, gstreamer1, does have "Remove gtk_doc option as it does nothing and doubt many use" Unless I am blind, I dont see 'python' but python3

@fosslinux
Copy link
Contributor

Please try removing python from hostmakedepends and use python3 instead.

This has already bene completed.

@the-eater
Copy link
Contributor

the gstreamer devs are angry with Void because of the sndio plugin and them getting bug reports about it, would it be possible to be more clear that the sndio plugin is not maintained by them?

@ndowens
Copy link
Contributor Author

ndowens commented Dec 19, 2020

Not sure the best way would to go about that. Wait for a member of Void to respond

@fosslinux
Copy link
Contributor

What am I missing? The plugin seems to come through their official tarball, how is it not maintained by them?

@the-eater do you have a ml link or something

@ericonr
Copy link
Member

ericonr commented Dec 19, 2020

@fosslinux we have a 1k line patch for adding sndio support.

@the-eater
Copy link
Contributor

What am I missing? The plugin seems to come through their official tarball, how is it not maintained by them?

@the-eater do you have a ml link or something

https://twitter.com/sdroege_/status/1340342884197920770

here's one of the devs talking about it

@q66
Copy link
Contributor

q66 commented Dec 20, 2020

this is quite bad, the patch should be either removed entirely, or separated into a "you're on your own" package... the official stance of void has always been not to patch in features, and it's even worse if it causes actual bugs for other people

so before we merge this, it needs to be dealt with

@ndowens
Copy link
Contributor Author

ndowens commented Dec 20, 2020

Removed sndio patch from gst-plugins-base1

now got to figure out why tests is failing

@ericonr
Copy link
Member

ericonr commented Dec 20, 2020

I'd prefer the solution of splitting it or making it a build option... We don't usually patch things, but sndio support has been consistently added throughout our packages.

I can try to take a look at the patch, but that will happen later; we should be careful with OpenBSD patches for sndio because they don't always implement proper fallback.

@q66
Copy link
Contributor

q66 commented Dec 20, 2020

what has been asked for or added does not matter - we're not the upstream, so we have no responsibility for maintaining entire components added on top, and as you can see, if we do that it leads to unfortunate incidents with upstream projects; i really don't want to become another gentoo, where our users come to upstreams with bug reports caused by their outlandish setups

if people want those features available, they should go to upstream, open reports and PRs there, and then we can pull it downstream, but maintaining entire new features within a distro is really bad

if we have other packages where features are patched in like this too, these are also candidates for removal

@ndowens ndowens changed the title  gstreamer1: update to 1.18.1  gstreamer1: update to 1.18.2 Dec 21, 2020
@ndowens ndowens marked this pull request as draft December 21, 2020 00:36
@fosslinux
Copy link
Contributor

I strongly agree with @q66. Why does sndio become the exception for pulling in features? Unlike libressl (for example), we have alternatives (such as pulseaudio), and patching this in with a 1k LOC patch and a ton of new files is not very good.

This patch was originally added in #8056 and has barely changed since then. upstream is not hostile to readding this patch, it's just that there was no maintainer (src: https://twitter.com/sdroege_/status/1340367227581829123). If someone is interested in putting that work in, then upstream will merge it and we will take it as downstream, as our policy is in general.

I honestly think while sndio is a great project there is not enough upstream support and patches are not being maintained. Other quite large patches include:

There are many other smaller patches which IMO are fine as they are just altering build systems, including:

  • baresip
  • firefox{,-esr}
  • icecat
  • qt5-webengine
  • thunderbird

Futhermore sndio seems to be "prioritized" in a lot of packages over alsa. (?!?!?!?!?)

@q66
Copy link
Contributor

q66 commented Dec 21, 2020

there should also be an alsa plugin for sndio, so people should just be able to use the alsa backend in apps and sndio should be able to pick them up

@ericonr
Copy link
Member

ericonr commented Dec 21, 2020

The alsa plugin has issues with syncing (watching videos with it is quite terrible, from what I've heard), and it doesn't work for audio inputs.

The portaudio patch was fixed up a while ago to actually implement proper fallback for other sound devices. I don't know about SDL or chromium.

It is my understanding that some void developers made the decision to make sndio a first class audio server; we are probably the best sndio experience out of the box on linux (possibly also BSDs other than OpenBSD) - it seems that sometimes even to the detriment of other sound servers, due to faulty fallbacks. It would suck a bit to change that policy by removing sndio support from a bunch of packages at the same time. If the current packages aren't known to cause any issues, I think it's ok to leave thing as is, as long as we have people trying to work to push these upstream (I'm willing to try gstreamer and SDL, possibly portaudio as well).

@travankor
Copy link
Contributor

travankor commented Dec 21, 2020

It would suck a bit to change that policy by removing sndio support from a bunch of packages at the same time. If the current packages aren't known to cause any issues, I think it's ok to leave thing as is, as long as we have people trying to work to push these upstream.

Is this an official policy or just something that happened naturally over time? I understand that there are a lot of packages patched to support sndio, but sndio is not really advertised or mentioned whenever Void Linux comes up.

@q66
Copy link
Contributor

q66 commented Dec 21, 2020

there was no concrete decision to do this, things were largely added by random people over time, and nothing was ever advertised

there is no "policy" on the matter, so don't call it that

@q66
Copy link
Contributor

q66 commented Dec 21, 2020

also, having large patches in ~3 projects and small ones in a couple others can hardly be called a conscious effort to make something a first class support

@ericonr
Copy link
Member

ericonr commented Dec 21, 2020

Ok, it wasn't a policy, bad choice of words. More of a natural thing that came about.

All I'm asking is we don't have a sndio removal spree. Let it happen if complaints come about for certain packages, but otherwise leave as is.

@ericonr
Copy link
Member

ericonr commented Dec 21, 2020

we do enable sndio on almost all projects that support it, and those patches are in widely used packages.

@q66
Copy link
Contributor

q66 commented Dec 21, 2020

again, removal/disablement of support in 3 projects (gstreamer is one, and qt5-webengine and portaudio are the others) can hardly be called a "spree" and is following actual policy that is in place (which is, we do not patch in features into upstream projects)

SDL can probably stay, since SDL2 has upstream support in it and SDL1 is dead

@q66
Copy link
Contributor

q66 commented Dec 21, 2020

it's especially because those large patches are in widely used packages that we should not be patching them

@ericonr
Copy link
Member

ericonr commented Dec 21, 2020

I don't want this to be a hill for me to die on; I can definitely build packages myself with those removals reverted. I should note that it will be a breaking change for those users who rely on said patches, though not a major one.

Portaudio having sndio also enables any application that uses it to talk to sndio, so I'd say it's a important package for the purpose of sndio support.

Anyway, this is the last I will touch the subject.

@q66
Copy link
Contributor

q66 commented Dec 21, 2020

again, if users rely on patches, it should be in their best interest to put in effort to work them into the respective upstreams, but maintaining that in distros is just wrong

i can tell you're a sndio user and therefore biased on the subject, but that is not a reason to keep potentially sketchy patches (and as a result give a poor impression to upstreams) either

@ndowens
Copy link
Contributor Author

ndowens commented Dec 21, 2020

Currently waiting to hear from upstream why tests fail in gst-plugins-bad1; so this is the current hold-up. If you are curious about current tests failing: https://termbin.com/z04a

@ericonr
Copy link
Member

ericonr commented Dec 21, 2020

Tests in bad are allowed to fail; it's the ugly duck.

Just do ninha check || msg_warn "Tests failed, not entirely unexpected" or similar.

@ndowens ndowens marked this pull request as ready for review December 22, 2020 03:16
@ndowens ndowens requested a review from ericonr December 22, 2020 03:16
build_options="cdparanoia gir sndio"
build_options_default="cdparanoia gir"

# this should not remain in this package in longer term
# either upstream, separate, or remove
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why did you partially undo 2e9ed79?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just added additional warning if someone wants to enable sndio since it isn’t to be supported upstream

@travankor
Copy link
Contributor

ping?

@travankor
Copy link
Contributor

@ndowens need to rebase

I can adopt this PR if you're busy, too.

@ndowens
Copy link
Contributor Author

ndowens commented Jan 17, 2021

@ndowens need to rebase

I can adopt this PR if you're busy, too.

Feel free

@ndowens ndowens closed this Jan 17, 2021
@travankor
Copy link
Contributor

#27996

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants