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

Camera plugin to implement MAVLink Camera Protocol client interface #292

Closed
6 of 10 tasks
shakthi-prashanth-m opened this issue Feb 27, 2018 · 28 comments
Closed
6 of 10 tasks
Assignees
Labels

Comments

@shakthi-prashanth-m
Copy link
Contributor

shakthi-prashanth-m commented Feb 27, 2018

Its is required to add a plugin to configure Camera as per https://mavlink.io/en/protocol/camera.html to interface Camera Streaming Daemon.

To start with, Camera plugin should

  • listen to heartbeat from Camera component
  • get basic camera information
  • configure camera settings
  • send start image capture
  • send stop image capture
  • get information about captured image

UPDATE:
@julianoes added camera plugin as part of #338 Its WIP. Thanks @julianoes .

I would add on top of it:

  • start video streaming
  • request video stream info
  • configure video streaming
  • stop video streaming
@julianoes
Copy link
Collaborator

I can add start, stop, captured image.

@julianoes julianoes changed the title Camera plugin to interface Camer streaming daemon Camera plugin to interface camera streaming daemon Feb 27, 2018
@hamishwillee
Copy link
Collaborator

  1. We should also consider fetching the Camera Definition File - advanced camera information
  2. The Camera Streaming Daemon also supports RTSP streams and (optionally) stream discovery (via Avahi). This probably isn't part of the same API, but we should consider if there is any sensible Dronecore integration point for that too.

@hamishwillee
Copy link
Collaborator

hamishwillee commented Feb 27, 2018

PS and probably we should rename this "Camera plugin to interface MAVLink Camera Protocol". The CSD is just one possible implementation of that protocol. Much of this might even work directly on a PX4 based camera.

@shakthi-prashanth-m
Copy link
Contributor Author

shakthi-prashanth-m commented Mar 3, 2018

@julianoes as discussed about video streaming feature addition, below is the sequence flow for the same. Please review.
https://mavlink.io/en/messages/common.html#MAV_CMD_VIDEO_START_STREAMING is WIP. But however CSD streams video using RTSP. So, probably we need to add a MAVLink command to get RTSP streaming URL which can be rendered using gstreamer. (added in the below sequence flow).

@hamishwillee

image

@mrpollo
Copy link
Member

mrpollo commented Mar 4, 2018

I think diagram is ok to illustrate how a plugin works but I can tell you the steps aren't going to be the same on every camera or product that integrates a camera, I would love to see this implemented in a way that every product vendor could extend this abstract camera plugin and build on top of it, @julianoes would this (abstract camera class) work with your implementation? is this useful?

@shakthi-prashanth-m
Copy link
Contributor Author

@mrpollo , every camera vendor should support MAVLink Camera protocol. So camera plugin is essentially a client to MAVLink Camera protocol compliment server. I didn't get your point about abstract camera class.

@hamishwillee
Copy link
Collaborator

@shakthi-prashanth-m @mrpollo

I agree with @shakthi-prashanth-m. The protocol pretty much exists to abstract the camera differences away from a "usage" point of view. So the steps will be much the same for every single camera and the DroneCore camera plugin will be very much "concrete". Whether they are the above steps in the diagram is a matter for debate.

There are some differences between supported camera settings. These are fetched from a camera definition file and the values are set using messages that have key/values. This is perhaps what Ramon was talking about - arguably a vendor might want to provide a nice front-end for getting and setting their particular key/values (settings). I certainly don't see how DroneCore can do anything useful here other than providing a generic API for making it easy to set/get parameter groups.

My understanding is that the MAVLink commands to get RTSP stream information have not been fully implemented. Instead Avahi can be used to query streams. @lbegani is the best person to comment on how to implement that part of the flow.

@lbegani can you comment on #292 (comment) (best way to integrate video streaming/MAVLink into an external API that uses CSD)

@shakthi-prashanth-m
Copy link
Contributor Author

I discussed with Lalit earlier he said it's WIP about MAVLink commands for video streaming but streaming feature itself exists.

