Skip to content

Add orientation transforms to stream#77439

Merged
uvjustin merged 15 commits into
home-assistant:devfrom
uvjustin:add-rotation-stream
Sep 6, 2022
Merged

Add orientation transforms to stream#77439
uvjustin merged 15 commits into
home-assistant:devfrom
uvjustin:add-rotation-stream

Conversation

@uvjustin
Copy link
Copy Markdown
Contributor

@uvjustin uvjustin commented Aug 28, 2022

Proposed change

As stream is fmp4/mp4 based, we can apply affine transformations without transcoding by simply modifying the transformation matrix in the mp4 metadata.
The still images generated by stream have always involved decoding a keyframe and reencoding it to jpeg. The additional work of applying a transformation is minimal as it only involves adding a simple numpy operation to the pipeline.
This PR updates stream to change the orientation of video and the generated still images using the 8 EXIF orientations as guidelines. Note that for video, the application of these transforms is player dependent and may only work in browser for now. Also, orientations 5 and 7 do not appear to work properly for video.
This PR also adds an orientation field to StreamSettings and exposes this as a property in Stream.
In Camera, an orientation property is added to CameraEntityPreferences and this is exposed to the frontend via the camera/update_prefs websocket endpoint. This is essentially how preload_stream is currently stored/handled.

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • 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
  • The code has been formatted using Black (black --fast 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.
  • Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

  • No score or internal
  • 🥈 Silver
  • 🥇 Gold
  • 🏆 Platinum

To help with the load of incoming pull requests:

@probot-home-assistant
Copy link
Copy Markdown

Hey there @hunterjm, @allenporter, mind taking a look at this pull request as it has been labeled with an integration (stream) you are listed as a code owner for? Thanks!
(message by CodeOwnersMention)

@uvjustin
Copy link
Copy Markdown
Contributor Author

It might be preferable to expose/store this option similar to how keepalive is exposed. That way the values can be updated more easily and on the fly, and we also don't have to plumb it through an option on every integration that uses stream.

@uvjustin
Copy link
Copy Markdown
Contributor Author

Added a frontend PR to use this feature.

@emontnemery
Copy link
Copy Markdown
Contributor

emontnemery commented Aug 29, 2022

Not directly related to this PR, but maybe camera's custom store should be deprecated in favor of entity registry options which is used for similar purposes by number, sensor and weather?

@uvjustin
Copy link
Copy Markdown
Contributor Author

Not directly related to this PR, but maybe camera's custom store should be deprecated in favor of entity registry options which is used for similar purposes by number, sensor and weather?

I wasn't familiar with the entity registry options but had a look...doesn't that require the camera entities to all have unique IDs?

@uvjustin
Copy link
Copy Markdown
Contributor Author

@emontnemery Since many cameras still don't use config flows, switching to the entity registry doesn't seem like an option.
However, since this is a new feature, we can restrict its use to entities in the entity registry. I've pushed a commit which stores the orientation preferences in the entity registry options, separately from the preload stream preferences. The preload stream preferences can be migrated another time.

Copy link
Copy Markdown
Contributor

@allenporter allenporter left a comment

Choose a reason for hiding this comment

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

This is really cool. A few comments below...

Comment thread homeassistant/components/camera/prefs.py Outdated
Comment thread homeassistant/components/stream/core.py Outdated
Comment thread homeassistant/components/stream/recorder.py Outdated
Comment thread tests/components/camera/test_init.py
Comment thread tests/components/camera/test_init.py Outdated
@uvjustin uvjustin merged commit 852b0ca into home-assistant:dev Sep 6, 2022
Comment thread homeassistant/components/camera/__init__.py
Comment thread homeassistant/components/camera/prefs.py
@uvjustin uvjustin mentioned this pull request Sep 6, 2022
23 tasks
uvjustin added a commit to uvjustin/home-assistant that referenced this pull request Sep 6, 2022
MartinHjelmare pushed a commit that referenced this pull request Sep 6, 2022
@github-actions github-actions Bot locked and limited conversation to collaborators Sep 7, 2022
@bramkragten
Copy link
Copy Markdown
Member

We should move these settings (both preload and orientation) to the entity registry, as we now have options in the entity registry

@uvjustin
Copy link
Copy Markdown
Contributor Author

I don't quite follow. This PR already puts the new orientation options in the entity registry.
As mentioned above, it doesn't seem like preload can be moved into the entity registry as many camera integrations still don't use unique IDs.

@bramkragten
Copy link
Copy Markdown
Member

OK, sorry then. Then I would suggest to use the normal api's to update the entity registry instead of camera/update_prefs

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants