hjdhjd / homebridge-unifi-protect

:video_camera: Complete HomeKit integration for all UniFi Protect device types with full support for most features including HomeKit Secure Video, and more. https://homebridge.io
Other
1.39k stars 83 forks source link

Add ability to crop video streams+snapshots #1034

Closed dansimau closed 5 months ago

dansimau commented 6 months ago

This PR adds the ability to crop streams and snapshots.



Demo & use case

Before:

IMG_7944

IMG_7945

New configuration options:

Screenshot 2024-01-07 at 16 45 02

Result:

IMG_7950 IMG_7949

hjdhjd commented 6 months ago

Thanks for the contribution - I’ll review the code and provide some feedback. One overall bit of feedback though: I will be unlikely to accept merging this if there’s an additional dependency or need for sharp/libvps. You should be able to do snapshots via FFmpeg (in fact, in some of the early releases of HBUP I did exactly that). I don’t want to introduce addditional tooling requirements for HBUP unless they’re absolutely essential, and in this case, FFmpeg is more than capable of doing the job.

I haven’t looked at the code yet, but the first bit of feedback is: please shift this to using FFmpeg exclusively.

Thanks and look forward to collaborating and seeing a future version of this functionality incorporated.

hjdhjd commented 6 months ago

Feedback provided. 😄

dansimau commented 6 months ago

@hjdhjd addressed feedback and snapshot cropping to use ffmpeg. Let me know if you have further comments.

Thanks!

github-actions[bot] commented 6 months ago

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

dansimau commented 6 months ago

Updated docs and made final tweaks. This is now ready for final review + merge from my point of view. Thanks!

github-actions[bot] commented 5 months ago

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

dansimau commented 5 months ago

@hjdhjd are you able to re-open this and take a look?

hjdhjd commented 5 months ago

Never fear…I will. I’m just buried in the real world right now. 😄 I will get to it in the next two weeks and sooner as I can. Please accept my apologies.

github-actions[bot] commented 5 months ago

This issue is locked to prevent necroposting on closed issues. Please create a new issue for related discussion, if needed.

hjdhjd commented 5 months ago

I’ll be dealing with this one over the weekend - appreciate your patience and understanding.

hjdhjd commented 5 months ago

@dansimau I'm reviewing the PR this week/weekend and will test it out and incorporate or revert for additional updates. I have to say overall, once more, one of the more elegant PRs I've received for HBUP.

hjdhjd commented 5 months ago

Merging this in, and will make edits to refine. Thanks for the contribution!

dansimau commented 5 months ago

Thanks for accepting! Sorry I didn't get to the edits in time.

github-actions[bot] commented 4 months ago

This issue is locked to prevent necroposting on closed issues. Please create a new issue for related discussion, if needed.