I see that command for getting video streaming info already exists:
https://mavlink.io/en/messages/common.html#MAV_CMD_REQUEST_VIDEO_STREAM_INFORMATION in response to which CSD should return
https://mavlink.io/en/messages/common.html#VIDEO_STREAM_INFORMATION.

If they are WIP, I am fine to work on it so that camera plugin can do video streaming.

@hamishwillee
Copy link
Collaborator

Sounds good.

The only bit that perhaps is a real question is the parsing of the camera definition and the way that the parameters are set. We might want to design our implementation to allow a manufacturer to provide a nicer layer over that bit for their own camera.

@lbegani
Copy link
Contributor

lbegani commented Mar 5, 2018

@shakthi-prashanth-m

  1. I think the target should be provide generic camera interfaces as a part of SDK, interfaces that would comply with MAVLink Camera Protocol (as @hamishwillee suggested, rename the task to "Camera plugin to interface MAVLink Camera Protocol")
  2. MAVLink Camera Protocol is not specific to any camera. Hence, the interfaces based on the protocol would also be generic. Nevertheless, it should still be possible to design it in a way so as to enable vendors to extend the plugin or customize in the way they want (as @mrpollo suggested)
  3. To test the interfaces, we can run Camera Server (CSD) at the vehicle end.
  4. MAVLink commands for video streaming were WIP and perhaps outdated now. I will take a relook on what needs to be done for these msgs. I would suggest skip WIP messages unless you want to first work on making them mainline, which would be an effort taking your focus away from camera plugin work
  5. Can you add an initial PR with basic camera functionality?

@shakthi-prashanth-m
Copy link
Contributor Author

@lbegani OK Sure. I'll add it soon. Thanks.

@julianoes
Copy link
Collaborator

I think we shouldn't have the streaming logic in the same diagram as the camera definition. They don't really depend on each, I think.

As said before, I'll add the camera definition part and simple camera commands because I already have begun that work internally.

Therefore, it would be great if @shakthi-prashanth-m can add the streaming stuff since I don't know how that works.

@lbegani
Copy link
Contributor

lbegani commented Mar 5, 2018

@julianoes I am not sure if it makes sense to add camera features before the basic infrastructure for camera is there in the SDK. @shakthi-prashanth-m and myself started working on this. Can you publish your proposal or WIP code?

@julianoes
Copy link
Collaborator

I will push it as soon as I get to it.

However, I don't think you depend on the basic functionality that I provide though.

@shakthi-prashanth-m shakthi-prashanth-m changed the title Camera plugin to interface camera streaming daemon Camera plugin that implements MAVLink Camera Protocol client interface Mar 5, 2018
@lbegani
Copy link
Contributor

lbegani commented Mar 5, 2018

Camera Plugin must define interfaces for the following -

  • Discover Camera Components (heartbeats)
  • Get/Process Camera Information
  • Get/Process Storage Information
  • Fetch Camera Definition File
  • Parse and export the parameters and its possible values
  • Get/Set/Reset Parameters
  • Start/Stop Image Capture
  • Start/Stop Video Capture

@shakthi-prashanth-m shakthi-prashanth-m changed the title Camera plugin that implements MAVLink Camera Protocol client interface Camera plugin to implement MAVLink Camera Protocol client interface Mar 5, 2018
@shakthi-prashanth-m
Copy link
Contributor Author

shakthi-prashanth-m commented Mar 6, 2018

As per the current model, a Camera application may look like this:

DroneCore drone_conn, camera_conn;

drone_conn.add_udp_connection(14540);    // 14540 for autopilot
camera_conna.add_udp_connection(24540);  // 24540 for camera

