From 2e018ad84113955b76e8c58eb07d8c8dab00b41a Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 11 May 2020 00:09:05 -0500 Subject: [PATCH] Make homekit camera snapshots HAP spec compliant (#35299) --- homeassistant/components/homekit/img_util.py | 62 +++++++++++++++++++ .../components/homekit/manifest.json | 2 +- .../components/homekit/type_cameras.py | 10 +-- requirements_all.txt | 3 + requirements_test_all.txt | 3 + tests/components/homekit/common.py | 17 ++++- tests/components/homekit/test_img_util.py | 62 +++++++++++++++++++ tests/components/homekit/test_type_cameras.py | 34 +++++++--- 8 files changed, 180 insertions(+), 13 deletions(-) create mode 100644 homeassistant/components/homekit/img_util.py create mode 100644 tests/components/homekit/test_img_util.py diff --git a/homeassistant/components/homekit/img_util.py b/homeassistant/components/homekit/img_util.py new file mode 100644 index 00000000000..835b04558e6 --- /dev/null +++ b/homeassistant/components/homekit/img_util.py @@ -0,0 +1,62 @@ +"""Image processing for HomeKit component.""" + +import logging + +from turbojpeg import TurboJPEG + +SUPPORTED_SCALING_FACTORS = [(7, 8), (3, 4), (5, 8), (1, 2), (3, 8), (1, 4), (1, 8)] + +_LOGGER = logging.getLogger(__name__) + + +def scale_jpeg_camera_image(cam_image, width, height): + """Scale a camera image as close as possible to one of the supported scaling factors.""" + turbo_jpeg = TurboJPEGSingleton.instance() + if not turbo_jpeg: + return cam_image.content + + (current_width, current_height, _, _) = turbo_jpeg.decode_header(cam_image.content) + + if current_width <= width or current_height <= height: + return cam_image.content + + ratio = width / current_width + + scaling_factor = SUPPORTED_SCALING_FACTORS[-1] + for supported_sf in SUPPORTED_SCALING_FACTORS: + if ratio >= (supported_sf[0] / supported_sf[1]): + scaling_factor = supported_sf + break + + return turbo_jpeg.scale_with_quality( + cam_image.content, scaling_factor=scaling_factor, quality=75, + ) + + +class TurboJPEGSingleton: + """ + Load TurboJPEG only once. + + Ensures we do not log load failures each snapshot + since camera image fetches happen every few + seconds. + """ + + __instance = None + + @staticmethod + def instance(): + """Singleton for TurboJPEG.""" + if TurboJPEGSingleton.__instance is None: + TurboJPEGSingleton() + return TurboJPEGSingleton.__instance + + def __init__(self): + """Try to create TurboJPEG only once.""" + try: + TurboJPEGSingleton.__instance = TurboJPEG() + except Exception: # pylint: disable=broad-except + _LOGGER.exception( + "libturbojpeg is not installed, cameras may impact HomeKit performance." + ) + TurboJPEGSingleton.__instance = False diff --git a/homeassistant/components/homekit/manifest.json b/homeassistant/components/homekit/manifest.json index 3d0c84d31b5..785c7bb2b87 100644 --- a/homeassistant/components/homekit/manifest.json +++ b/homeassistant/components/homekit/manifest.json @@ -2,7 +2,7 @@ "domain": "homekit", "name": "HomeKit", "documentation": "https://www.home-assistant.io/integrations/homekit", - "requirements": ["HAP-python==2.8.3","fnvhash==0.1.0","PyQRCode==1.2.1","base36==0.1.1"], + "requirements": ["HAP-python==2.8.3","fnvhash==0.1.0","PyQRCode==1.2.1","base36==0.1.1","PyTurboJPEG==1.4.0"], "dependencies": ["http", "camera", "ffmpeg"], "after_dependencies": ["logbook"], "codeowners": ["@bdraco"], diff --git a/homeassistant/components/homekit/type_cameras.py b/homeassistant/components/homekit/type_cameras.py index e7bbf4f146b..fe181d56c8c 100644 --- a/homeassistant/components/homekit/type_cameras.py +++ b/homeassistant/components/homekit/type_cameras.py @@ -29,10 +29,12 @@ from .const import ( CONF_VIDEO_MAP, CONF_VIDEO_PACKET_SIZE, ) +from .img_util import scale_jpeg_camera_image from .util import CAMERA_SCHEMA _LOGGER = logging.getLogger(__name__) + VIDEO_OUTPUT = ( "-map {v_map} -an " "-c:v {v_codec} " @@ -246,11 +248,11 @@ class Camera(HomeAccessory, PyhapCamera): def get_snapshot(self, image_size): """Return a jpeg of a snapshot from the camera.""" - return ( + return scale_jpeg_camera_image( asyncio.run_coroutine_threadsafe( self.hass.components.camera.async_get_image(self.entity_id), self.hass.loop, - ) - .result() - .content + ).result(), + image_size["image-width"], + image_size["image-height"], ) diff --git a/requirements_all.txt b/requirements_all.txt index a9330f4b37e..15ccf9e0918 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -78,6 +78,9 @@ PySocks==1.7.1 # homeassistant.components.transport_nsw PyTransportNSW==0.1.1 +# homeassistant.components.homekit +PyTurboJPEG==1.4.0 + # homeassistant.components.vicare PyViCare==0.1.10 diff --git a/requirements_test_all.txt b/requirements_test_all.txt index c0192c7e400..0633fd94c08 100644 --- a/requirements_test_all.txt +++ b/requirements_test_all.txt @@ -23,6 +23,9 @@ PyRMVtransport==0.2.9 # homeassistant.components.transport_nsw PyTransportNSW==0.1.1 +# homeassistant.components.homekit +PyTurboJPEG==1.4.0 + # homeassistant.components.remember_the_milk RtmAPI==0.7.2 diff --git a/tests/components/homekit/common.py b/tests/components/homekit/common.py index 8f38332f112..4ad5f60551d 100644 --- a/tests/components/homekit/common.py +++ b/tests/components/homekit/common.py @@ -1,5 +1,7 @@ """Collection of fixtures and functions for the HomeKit tests.""" -from tests.async_mock import patch +from tests.async_mock import Mock, patch + +EMPTY_8_6_JPEG = b"empty_8_6" def patch_debounce(): @@ -8,3 +10,16 @@ def patch_debounce(): "homeassistant.components.homekit.accessories.debounce", lambda f: lambda *args, **kwargs: f(*args, **kwargs), ) + + +def mock_turbo_jpeg( + first_width=None, second_width=None, first_height=None, second_height=None +): + """Mock a TurboJPEG instance.""" + mocked_turbo_jpeg = Mock() + mocked_turbo_jpeg.decode_header.side_effect = [ + (first_width, first_height, 0, 0), + (second_width, second_height, 0, 0), + ] + mocked_turbo_jpeg.scale_with_quality.return_value = EMPTY_8_6_JPEG + return mocked_turbo_jpeg diff --git a/tests/components/homekit/test_img_util.py b/tests/components/homekit/test_img_util.py new file mode 100644 index 00000000000..4ada89b3acd --- /dev/null +++ b/tests/components/homekit/test_img_util.py @@ -0,0 +1,62 @@ +"""Test HomeKit img_util module.""" +from homeassistant.components.camera import Image +from homeassistant.components.homekit.img_util import ( + TurboJPEGSingleton, + scale_jpeg_camera_image, +) + +from .common import EMPTY_8_6_JPEG, mock_turbo_jpeg + +from tests.async_mock import patch + +EMPTY_16_12_JPEG = b"empty_16_12" + + +def test_turbojpeg_singleton(): + """Verify the instance always gives back the same.""" + assert TurboJPEGSingleton.instance() == TurboJPEGSingleton.instance() + + +def test_scale_jpeg_camera_image(): + """Test we can scale a jpeg image.""" + + camera_image = Image("image/jpeg", EMPTY_16_12_JPEG) + + turbo_jpeg = mock_turbo_jpeg(first_width=16, first_height=12) + with patch( + "homeassistant.components.homekit.img_util.TurboJPEG", return_value=False + ): + TurboJPEGSingleton() + assert scale_jpeg_camera_image(camera_image, 16, 12) == camera_image.content + + turbo_jpeg = mock_turbo_jpeg(first_width=16, first_height=12) + with patch( + "homeassistant.components.homekit.img_util.TurboJPEG", return_value=turbo_jpeg + ): + TurboJPEGSingleton() + assert scale_jpeg_camera_image(camera_image, 16, 12) == EMPTY_16_12_JPEG + + turbo_jpeg = mock_turbo_jpeg( + first_width=16, first_height=12, second_width=8, second_height=6 + ) + with patch( + "homeassistant.components.homekit.img_util.TurboJPEG", return_value=turbo_jpeg + ): + TurboJPEGSingleton() + jpeg_bytes = scale_jpeg_camera_image(camera_image, 8, 6) + + assert jpeg_bytes == EMPTY_8_6_JPEG + + +def test_turbojpeg_load_failure(): + """Handle libjpegturbo not being installed.""" + + with patch( + "homeassistant.components.homekit.img_util.TurboJPEG", side_effect=Exception + ): + TurboJPEGSingleton() + assert TurboJPEGSingleton.instance() is False + + with patch("homeassistant.components.homekit.img_util.TurboJPEG"): + TurboJPEGSingleton() + assert TurboJPEGSingleton.instance() diff --git a/tests/components/homekit/test_type_cameras.py b/tests/components/homekit/test_type_cameras.py index 99692f06836..81dfb9410f0 100644 --- a/tests/components/homekit/test_type_cameras.py +++ b/tests/components/homekit/test_type_cameras.py @@ -15,11 +15,14 @@ from homeassistant.components.homekit.const import ( CONF_VIDEO_CODEC, VIDEO_CODEC_COPY, ) +from homeassistant.components.homekit.img_util import TurboJPEGSingleton from homeassistant.components.homekit.type_cameras import Camera from homeassistant.components.homekit.type_switches import Switch from homeassistant.exceptions import HomeAssistantError from homeassistant.setup import async_setup_component +from .common import mock_turbo_jpeg + from tests.async_mock import AsyncMock, MagicMock, patch MOCK_START_STREAM_TLV = "ARUCAQEBEDMD1QMXzEaatnKSQ2pxovYCNAEBAAIJAQECAgECAwEAAwsBAgAFAgLQAgMBHgQXAQFjAgQ768/RAwIrAQQEAAAAPwUCYgUDLAEBAwIMAQEBAgEAAwECBAEUAxYBAW4CBCzq28sDAhgABAQAAKBABgENBAEA" @@ -135,15 +138,30 @@ async def test_camera_stream_source_configured(hass, run_driver, events): await acc.stop_stream(session_info) await hass.async_block_till_done() - assert await hass.async_add_executor_job(acc.get_snapshot, 1024) + turbo_jpeg = mock_turbo_jpeg( + first_width=16, first_height=12, second_width=300, second_height=200 + ) + with patch( + "homeassistant.components.homekit.img_util.TurboJPEG", return_value=turbo_jpeg + ): + TurboJPEGSingleton() + assert await hass.async_add_executor_job( + acc.get_snapshot, {"aid": 2, "image-width": 300, "image-height": 200} + ) + # Verify the bridge only forwards get_snapshot for + # cameras and valid accessory ids + assert await hass.async_add_executor_job( + bridge.get_snapshot, {"aid": 2, "image-width": 300, "image-height": 200} + ) - # Verify the bridge only forwards get_snapshot for - # cameras and valid accessory ids - assert await hass.async_add_executor_job(bridge.get_snapshot, {"aid": 2}) with pytest.raises(ValueError): - assert await hass.async_add_executor_job(bridge.get_snapshot, {"aid": 3}) + assert await hass.async_add_executor_job( + bridge.get_snapshot, {"aid": 3, "image-width": 300, "image-height": 200} + ) with pytest.raises(ValueError): - assert await hass.async_add_executor_job(bridge.get_snapshot, {"aid": 4}) + assert await hass.async_add_executor_job( + bridge.get_snapshot, {"aid": 4, "image-width": 300, "image-height": 200} + ) async def test_camera_stream_source_configured_with_failing_ffmpeg( @@ -289,7 +307,9 @@ async def test_camera_with_no_stream(hass, run_driver, events): await hass.async_block_till_done() with pytest.raises(HomeAssistantError): - await hass.async_add_executor_job(acc.get_snapshot, 1024) + await hass.async_add_executor_job( + acc.get_snapshot, {"aid": 2, "image-width": 300, "image-height": 200} + ) async def test_camera_stream_source_configured_and_copy_codec(hass, run_driver, events):