Add homekit camera support

[xdissent]

Proposed change

Add camera support to the HomeKit integration. Based on camera accessory from pyhap.

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)
• Breaking change (fix/feature causing existing functionality to break)
• Code quality improvements to existing code or addition of tests

Example entry for configuration.yaml:

# Example configuration.yaml
stream:

ffmpeg:

camera:
  - platform: ffmpeg
    name: FfmpegCam
    input: http://some/ffmpeg-compatible/stream
  - platform: mjpeg
    name: MjpegCam
    mjpeg_url: http://mjpeg/webcam/?action=stream
    still_image_url: http://mjpeg/webcam/?action=snapshot
  - platform: generic
    name: StreamCam
    still_image_url: http://rtsphost/screenshot
    stream_source: rtsp://rtsphost:8555/unicast

homekit:
  name: Cam Bridge
  entity_config:
    camera.mjpegcam:
      # Required since mjpeg camera doesn't support stream_source()
      stream_source: http://mjpeg/webcam/?action=stream
    camera.ffmpegcam:
      # Set maximums for negotiating resolutions
      max_fps: 15
      max_width: 640
      max_height: 480
    camera.streamcam:
      # IP address from which ffmpeg will stream to rtp, if different
      stream_address: 10.0.0.1
      # Can override stream_source for camera types that provide it
      stream_source: rtsp://rtsphost:8555/another-stream
      # Opt-in only audio support
      support_audio: True

Additional information

• This PR fixes or closes issue: fixes #
• This PR is related to issue:
• Link to documentation pull request: Add homekit camera docs home-assistant.io#12303

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:

• Documentation added/updated for www.home-assistant.io

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.
• Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

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

[homeassistant]

Hi @xdissent,

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

[Jc2k]

@xdissent Thanks for doing this!

There isn't a maintainer for this integration at the moment but i'll try to stand in a bit as I know the HomeKit protocol from maintaining homekit_controller.

Can you explain whats going on with PyhapCamera a bit? Where you call methods on it and pass them self are a bit weird. It looks like multiple inheritance but without actually inheriting?

[xdissent]

@Jc2k It's weird multiple inheritance! I tried really hard to figure out how to subclass both pyhap camera and ha accessory, but the way the different constructors take their args made it impossible. It's been quite a while since I've been deep into python though, so maybe there is a way.

[xdissent]

@frenck Added docs: home-assistant/home-assistant.io#12303 Did I do that right?

[frenck]

This feature is not going to work for a large part of our userbase: Home Assistant & Home Assistant Core on Docker users.

This is because it relies on an audio feature, which is not compiled in FFmpeg by default.

FFmpeg compile options in our Docker images:

--prefix=/usr --enable-avresample --enable-avfilter --enable-gnutls --enable-gpl --enable-libass --enable-libmp3lame --enable-libvorbis --enable-libvpx --enable-libxvid --enable-libx264 --enable-libx265 --enable-libtheora --enable-libv4l2 --enable-postproc --enable-pic --enable-pthreads --enable-shared --enable-libxcb --disable-stripping --disable-static --disable-librtmp --enable-vaapi --enable-vdpau --enable-libopus --disable-debug

libfdk_aac isn't in the list of encoders either when running ffmpeg -encoders in our containers.

Therefore, I'm afraid this will cause issues for people, since it implies to work, but doesn't.

[Jc2k]

There are 2 possible audio codecs that can be offered to the iDevice AIUI. @frenck from your point of view, if this did some negotiation on our side and only offered aac if it was actually available would it be OK? Are there any utils in the ffmpeg integration for this? A wrapper around ffmpeg -encoders etc.

[Jc2k]

@Jc2k It's weird multiple inheritance! I tried really hard to figure out how to subclass both pyhap camera and ha accessory, but the way the different constructors take their args made it impossible. It's been quite a while since I've been deep into python though, so maybe there is a way.

I think I see what you mean here. But there's quite a bit of code duplication, so I think we need to work with upstream here and find a way to avoid that.

[Jc2k]

Managed to get the inheritance to work here: Jc2k@eff15d7 - this assumes the 2 changes are merged upstream (change to make microphone service optional and change to set v_ssrc).

[stickpin]

@Jc2k, @xdissent I think it will be good to add video codec as a configurable parameter as well with libx264 as a default value.

Some cameras already sending h264 stream and there's no real reason to re-encode it, so in this case, you can just use: -c:v copy

[Jc2k]

Good idea. Ideally we'd do stuff like that automatically rather than needing the user to know. Ideally the ffmpeg infastructure could autodetect this for us. (My rule of thumb is adding more config options are a pain in the butt for QA and a pain in the butt for user support, and homekit doesn't have anyone to do either for it right now).

[bdraco]

@xdissent
I gave this a shot.

1. I was able to get screen caps in Homekit. Nice work!

2. Video had some issues. It might be due to the below though
   

3. I noticed the Chacha20_Poly1305 performance issue noted here Chacha20_Poly1305 performance can result in unresponsive homekit ikalchev/HAP-python#228 bring home assistant to 100% cpu

[stickpin]

@Jc2k, @xdissent sorry for pushing you guys but are you planning to finalize this PR?
If not, I would like to take it over from the point @Jc2k is managed to get to and to try to finalize it.
Or we can collaborate all together to finalize this PR.

I am really interested in having this feature in HASS, so I can finally deprecate Homebridge. :)

[Jc2k]

