Skip to content

Select default mime type when empty #303

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
May 30, 2023
Merged

Select default mime type when empty #303

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions src/viam/components/camera/service.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from typing import Dict
from google.api.httpbody_pb2 import HttpBody
from grpclib.server import Stream

Expand Down Expand Up @@ -26,6 +27,7 @@ class CameraRPCService(CameraServiceBase, ResourceRPCServiceBase[Camera]):
"""

RESOURCE_TYPE = Camera
_camera_mime_types: Dict[str, CameraMimeType] = {}

async def GetImage(self, stream: Stream[GetImageRequest, GetImageResponse]) -> None:
request = await stream.recv_message()
Expand All @@ -39,6 +41,16 @@ async def GetImage(self, stream: Stream[GetImageRequest, GetImageResponse]) -> N
timeout = stream.deadline.time_remaining() if stream.deadline else None
image = await camera.get_image(request.mime_type, timeout=timeout, metadata=stream.metadata)
try:
if not request.mime_type:
if camera.name not in self._camera_mime_types:
props = await camera.get_properties()
if props.supports_pcd:
self._camera_mime_types[camera.name] = CameraMimeType.PCD
else:
self._camera_mime_types[camera.name] = CameraMimeType.JPEG

request.mime_type = self._camera_mime_types[camera.name]
Copy link
Member

Choose a reason for hiding this comment

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

Okay so I'm not sure if we want to do any of this client side (is this client side lol?). The idea is that an empty request should be able to slide past. The server side should determine the mime type using logic like ^^ (like in RDK). I guess my question is why would we choose the request mimetype and not the response?

Copy link
Member Author

Choose a reason for hiding this comment

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

@kharijarrett this is the server side but in python. So if someone is making a modular camera in python, rpc requests coming to that module would hit this code.

Copy link
Member

Choose a reason for hiding this comment

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

Makes all the difference in the world lol this LGTM!


mimetype, is_lazy = CameraMimeType.from_lazy(request.mime_type)
if CameraMimeType.is_supported(mimetype):
response_mime = mimetype
Expand Down
4 changes: 2 additions & 2 deletions tests/mocks/components.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@ def __init__(self, name: str):
self.image = Image.new("RGBA", (100, 100), "#AABBCCDD")
self.point_cloud = b"THIS IS A POINT CLOUD"
self.props = Camera.Properties(
True,
False,
IntrinsicParameters(width_px=1, height_px=2, focal_x_px=3, focal_y_px=4, center_x_px=5, center_y_px=6),
DistortionParameters(model="no_distortion"),
)
Expand All @@ -372,7 +372,7 @@ async def get_image(self, mime_type: str = "", timeout: Optional[float] = None,

async def get_point_cloud(self, *, timeout: Optional[float] = None, **kwargs) -> Tuple[bytes, str]:
self.timeout = timeout
return self.point_cloud, CameraMimeType.PCD.value
return self.point_cloud, CameraMimeType.PCD

async def get_properties(self, *, timeout: Optional[float] = None, **kwargs) -> Camera.Properties:
self.timeout = timeout
Expand Down
7 changes: 6 additions & 1 deletion tests/test_camera.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ def point_cloud() -> bytes:
@pytest.fixture(scope="function")
def properties() -> Camera.Properties:
return Camera.Properties(
True,
False,
IntrinsicParameters(width_px=1, height_px=2, focal_x_px=3, focal_y_px=4, center_x_px=5, center_y_px=6),
DistortionParameters(model="no_distortion"),
)
Expand Down Expand Up @@ -142,6 +142,11 @@ async def test_get_frame(self, camera: MockCamera, service: CameraRPCService, im
assert img == image
assert response.mime_type == "unknown"

# Test empty mime type. Empty mime type should default to JPEG for non-depth cameras
request = GetImageRequest(name="camera")
response: GetImageResponse = await client.GetImage(request)
assert service._camera_mime_types["camera"] == CameraMimeType.JPEG

@pytest.mark.asyncio
async def test_render_frame(self, camera: MockCamera, service: CameraRPCService, image: Image.Image):
assert camera.timeout is None
Expand Down