CarlosDerSeher / snapclient

snapclient on ESP32
GNU General Public License v3.0
118 stars 15 forks source link

Build ontop of ESPHome (make this an ESPHome component) #92

Open oliv3r opened 1 month ago

oliv3r commented 1 month ago

Snapclient on ESP32 is great. What's even cooler, is if we can do small automation tasks. E.g. what if we connect an external button the the ESP32 to mute? Or volume keys? Or be able to turn on/off an external Amplifier?

These features could be added to this firmware for sure, but having it being controlled by Home-Assistant would be even cooler :)

I am developping a PCB for the Vappeby Gen3 Speaker (https://www.ikea.com/us/en/p/vappeby-bluetooth-speaker-black-gen-3-10517371/#content) to use it as an snapclient speaker.

However I don't want to break the existing functionality (e.g. bluetooth speaker, buttons etc) but do want it to be able to remotely powered on.

This is trivial with esphome and home-assistant, but then I can't get snapclient of course. Same vice-versa.

This post is just something to get the discussion going ...

P.S. are you aware of snapclientc? Maybe also worth to look into to combine some efforts.

DerPicknicker commented 1 month ago

We already had this discussion. Not possible the sync algorithm is highly optimised and it would require a lot of rewriting from @CarlosDerSeher .

Why not just use your PCB use a supported dac and you're ready to go. What benefit would you see? The webinstaller already makes it easy to use.

You could just add a listner for http-request so then you can send those commands via HA

CarlosDerSeher commented 1 month ago

@oliv3r I saw snapclientc a while ago but can't find it anymore. Is this still active?

Regarding esphome, this could be done I guess but I really don't know that much about the codebase and how integration could be done. This esphome component would need exclusive control of I2S.

I don't see myself doing this in the near future because I really don't have time for this but could support someone who is willing to try to take that adventure.

DerPicknicker commented 1 month ago

@CarlosDerSeher ... I asked on discord, and in some other discussions nobody was able to tell me how the ESPHOME structure is looking. For non timing relevant tasks it's fine but for your sync algorithm it's needed to know such basic things.

I gave up, the documentation lacks and my time is more worth than reading Bad documentation..... sorry for the harsh words the project is great but why they don't offer a drawing of their system structure.

It's so frustrating at some point that such big projects doesn't make it easy to get an understand of their system architecture. It's crucial for snapcast and other timing relevant stuff.

oliv3r commented 1 month ago

https://github.com/christf/snapcastc last commit in june, so certainly active.

From an architectual side, both suffer from one major issie, and that is that the streaming library is so intertwined with everything. It would be nice to have isolated libraries...

CarlosDerSeher commented 1 month ago

What exactly do you think needs to be isolated? Which are the problematic sections in this code in your opinion?

oliv3r commented 1 month ago

With snapcast and snapcastC? for one the library, which @bridadan did; but then this library got stale, and you/@jorgenkraghjakobsen "forked" it if you will. That means we have technically 5 libraries in the wild, each having their own life. From a library point of view, this is undesireable, all should be using one and the same library.

Personally, I blame C++ for this, as it's much harder to integrate into systems. Where it a simple C (at least the interface, but you still have to deal with C++ dependencies, libstdc++ at the least, which is for certain platforms unacceptable), like @bidadan did, things would be much cleaner and easier for the library part.

But more things in snapcast and snapcastC are entangled, where they probably shouldn't be.

CarlosDerSeher commented 1 month ago

So yo mean the snapcast protocol implementation itself should be put in a library? This should be doable but the thing with my repo is, that I highly optimized overall RAM usage and also peak RAM usage so also modules without PSRAM can be used. That's why I don't really use the original library from jorgen / bridadan bbut only parts and also changed some things.

bridadan commented 1 month ago

If there's a library that's considered the most up-to-date, I'd be happy to place a link to it in my repo's readme (or better yet, feel free to send a PR!). I have no plans to pick it up again in the future and I don't want to add to the confusion 🙂

oliv3r commented 1 month ago

So yo mean the snapcast protocol implementation itself should be put in a library? This should be doable but the thing with my repo is, that I highly optimized overall RAM usage and also peak RAM usage so also modules without PSRAM can be used. That's why I don't really use the original library from jorgen / bridadan bbut only parts and also changed some things.

But making it a library still makes sense, any other system can very effectivily use the library with all RAM optimilizations. Where needed, these could be put behind flags at a later time.

DerPicknicker commented 1 month ago

The Big Problem is the structure of ESPHOME. It's not clear how they designed their system.. Besides that it misses some options for Components, like prioritizing. I guess you can do that inside your Component but the ESPHOME Runtime has to prioritizing your component. That's a crucial feature for an Audio-Sync application... If you suspend a FreeRTOS Task and wait too long you will clearly hear dropouts. I don't criticize them but I ask in several channels and nobody could tell me how the internals are looking and where I can find the documentation. I like the simplicity of the project but sometimes it blocks you from doing some stuff... Like Arduino IDE vs. IDF...

I found this: https://github.com/mads03dk/esphome currently not finished but maybe a good starting point.

CarlosDerSeher commented 1 month ago

There are 2 questions which need to be answered regarding esphome:

  1. Do we need to use their audio component or are we allowed to lock i2s control for our own component and make them mutually exclusive from config view.

  2. Is there a parent task calling all component tasks (or similar), or can we install any tasks with the priorities and on the core we want?

DerPicknicker commented 1 month ago

@CarlosDerSeher I can't give you an answer... Maybe @mads03dk can say something because he started working on an ESPHOME integration...

alexyao2015 commented 1 month ago

I can help answer some questions since I've worked in the codebase a bit.

Esphome runs essentially on the Arduino model where it has its "main" setup and loop function calls. Components register their own setup and loop calls which are processed along with other components. The backend implementation of loops are ran depends on if Arduino or idf is selected. On Arduino it uses the standard setup and loop functions. On idf, it creates a loop task that run the main esphome loop. For idf, this means esphome running on a single core always because there is only one loop task.

You are free to essentially do whatever you please inside the constraints of your components, including creating new tasks and using the normal idf calls to gpio. Of course, if you want to use esphome abstraction on top of the gpip, it's also possible but not required. I would think since esp32 is dual core, you could run the Snapcast client on the other inactive core.

CarlosDerSeher commented 1 month ago

Thanks for your input appreciate it. This sounds like we could indeed implement sync in our own high priority task, so it should be possible to port it I guess.

DerPicknicker commented 1 month ago

@CarlosDerSeher .. which files are for the snapcast and sample stuffing relevant?

CarlosDerSeher commented 1 month ago

player.c implements i2s and syncing

DerPicknicker commented 1 month ago

So if I understand ESPHOME correctly - we can create an FreeRtos Task with a high priority and pin that to the core. The only problem I see is that if WiFi is used by second core.

And your code is C and every resource I found is that ESPHOME using c++. So this will be a challenge.. Maybe if we use the IDF as core we can use your code without converting it to c++...

DerPicknicker commented 4 weeks ago

@alexyao2015 ... do you know if we can use C code inside a ESPHOME component?

CarlosDerSeher commented 4 weeks ago

Normally c code called from c++ isn't a problem

alexyao2015 commented 4 weeks ago

C should work fine. It's just platformio under the hood and it copied it all into a platformio project. You'll likely want to choose the idf framework instead of Arduino so you can use freertos tasks.

alexyao2015 commented 4 weeks ago

Here's some relevant code for how it sets up the main loop task. https://github.com/esphome/esphome/blob/dev/esphome/components/esp32/core.cpp

DerPicknicker commented 4 weeks ago

@alexyao2015 thank you for the further information.. if I had more time I would start working on this. Maybe after the summer..

@CarlosDerSeher here is so much more coming: https://futureproofhomes.net/products/satellite1-pcb-dev-kit

This would be the ideal snapcast device ...

DerPicknicker commented 4 weeks ago

@CarlosDerSeher in which file you're connecting to the snapcast server and getting the data? Http task?

And is all code running on the same core?

CarlosDerSeher commented 4 weeks ago

Yes http task.

Most tasks are not pinned to any specific core. Only player task is pinned to core 1 with high priority

DerPicknicker commented 4 weeks ago

@CarlosDerSeher ...

So I think it would be much easier if we can or you can write down all needed functions. Because I tried now understanding your whole concept and until I get every detail it may be faster if you can write down which functions are essentially needed for snapcast connections.

I think a library based on your syncing algorithm would be amazing because it would be much easier to integrate it into ESPHOME or other systems.

CarlosDerSeher commented 4 weeks ago

You are talking about a library specifically for esp32 or C / freeRTOS in general?

When I have time I'll try to do some documentation for the code. Maybe some clean up before that. But there is still #54 which has to be resolved in my opinion or rhe code can't be called stable. Some parts of the code rely on knowing the chunk size/duration and that's a real problem currently and I think this part needs a bit of more work before it can be put in a library.

A library of snapclient should be somewhat independent of i2s functions and I think those should be passed as pointers to the lib during initialization? And also DMA buffer size and such...

DerPicknicker commented 4 weeks ago

@CarlosDerSeher ..

I am talking about only a esp32 library because this is the most easiest way...

I think we should have at a top level this functionset

The i2s component is still not clear. Do we need to use the ESPHOME component or can we use the one from espressif. If we use the Espressif implementation we should also include that. (I know that ESPHOME is just a abstraction of Espressif...)

CarlosDerSeher commented 4 weeks ago

Audio syncing, audio buffer and i2s are intertwined. Syncing highly relies on i2s DMA buffer size and chunk duration. It works like this (pseudo code):

Get audio chunk
Fill DMA buffer
Wait until its timestamp is now
Start i2s playback
While True:
  Get audio chunk
  Check timestamp and do sample stuffing
  Put it to i2s (which blocks until enough space is available)
DerPicknicker commented 3 weeks ago

@CarlosDerSeher thank you for your clarification. Maybe we will wait until you would call it stable and then we can continue on this discussion?