@stickpin i'm only here with my reviewer hat on, and only contributed Jc2k@eff15d7 so that I could be a stickler about insisting on reducing code duplication :=) i plan to stick aroud and keep reviewing and nitting but thats about it (my priority this cycle is stablising homekit_controller after a big update).

If we are starting from my last commit then I would maybe start with the changes that need pushing upstream:

• v_ssrc (https://github.com/ikalchev/HAP-python/blob/dev/pyhap/camera.py#L695)
• Making the microphone characteristic optional upstream (https://github.com/ikalchev/HAP-python/blob/dev/pyhap/camera.py#L425)
• The _get_streaimg_status typo

They need doing (and a new tag cutting) but don't step on this PR too much. That gives @xdissent time to come back and comment. It's only been 4 days, so seems fair.

[xdissent]

@Jc2k thanks so much for figuring out the inheritance thing. That will really clean things up. As you noted, a good deal needs to go upstream and we're kinda in a holding pattern until that gets released. @stickpin if you wanted to help with that effort or any other improvements then knock yourself out! I should have time in the next couple of days tho.

Regarding ffmpeg options - I do feel like we probably should have some kind of escape hatch in the configuration that lets you fully control the ffmpeg command. But because the parameters of the stream are decided by the client, I'm not sure what that should look like. Using -c:v copy would be great, but only if the client wants a stream that matches the source stream params (which we don't know until you start trying to stream the source).

I'm also wondering if we're not duplicating some of the effort behind the stream integration's request_stream() stuff? I tried using some of that but I think it was maybe too tightly coupled to hls and media recorder right now.

[orrpan]

tested it with CamON Live Streaming android app (rtsp), worked perfectly!

[Jc2k]

Anyone started looking into upstreaming the bits I called out yet?

[xdissent]

@Jc2k I just sent a pr. I didn't make the microphone optional because I took a look at the spec and it's required.

[bdraco]

LGTM, but need another reviewer

[Jc2k]

LGTM, too. Pleased with how the HK bits have turned out. type_cameras.py only 210 lines!

[bdraco]

@Jc2k, @xdissent I think it will be good to add video codec as a configurable parameter as well with libx264 as a default value.

Some cameras already sending h264 stream and there's no real reason to re-encode it, so in this case, you can just use: -c:v copy

@stickpin I don’t have a camera that supports that so I can’t test but if you want to do a follow up PR after this merges, I’m happy to review it.

Any chance you can take the latest version of this branch for a spin?

[bdraco]

Ok to merge if @xdissent confirms everything is working as expected.

[leonardpitzu]

Works OK for me with 2 Foscam cameras. Thanks to all of you for the work!

[stickpin]

@Jc2k, @xdissent I think it will be good to add video codec as a configurable parameter as well with libx264 as a default value.
Some cameras already sending h264 stream and there's no real reason to re-encode it, so in this case, you can just use: -c:v copy

@stickpin I don’t have a camera that supports that so I can’t test but if you want to do a follow up PR after this merges, I’m happy to review it.

Any chance you can take the latest version of this branch for a spin?

@bdraco working on it. will do push soon... :)

[stickpin]

@bdraco, @xdissent, @Jc2k I've added video codec support and submitted PR here: xdissent#1

Everything works as it should with my YI Hack cameras.

I'm not sure how to commit to this PR. :)

[bdraco]

@bdraco, @xdissent, @Jc2k I've added video codec support and submitted PR here: xdissent#1

Everything works as it should with my YI Hack cameras.

I'm not sure how to commit to this PR. :)

Please send a new PR once this one merges

[xdissent]

It looks like -tune zerolatency is breaking my cams, don't know why that would be happening now though. Perhaps the whole ffmpeg cmd should be configurable?

Another issue I see is reconfigure_stream doesn't stop/restart a running stream, it just tries to run another ffmpeg process. Flipping the phone from portrait to landscape is an easy way to force reconfigure_stream for testing.

The asyncio stuff is definitely working again now though. Looks real close!

[bdraco]

It looks like -tune zerolatency is breaking my cams, don't know why that would be happening now though. Perhaps the whole ffmpeg cmd should be configurable?

It looks like -tune zerolatency was in the original commit so I'm not sure what is going on. Ideally we want to minimize the number of options needed to get this working as I'm not looking forward to supporting customizable ffmpeg commands 😢

Another issue I see is reconfigure_stream doesn't stop/restart a running stream, it just tries to run another ffmpeg process. Flipping the phone from portrait to landscape is an easy way to force reconfigure_stream for testing.

I'll take a look at what pyhap is doing under the hood. We can always revert back to the stub what was there before if we need to.

The asyncio stuff is definitely working again now though. Looks real close!

[bdraco]

I couldn't replicate the issue with switching phone orientation but it didn't break anything for me to disable reconfigure stream.

@xdissent All working with the last change?

[bdraco]

/AzurePipelines Run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[xdissent]

Ugh, sorry, I think we want zerolatency back in there... it does seem to effect some cams (stuttering without it) and it doesn't exactly fix my trouble cams. However, just using the copy codec does fix my trouble cams. So, I'll probably just go that route with those whenever that's an option. I'm not getting multiple ffmpeg processes anymore so looks like reconfigure_stream stuff is fixed and everything else looks good!

[bdraco]

Local tests pass, retested ok from homekit. Definitely works better with -tune zerolatency

[bdraco]

I'll merge when the CI passes (unless it doesn't), and then @stickpin you should be good to send in that PR. Hopefully with a test 😄 ❓