canonical / iot-example-graphical-snap

Developer Guide for Embedding IoT GUI with Ubuntu Frame
MIT License
22 stars 11 forks source link

fix(electron): audio-playback plug #7

Closed ChiragMoradiya closed 1 year ago

ChiragMoradiya commented 1 year ago

load pulseaudio library

ChiragMoradiya commented 1 year ago

Oh! Sorry about that

On Tue, 11 Oct, 2022, 4:40 pm Alan Griffiths, @.***> wrote:

@.**** commented on this pull request.

In snap/snapcraft.yaml https://github.com/MirServer/iot-example-graphical-snap/pull/7#discussion_r992180389 :

@@ -41,7 +41,7 @@ plugs:

environment:

This is one of four snippets that relate to providing the userspace graphics needed by your application.

  • LD_LIBRARY_PATH: $SNAP/graphics/lib
  • LD_LIBRARY_PATH: $SNAP/graphics/lib:$SNAP/usr/lib/aarch64-linux-gnu/pulseaudio

This should not hard-code the architecture. See ${SNAPCRAFT_ARCH_TRIPLET}

β€” Reply to this email directly, view it on GitHub https://github.com/MirServer/iot-example-graphical-snap/pull/7#pullrequestreview-1137279213, or unsubscribe https://github.com/notifications/unsubscribe-auth/AASJ3BNNNU77RTH43WSUNYTWCVDJ7ANCNFSM6AAAAAARCB6O3A . You are receiving this because you authored the thread.Message ID: @.***>

clorichel commented 1 year ago

Thanks for contributing this @ChiragMoradiya πŸ€—πŸ€“

After:

I can confirm that this just works πŸ‘Œ and I suggest you @AlanGriffiths to just merge: getting sound out of this iot snap example is a great addition ❀️

Guess we can also update https://discourse.ubuntu.com/t/packaging-an-electron-application-quick-start-as-an-iot-gui/29078 to mention audio output and the required step (pulseaudio from latest/beta and the snap connect).

AlanGriffiths commented 1 year ago

I suggest you @AlanGriffiths to just merge

As you illustrate, this "works" but isn't a solution I can recommend as the pulseaudio snap is unsupported (it hasn't been updated for five years). Because I can't recommend this approach I don't think it appropriate to merge into an example that many may follow without understanding the risks.

The alternative approach of incorporating the audio stack into the snap would also work and I could recommend that. But it goes beyond the intended scope of the exercise. Audio is not addressed by any of the GTK, Qt, SDL, X11, ... examples.

It is useful to have this PR here as people, like yourself, can find and experiment with this solution.

ChiragMoradiya commented 1 year ago

@AlanGriffiths I assume that the solution provided has been misunderstood OR at least my understanding is wrong.

What I understand about the problem/cause of the issue & it's fix is as follows.

Cause

Snap uses audio-playback plug to enable Audio Output. But, as pulseaudio library isn't loaded it fails to connect to audio-playback plug.

Fix

Install pulseaudio (libpulse0) library; and mention it in LD_LIBRARY_PATH classpath.

So, standard audio-playback interface will start work.

I assume, it's not using/dependent-on pulseaudio snap. Fix still uses audio-playback interface which was already used.

Please, correct me if I am wrong.

Saviq commented 1 year ago

@ChiragMoradiya that will only work on Ubuntu Classic, on Ubuntu Core it's dependent on the pulseaudio snap, because that's currently the only one providing the audio-playback slot to connect to.

It's likely better to use the alsa interface and supply the pieces necessary for that to work in your snap - but that's what Alan mentioned is out of scope for these guides. They're targeted at Ubuntu Core, so providing a solution for Classic would be misleading β€” and we would rather focus on the graphical side of things.

ChiragMoradiya commented 1 year ago

@Saviq thank you for the explanation. It makes sense.

clorichel commented 1 year ago

Thanks Alan for your feedback! ❀️ I understand your position, which totally makes sense. I find it weird that this pulseaudio snap isn't being updated with the flow, and really odd that the pulseaudio snap is on the website but not on the search results of the snap store 🀯 but I guess audio is not the focus of most IoT projects and I just have to live with it πŸ˜„πŸ€“

Kind of agree with Chirag too, though πŸ€—: having the audio-playback plug listed on this snap is a bit misleading because without the additional LD_LIBRARY_PATH and libpulse0, users cannot just snap connect it to their system audio-playback interface, simplicity of which is exactly one of the point of the snap ecosystem πŸ™ƒ To be clear: configuration of their system (through the pulseaudio snap or whatever else is providing an audio-playback interface) is totally out of the scope of this example snap, for sure πŸ‘Œ

Having said that, I'm laughing in advance when thinking we might end up agreeing that the audio-playback plug should just be removed to avoid mislead users: totally looks like bad politics πŸ˜‚

Finally, thanks MichaΕ‚ for clarifying what Alan meant by "incorporating the audio stack": this PR comments definitely are useful for people that will end up here, which they will, because Electron on Ubuntu Core opens up a world of opportunities for web developers teams πŸ‘πŸ€“

AlanGriffiths commented 1 year ago

we might end up agreeing that the audio-playback plug should just be removed to avoid mislead users: totally looks like bad politics

Yes (I think it was included by mistake).

The "audio on Ubuntu Core" story should be a lot clearer, and relics from past experiments (such as the pulseaudio snap) confuse the issue even though they are not searchable on the store.