Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Secure Video #1200

Merged
merged 3 commits into from
Jan 9, 2022
Merged

Secure Video #1200

merged 3 commits into from
Jan 9, 2022

Conversation

california444
Copy link
Contributor

WORK IN PROGRESS

This PR depends on the integration of:

Please understand this PR as a proof of concept or an initial version. I don't find enough time to make more progress , but I wanted to share the code as a lot of you have asked for it.
Big thanks to koush, I took over a lot of the "complex stuff" from his code base and tried to make it work with camera-ffmpeg and made more modifications and optimisations.
It is working more or less stable for me...

What is included:

  • 2 new options in the settings under the experimental section:
    -- recording: if enabled, the Secure Video Services are activated
    -- prebuffer: if enabled the videostream from the camera gets continuously recorded and the prebuffer is filled
  • if motion and/or doorbell is enabled for a camera, the linked motion/doorbell services from Secure Video are reused
  • motion/doorbell events trigger the handleFragments request of the new reocrdingDelegate
  • fmp4 fragmentation according to the secure video spec
  • prebuffering if activated

Open tasks:

  • currently libx264 is hardcoded as codec. The video code should be configured according to the existing logic as for regular streaming
  • audio is disabled as it caused some problems in my setup.
  • not sure how good error situations are handled. I guess it should be made a lot more robust
  • I haven't tested how stable ffmpeg works if it runs "forever" for the prebuffering. Maybe it is a good idea to restart it once a while
  • log outputs, especially of ffmpeg can definitely be improved
  • maybe more options in the settings like configuring the prebuffer length
  • ....

@Sunoo
Copy link
Owner

Sunoo commented Sep 22, 2021

Fantastic. I'm happy to finish this up after the prerequisites are in place. Thanks so much for your efforts so far. I'll try to find time to really review this code this weekend, then I'll probably pull it into a branch here.

@Stetocasa
Copy link

@Sunoo any potential beta release date to test HKSV on your Camera plugin?

@Sunoo
Copy link
Owner

Sunoo commented Oct 1, 2021

@Stetocasa It should be shortly after HAP-NodeJS and Homebridge merge in support.

@Stetocasa
Copy link

Thanks @Sunoo do you know which issue follow on GitHub to know on time for the merge release ?

@Sunoo
Copy link
Owner

Sunoo commented Oct 1, 2021

@Stetocasa This PR depends on homebridge/HAP-NodeJS#904 and homebridge/homebridge#2973

@california444
Copy link
Contributor Author

Actually there is a third, optional PR directly against the HAP-NodeJS fork from koush to add doorbell support
koush/HAP-NodeJS#1.
It seems like due to missing rights I can not add commit to the PR in HAP-NodeJS

@Sunoo
Copy link
Owner

Sunoo commented Oct 1, 2021

@california444 Ah, wasn’t aware of that one, thanks for pointing that out. If that doesn’t make it in the next Homebridge release for whatever reason, I’d probably merge support for motion to this plugin, then add doorbell later whenever that gets pulled in.

@bdruth
Copy link

bdruth commented Oct 8, 2021

@california444 I'm getting the same errors locally when tsc runs that the first NodeJS build in the CI workflow ran into. What did you use to run this locally?

@Sunoo
Copy link
Owner

Sunoo commented Oct 8, 2021

@bdruth The issue is almost certainly just that it’s depending on versions of HAP-NodeJS and Homebridge that haven’t been published anywhere yet.

@bdruth
Copy link

bdruth commented Oct 8, 2021

@bdruth The issue is almost certainly just that it’s depending on versions of HAP-NodeJS and Homebridge that haven’t been published anywhere yet.

Hmm, ok ... I'm trying to use yalc locally to avoid that, and it seems to be referencing things right (worked to get your homebridge PR branch to compile against koush's HAP-NodeJS PR branch) ... but not working as well for the plugin :(.

@bdruth
Copy link

bdruth commented Oct 8, 2021

Also, quick-followup, when I look closer at the errors, they seem odd for just being issues around not seeing the right dependent code, e.g.:

src/streamingDelegate.ts:198:24 - error TS2565: Property 'recordingDelegate' is used before being assigned.

198         delegate: this.recordingDelegate

@Sunoo
Copy link
Owner

Sunoo commented Oct 8, 2021

I haven’t had a chance to really dive into the PR yet, so I can’t speak to the code quality. (Real life has been pretty hectic.) I agree that error does seem potentially like an issue in the code though, not dependencies.

@bdruth
Copy link

bdruth commented Oct 8, 2021

@california444 I tried building this against your PR branch to koush's HAP-NodeJS fork, but even then, I'm still getting a bunch of errors. TypeScript: not.a.fan. Gah. As near as I can tell, your code's all valid, it's just that TypeScript hates it.

@california444
Copy link
Contributor Author

@bdruth in tsconfig.json try setting strict to false. I remember I changed something to make progress faster ;-) maybe I did not turn it back before committing…