while (!drone_conn.is_connected() ||
       !camera_conn.is_connected())
    sleep_for(seconds(1); // wait for both autopilot and camera to be ready.

Device &camera = camera_conn.get_device();
camera.start_video_capture();

Device &drone = drone_conn.get_device();
auto offboard = std::make_shared<Offboard>(&drone);
offboard.set_velocity_ned(....);
...

camera.stop_video_capture();

Does it make sense ? @julianoes @hamishwillee

@hamishwillee
Copy link
Collaborator

Kind of. What if there are multiple cameras (and yes, that is a realistic scenario).

@lbegani
Copy link
Contributor

lbegani commented Mar 6, 2018

drone_conn.add_udp_connection(14540);    // 14540 for autopilot
camera_conna.add_udp_connection(24540);  // 24540 for camera

Both autopilot and camera component send the heartbeat to same UDP port (14550)

@shakthi-prashanth-m
Copy link
Contributor Author

I think, we can't use DroneCore class for handling camera connection. Because, we handle only Autopilot MAVLink messages in core and we need to handle whole camera connection handling in plugin itself.

What if there are multiple cameras

Yes, we have to handle multiple cameras as well, so it makes sense to do discover, connect, etc in camera plugin itself rather than re-using DroneCore class for that. In that case, how does it become DroneCore plugin ? :) Of-course, DroneCore applications can use it. But that is even possible if it exists outside as well. Isn't it ?

@shakthi-prashanth-m
Copy link
Contributor Author

shakthi-prashanth-m commented Mar 6, 2018

Both autopilot and camera component send the heartbeat to same UDP port (14550)

Right now, we can't listen on the same port for both autopilot and camera. So, we have to choose another well-known port say, 14550. (We have to make sure not to launch QGC same time).

@julianoes
Copy link
Collaborator

I think it should look like that:

DroneCore dc;

dc.add_udp_connection(14540);    // for autopilot
dc.add_udp_connection(24540);  // for camera

Autopilot and camera should then have both the same system ID but different component IDs. As @shakthi-prashanth-m suggested we need proper support for multiple components (#304).

For the case of multiple camera, we use more UDP connections if required, and we distinguish them by component ID, see:
http://mavlink.org/messages/common#MAV_COMP_ID_CAMERA
http://mavlink.org/messages/common#MAV_COMP_ID_CAMERA2
etc.

@shakthi-prashanth-m
Copy link
Contributor Author

I agree that we distinguish camera components by component id.

For the case of multiple camera, we use more UDP connections if required,

All camera components deal with a single UDP port, I think. Consider, CSD for example. It will use port 14550 for all camera components. @lbegani is that right ?

@julianoes, with multiple component support in DroneCore, we can use same port for autopilot and camera isn't it ? We don't request for Autopilot version if Heartbeat has MAV_TYPE_CAMERA, etc ?

@lbegani
Copy link
Contributor

lbegani commented Mar 6, 2018

Regarding UDP connections, there may be multiple scenarios that need to be considered. We may not to support all of them

  1. Both autopilot and camera components sending mavlink messages to same port
  2. Autopilot to one port (X), camera components to another port (Y)
  3. Autopilot to one port (X), camera component to different ports (Y,Z, ...)

@julianoes
Copy link
Collaborator

julianoes commented Mar 6, 2018

@julianoes, with multiple component support in DroneCore, we can use same port for autopilot and camera isn't it ?

Yes, we can but it all depends on how it is all set up. For Yuneec's case, e.g. the camera and autopilot are on the same UDP port because the autopilot communicates "through" the camera.

We don't request for Autopilot version if Heartbeat has MAV_TYPE_CAMERA, etc ?

Correct, we shouldn't.

Regarding UDP connections, there may be multiple scenarios that need to be considered.

Right. I'm not sure where 3. is needed though.

@hamishwillee
Copy link
Collaborator

Autopilot and camera should then have both the same system ID but different component IDs. As @shakthi-prashanth-m suggested we need proper support for multiple components (#304).

A camera will be associated with a device and share its system ID. Do we need to consider systems where a mavlink camera might exist separately from a device?

I think we shouldn't have the streaming logic in the same diagram as the camera definition. They don't really depend on each, I think.

Jumping back a big, I agree with @julianoes on the above. Even if camera vs streaming operations can be handled together I think it makes sense for readers to see them separately.

@hamishwillee
Copy link
Collaborator

As an aside - CSD does not yet support video capture. We can still define the interface, but they won't work yet.

@julianoes
Copy link
Collaborator

@shakthi-prashanth-m can we close this?

@shakthi-prashanth-m
Copy link
Contributor Author

Yes, we can! And it is already closed :-)

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

No branches or pull requests

6 participants