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

WIP: Basic video streaming discovery using MAVLink #5271

Closed
wants to merge 5 commits into from

Conversation

Projects
None yet
8 participants
@otaviobp
Copy link
Contributor

otaviobp commented Jun 9, 2017

TL;DR: Adding Basic video streams discovery on QGC using MAVLink messages merged on mavlink/mavlink#677.

Justification:
Following the discussions on previous QGroundControl and mavlink repositories pull requests, as well as in the px4 discuss page and conversation with other developers interested in extending camera support in MAVLink, we concluded that we will need an extended way to send all information related to cameras from the vehicle to the Ground Station, in order to have a nice and custom UI. As @dogmaphobic commented in other pull request, the suggestions on how to do it are being held on this page: http://discuss.px4.io/t/mavlink-camera-api-discussion/3278/21.

But, we have also discussed that it would be important to have a basic video camera support using only MAVLink messages, to be used as a fallback. And this is what I am proposing in this PR: To use MAVLink messages already merged in mavlink repository to discover video streams and present some available options to the user.

How to test it:
We have an implementation of a camera daemon that runs in a drone's companion board and it is used to advertise available cameras and stream videos using RTSP. It is available at https://github.com/01org/camera-streaming-daemon and all MAVLink messages used by this PR are implemented in that daemon. An easy way to test it is to run it on any system that have V4L2 compatible cameras, that will be advertised and streamed by the daemon. If you are running this on a drone you will also need a proxy to redirect camera specific MAVLink messages to the drone. I have tested that with https://github.com/01org/mavlink-router.

Video Support:
So far only RTSP video is supported. It is easy to add support for UDP video streaming though.

Why adding a new Widget:
Instead of adding the streams discovered using MAVLink to the video source ComboBox in General Settings page, I added a new option called "MAVLink Auto Discovery Streams" and created a new Widget to display the discovered streams and other stream settings. Why did I take this approach? Because I think it is important for the user to be able to change the video source and settings and watch the video at the same time and this is not possible if I used the General Settings for this. When you have multiple cameras in a drone, it is not practical to have the video selection in a different screen. Anyway, I accept suggestions regarding the proposed UI.

video streaming: Add a class to handle mavlink video streaming messages
Use HEARTBEAT message with component id MAV_COMP_ID_CAMERA to discover
camera daemons on the network. Class MAVLinkVideoManager will be used
to send and receive video streaming related messages.

@DonLakeFlyer DonLakeFlyer requested a review from dogmaphobic Jun 11, 2017