@bdruth
Copy link

bdruth commented Oct 8, 2021

Hehe, that'll do it.

@Sunoo
Copy link
Owner

Sunoo commented Oct 8, 2021

I usually have my TypeScript environment configured to be pretty paranoid, it helps catch issues that could bite you down the line.

Sounds like I’ve got some cleanup to do to make this code play nice with that. :P

@california444
Copy link
Contributor Author

Some clean up must be done for sure…
Some of the unsuccessful checks will go away once the other PRs are merged and referenced correct.
And some problems are in code segments which I expect to be changed anyway like ffmpeg logging…
It is definitely not a final PR. But I wanted to share my code as I don‘t find the time to further work on right now..

@Sunoo
Copy link
Owner

Sunoo commented Oct 8, 2021

I appreciate the effort you put toward this, and am happy to take it the last but once the remaining blocks are in place.

@bdruth
Copy link

bdruth commented Oct 8, 2021

I'm so close to having this compiling, to where I'm in a position to be able to contribute, but for some reason I'm still getting this:

src/streamingDelegate.ts:196:11 - error TS2322: Type '{ prebufferLength: number; eventTriggerOptions: number; mediaContainerConfigurations: { type: number; fragmentLength: number; }[]; video: { codec: { profiles: H264Profile[]; levels: H264Level[]; }; resolutions: [...][]; }; audio: { ...; }; motionService: true; doorbellService: true; }' is not assignable to type 'CameraRecordingOptions'.
  Object literal may only specify known properties, and 'doorbellService' does not exist in type 'CameraRecordingOptions'.

196           doorbellService:true
              ~~~~~~~~~~~~~~~~~~~~

  node_modules/homebridge/node_modules/hap-nodejs/dist/lib/controller/CameraController.d.ts:29:9
    29         options: CameraRecordingOptions;
               ~~~~~~~
    The expected type comes from property 'options' which is declared here on type '{ options: CameraRecordingOptions; delegate: CameraRecordingDelegate; }'

And this does seem to be a typing issue, but I don't understand why. I've pulled the branch with the doorbellService changes at this point, so it should be seeing that :/

@bdruth
Copy link

bdruth commented Oct 8, 2021

Nvm - I think I got it. I just needed to rm -rf my node_modules in all the places to pull the updates through.

@bdruth
Copy link

bdruth commented Oct 8, 2021

@california444 I may be misunderstanding the prebuffer approach, but is the gist of that, that camera-ffmpeg is always streaming from the camera, effectively? If so, why does the plugin launch a new ffmpeg process when opening the camera stream in Home? Would it be possible to just shunt whatever stream is being used for prebuffer?

@california444
Copy link
Contributor Author

@california444 I may be misunderstanding the prebuffer approach, but is the gist of that, that camera-ffmpeg is always streaming from the camera, effectively? If so, why does the plugin launch a new ffmpeg process when opening the camera stream in Home? Would it be possible to just shunt whatever stream is being used for prebuffer?

If prebuffer is enabled the camera video is streamed all the time in order to fill the prebuffer so that we have the last 4 seconds of an motion/doorbell event in the buffer.

Please correct me if I’m wrong, but I guess for each open camera stream in HOME the plug-in starts a new ffmpeg process in the current implementation.
If I remember correct ffmpeg can handle multiple output streams. So in general I think your approach could work, but would require a larger rework to support more than one video stream. And for a regular stream you don‘t want to have the prebuffer. So some more logic to differentiate between the streams….

@bdruth
Copy link

bdruth commented Oct 9, 2021

Yeah, I realized as I was thinking about it that at the very least, we need another ffmpeg process to create the SRTP output to the mac/iOS device that's requesting the stream. Would be interesting if we could divert (via pipe: maybe?) the original stream at that point (sans prebuffer, good point on that).

My ultimate goal is to minimize the time between tapping on a camera feed in Home and having the live stream start. My Eufy cam (HKSV native) is pretty quick on that front.

@seydx
Copy link

seydx commented Oct 10, 2021

Hey @california444

I used your HSV implementation for my homebridge-camera-ui plugin and modified it slightly (to make it compatible with the user interface)

https://github.com/SeydX/homebridge-camera-ui/tree/hsv

HSV works ok so far with no errors, only FFmpeg doesn't always cooperate well. When I manually trigger a movement, sometimes the following error message comes up: Error writing trailer of tcp://127.0.0.1:34837: Broken pipe.

