core icon indicating copy to clipboard operation
core copied to clipboard

ESPHome media proxy

Open synesthesiam opened this issue 1 year ago • 3 comments

Breaking change

Proposed change

Adds an HTTP view to ESPHome that will allow devices to proxy media conversion through HA using ffmpeg. This will enable devices to support more media formats by having HA convert media to the device's preferred format, sample rate, etc.

To ensure that this web view is only used for the intended purposes, a function async_allow_proxy_url must first be called by the ESPHome integration to add a URL to an allow list for a given device id. Once allowed, a URL of the form /api/esphome/ffmpeg_proxy/<device_id>/<ulid>.<extension> will stream the converted audio to the client.

A URL is only allowed to be requested once for a registered device. Once requested, async_allow_proxy_url must be called again for each device id/url pair.

Depends on the following PRs:

  • aioesphomeapi: https://github.com/esphome/aioesphomeapi/pull/925
  • esphome: https://github.com/esphome/esphome/pull/7318

Type of change

  • [ ] Dependency upgrade
  • [ ] Bugfix (non-breaking change which fixes an issue)
  • [ ] New integration (thank you!)
  • [x] New feature (which adds functionality to an existing integration)
  • [ ] Deprecation (breaking change to happen in the future)
  • [ ] Breaking change (fix/feature causing existing functionality to break)
  • [ ] Code quality improvements to existing code or addition of tests

Additional information

  • This PR fixes or closes issue: fixes #
  • This PR is related to issue:
  • Link to documentation pull request:

Checklist

  • [ ] The code change is tested and works locally.
  • [ ] Local tests pass. Your PR cannot be merged unless tests pass
  • [ ] There is no commented out code in this PR.
  • [ ] I have followed the development checklist
  • [ ] I have followed the perfect PR recommendations
  • [ ] The code has been formatted using Ruff (ruff format homeassistant tests)
  • [ ] Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • [ ] The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • [ ] New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • [ ] For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.

To help with the load of incoming pull requests:

synesthesiam avatar Aug 06 '24 16:08 synesthesiam

Hey there @ottowinter, @jesserockz, @kbx81, @bdraco, mind taking a look at this pull request as it has been labeled with an integration (esphome) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of esphome can trigger bot actions by commenting:

  • @home-assistant close Closes the pull request.
  • @home-assistant rename Awesome new title Renames the pull request.
  • @home-assistant reopen Reopen the pull request.
  • @home-assistant unassign esphome Removes the current integration label and assignees on the pull request, add the integration domain after the command.
  • @home-assistant add-label needs-more-information Add a label (needs-more-information, problem in dependency, problem in custom component) to the pull request.
  • @home-assistant remove-label needs-more-information Remove a label (needs-more-information, problem in dependency, problem in custom component) on the pull request.

home-assistant[bot] avatar Aug 06 '24 16:08 home-assistant[bot]

Still need a merge/bump in aioesphomeapi for this PR to function.

synesthesiam avatar Aug 25 '24 15:08 synesthesiam

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks :+1:

Learn more about our pull request process.

home-assistant[bot] avatar Aug 28 '24 12:08 home-assistant[bot]

Would this be better located in the Media Source integration? Pass in the format capability of the Player to the Source and let it decide what to return (and if its playable). Seems like we are isolating good functionality to just esp32 Media Player.

rwrozelle avatar Sep 01 '24 14:09 rwrozelle

ESP-ADF provides the ability to configure a esp_decoder, that can be setup to consume multiple audio formats (mp3, flac, wav, etc.). My understanding is that you would setup an array of supported formats in the media_player and the play_media would down select to just one of these. Is this correct: a supported format could be forced through ffmpeg proxy to the "default" format with a possible loss of fidelity (flac to mp3) for example. Wouldn't the preferred approach be to find the content-type of the original url and see if its in the list and not force the conversion?

rwrozelle avatar Sep 02 '24 14:09 rwrozelle

It would be ideal to keep the original format, sample rate, bits per sample, and number of channels when possible. But the content type isn't enough for this (beyond the format). And trying to parse headers is a complex problem too.

I'm aware of the decoders in esp-adf, but I'm purposely avoiding it because of its large dependency tree and frequent API deprecations/breakages by Espressif.

synesthesiam avatar Sep 04 '24 02:09 synesthesiam