/**
* This methos detects the first heartbeat from csd and requests camera information with command MAV_CMD_REQUEST_CAMERA_INFORMATION
* @param link : The interface to read from and send to
* @param message : The recieved mavlink message

This comment has been minimized.

Copy link
@staroselskii

staroselskii Jun 12, 2017

A small typo
s/recieved/received

This comment has been minimized.

Copy link
@otaviobp

otaviobp Jun 14, 2017

Author Contributor

fixed. thanks

@DonLakeFlyer

This comment has been minimized.

Copy link
Contributor

DonLakeFlyer commented Jun 12, 2017

This all seems like very special use. Are there any plans for either PX4 Pro or ArduPilot firmware to support this. Without that I tend to not like this sort of thing. The reason is that it bit rots almost immediately without anyone using it.

@lfelipe

This comment has been minimized.

Copy link

lfelipe commented Jun 13, 2017

@DonLakeFlyer as Otavio mentioned, there is an implementation for this as part of Camera Streaming Daemon (which can be run together with both PX4 and Ardupilot). The idea with this proposal is to get more testing and use this as a testbed for the proposals and implementation of messages that are being discussed as part of the ongoing camera efforts within MAVLink and Dronecode projects. Implementing support for these messages inside the flight stack itself might make sense in some cases, but particularly for ours we are more interested in having this on the side of the companion computer (we're testing on Intel Aero, but there is nothing platform specific on the project and the idea is that it can be used by anyone interested in this).

@DonLakeFlyer

This comment has been minimized.

Copy link
Contributor

DonLakeFlyer commented Jun 14, 2017

Ah, I see thanks for the explanation.

@dogmaphobic Can you review?

SaliniVenate and others added some commits May 19, 2017

send MAV_CMD_REQUEST_CAMERA_INFORMATION once a heartbeat is detected …
…from camera streaming daemon and then recieve MAVLINK_MSG_ID_CAMERA_INFORMATION
video streaming: Use MAVLink Auto Discovery for video streams
Use MaAVLink messages to discover video streams and get its settings.
If "MAVLink Auto Discovery Streams" is selected as the Video Source,
MAVLink message will be used to get the list of available cameras and
video stream URI.
A new Widget was added letting the user to change the video stream and,
in the future, to configure its settings.

Based on previous branch from Ricardo de Almeida Gonzaga <ricardo.gonzaga@intel.com>
video streaming: Try to stream video in a prefered resolution
Add to the video streaming widget a list of common video resolutions.
When user select an specific resolution, a tentative of setting that
resolution is performed. Server should select the video stream that is
closer to the desired resolution.
video streaming: Add a refresh button to Video Streaming Widget
Add a button to da a new request for cameras to the vehicle camera
controller.

@otaviobp otaviobp force-pushed the otaviobp:csd_mavlink_support branch from 0edb3b5 to be82e12 Jun 14, 2017

@AlexeyBulatov

This comment has been minimized.

Copy link
Contributor

AlexeyBulatov commented Jun 15, 2017

Why does the camera have it's own sys_id different from vehicle_id? I think camera should be a part of vehicle system and has only another comp_id.
Also widgets aren't supported on ios and android.

@lbegani

This comment has been minimized.

Copy link
Contributor

lbegani commented Jun 16, 2017

@AlexeyBulatov

Why does the camera have it's own sys_id different from vehicle_id? I think camera should be a part of vehicle system and has only another comp_id

Video streaming from multiple cameras on-board is managed by a system that is not part of the flight stack. This new system is camera-streaming-daemon which is a process running on the companion board. The camera-streaming-daemon registers its presence by sending mavlink heartbeat messages to the GCS and later exchanges mavlink messages directly with the GCS without having the mavlink messages go through the Flight Stack. Different system = different sys_id.
However, in the implementation where Flight Stack will be managing the video streaming and/or the mavlink messages related to video streaming, sys_id used will that be of vehicle_id.

@lbegani

This comment has been minimized.

Copy link
Contributor

lbegani commented Jun 21, 2017

@dogmaphobic @julianoes Can you please review?

@dogmaphobic

This comment has been minimized.

Copy link
Contributor

dogmaphobic commented Jun 21, 2017

@otaviobp I've been out of town and only now I'm looking at this. I've also pushed a PR that changes slightly how to handle video streaming (look at #5318). Let me clone this PR (your repo) so I can build and take a better look at it. If you don't mind, I will merge/fix the conflicts created since you pushed it.

Yes, the idea all along was to have a camera control widget. I have not yet added one as I've been waiting on a resolution on how to discover the camera parameters (the ongoing discussion in http://discuss.px4.io/t/mavlink-camera-api-discussion/3278/21)

Also, yes, the idea is to have a basic, MAVLink only interface for discovery and settings (such as you've done here). The notion of a camera definition file (the Json file mentioned above) are for extending the functionality that would be beyond the scope and capabilities of using MAVLink alone.

This is all in flux as things are still being sorted out. With that said, at first glance this PR is a move in the right direction. Note that my focus on the discussion above was in controlling an onboard camera for recording videos and capturing images. Video streaming was a secondary function. The primary method for discovery and setting of video streaming will probably still be through MAVLink only as you've done here.

@dogmaphobic

This comment has been minimized.

Copy link
Contributor

dogmaphobic commented Jun 22, 2017

Ok, now that I see how the widget was created, it won't work. You are using the old QtWidget type widget, which is being phased out. Those won't even build for mobile platforms.

This will be quite a bit more involved. What needs to be done is for a widget to be written in QML and loaded (from a Loader) within FlightDisplayVideoVideo.qml. A controller (cpp) would need to be built to help drive it. The widget would only be loaded if the camera vehicle reports a camera is available.

Given the amount of work involved, let's wait for 3.2 to be done and out. In the mean time I will create the widget and controller (using the code you have here for video streaming discovery). We can then go from there and move along with the rest of the Video/Photo controls discussed above.

In summary, leave this here for now. I will get going with the base code and use the MAVLink interface you've added here. Once that's done, we can close this PR and start working on the new code.

In addition, we will also need to deal with the Flight Widget, which takes an enormous amount of screen space. We will have to have both Flight Widget and Camera Control widget fit within the UI without covering too much space, even if the Camera/Video control will only be shown when the video view is shown.

The widget itself should be just a camera/video icon to switch the camera between video and photo mode and a shutter button to Start/Stop recording videos or capture a photo. A Settings button would also be present. When this Settings button is pressed, a Video/Photo/Streaming Video settings dialog will be shown in the middle of the screen (with the video in the background). In order for this to be properly shown, we need to know if the vehicle has video recording, photo capture, and/or video streaming capabilities. That will determine how the controller is built. In addition, when the Video/Photo/Streaming settings is shown, we need to know what parameters are there and what options each parameter offers. That's the whole point of the ongoing discussion.

Ha! And lastly, we also need to figure out how to integrate the local video recording. That's the recording of the video stream to local storage as opposed to recording video on the camera itself (at a likely much higher resolution and quality).

@lbegani

This comment has been minimized.

Copy link
Contributor

lbegani commented Jun 27, 2017

@dogmaphobic
Can we get this merged now with some basic UI. More comprehensive UI can be added later. With this change, we will get the use case functional. Instead of widgets, we can append the list of existing video sources with the new video streams discovered. On their selection, video streaming can be viewed in the Fly View.

@dogmaphobic

This comment has been minimized.

Copy link
Contributor

dogmaphobic commented Jun 27, 2017

I wasn't planning on doing this until after 3.2 goes out. @DonLakeFlyer?

@DonLakeFlyer

This comment has been minimized.

Copy link
Contributor

DonLakeFlyer commented Jun 27, 2017

I wasn't planning on doing this until after 3.2 goes out.

Correct. This isn't going into 3.2

@DonLakeFlyer

This comment has been minimized.

Copy link
Contributor

DonLakeFlyer commented Feb 16, 2018

Can I close this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.