Due to the error, the recording is not saved with HSV (missing in the timeline) but the recording exists (because it is also passed to the user interface and processed).

Have you had this error message?

Also, I think the prebuffering is awesome, thanks for that.

A small note: If the connection to the camera is interrupted and re-established, it would be great if the prebuffering is restarted or?

@california444
Copy link
Contributor Author

@seydx good to hear it is working in general. I did not experience this problem. When does this error occur. Right at the beginning of the stream? Maybe the debug logs provide more information?

@seydx
Copy link

seydx commented Oct 10, 2021

@california444

I have not been able to debug it any further. Error occurs only at the end when recording is finished

@california444
Copy link
Contributor Author

@seydx did you change the prebuffer length? There is a 30s timeout in one of the servers. If the prebuffer +fragments exceed 30s that could maybe an issue…

@seydx
Copy link

seydx commented Oct 10, 2021

@california444

No, I had only played with the video duration, but the prebuffer length was always at 4000

@arkadicolson
Copy link

I would like to test this to. What's the easiest way to get the code on my homebridge?

@github-actions github-actions bot added the stale label Nov 8, 2021
@github-actions github-actions bot closed this Nov 15, 2021
@Sunoo Sunoo removed the stale label Nov 15, 2021
@Sunoo Sunoo reopened this Nov 15, 2021
@github-actions github-actions bot added the stale label Dec 13, 2021
@github-actions github-actions bot closed this Dec 20, 2021
@Sunoo Sunoo added long running and removed stale labels Dec 20, 2021
@Sunoo Sunoo reopened this Dec 20, 2021
@california444
Copy link
Contributor Author

@Sunoo There is a new HAP-NodeJS PR 920, which will hopefully bring HSV support soon. As far as I can see the API has changed compared to the initial PR 904, so that this PR here will no longer work and needs to be modified.
Are you going to take over this PR? I probably will not find enough time in the near future to make it work with acceptable code quality....

@Sunoo
Copy link
Owner

Sunoo commented Jan 4, 2022

@california444 Yea, I’ve already been looking into that new PR. Now that it seems support is actually coming, I’m happy to start working on this in earnest. Hopefully will put a bunch of work in today, but we’ll see how my day job goes.

@adx74
Copy link

adx74 commented Jan 6, 2022

Hi, HAP-NodeJS PR 920 has been merged.

That’s mean that we can now have HSV ?
And thanks @Sunoo for this work !

@Sunoo
Copy link
Owner

Sunoo commented Jan 6, 2022

@adx74 My plan is to get at least initial support in place on my plugin this weekend. Homebridge will still need to also merge that PR before it is easily usable by everyone though.

@adx74
Copy link

adx74 commented Jan 6, 2022

@Sunoo ofc, did you think HomeBridge gonna add this quickly ?

@Sunoo
Copy link
Owner

Sunoo commented Jan 6, 2022

I would expect it not to take too long, most of the changes were in HAP.

@california444
Copy link
Contributor Author

With the changed API maybe more exports are now required in the homebridge PR.
@Sunoo I am not at home right now. If you find missing exports feel free to add them to the homebridge PR. Otherwise I try to look at it as soon as possible.

@Sunoo
Copy link
Owner

Sunoo commented Jan 6, 2022

@california444 I’m definitely gonna take over this PR. I’ll try to take a look at the HB PR after my day job today, my recollection is that it was pretty slim though.

@Sunoo Sunoo changed the base branch from master to hksv January 9, 2022 16:34
@Sunoo Sunoo merged commit c75c741 into Sunoo:hksv Jan 9, 2022
"type": "boolean",
"required": true,
"default": false,
"description": "Enables Homekit Secure Video and records video on motion and doorbell events. Requires a home hub and iCloud plan with at least 200GB of storage."
Copy link

@nanosonde nanosonde Jan 10, 2022

Choose a reason for hiding this comment

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

Is this comment still true? IMHO, Apple changed this. The 50GB plan also offers support for a Homekit Secure Video Camera. However, only one is supported.
See https://support.apple.com/en-us/HT201238


this.log.debug("Start recording...", this.cameraName);

const session = await this.startFFMPegFragmetedMP4Session(this.videoProcessor, ffmpegInput, audioArgs, videoArgs);

Choose a reason for hiding this comment

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

Typo in startFFMPegFragmetedMP4Session

@schwie
Copy link

schwie commented Jan 26, 2022

Presumably the team is already aware of this, if not it could be helpful:

https://github.com/Supereg/secure-video-specification

@albinmedoc
Copy link

What happend with this PR. This feature doesn't exist in the hksv branch... even if it has been merged into it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants