qgroundcontrol
qgroundcontrol copied to clipboard
Change UDP video streams to use an address:port
This allows for the following specific benefits:
- Specifying multicast addresses for video streams
- Restricting devices/addresses that are used to receive video streams
Provision has been made to allow for just a port number to be entered to keep previous functionality. However, the settings fact has been renamed so perhaps something else must be done to migrate existing settings. Alternative would be to leave the settings fact listed as udpPort but keep the new functionality.
Anything that affects UI needs screen shots
Nevermind. Simple enough to see minimal UI changed.
However, the settings fact has been renamed so perhaps something else must be done to migrate existing settings.
Yes, you need to add code to VideoSettings to migrate any old value to the new one and remove the old value.
Provision has been made to allow for just a port number to be entered to keep previous functionality.
I wonder about this part as well. Does RTSP or TCP url setting allow this? If not these should match how those work. Either port only is allowed everywhere or full url is required everywhere. I think consistency is more important here.
Might also be nice in all of these URL fields to add ```placeholderText`` which shows the required url format.
Thanks for the feedback zdanek and DonLakeFlyer. I haven't had a chance to circle back on this and clean it up, but I aim to do so at some point in the next few weeks.
The GStreamer does not need any adjustments, at least not for recent releases of GStreamer. I have not confirmed specifically with the recommended release, but I can confirm that this patch does enable multicast across a fairly wide range of releases (tested on arm64 and amd64 Linux builds, and a Windows build). If there's additional work to be done ensuring that the GStreamer side is also compatible, then perhaps that's another issue/PR for the "back-end" work.
Regarding the needed changes for address/port layout, and the notes on how to actually pick the right bind address, I do agree with the points raised. I think I'll make an adjustment to instead add a new parameter for the bind address. This should make it easier to parse when actually using the setting, as well as being a bit easier to describe in the tooltip.
I have not done it in the original patch, but it would be smart here to make sure the new code works for IPv6 as well, I'll throw that into the mix too. Not sure if there is any platform compatibility concerns for IPv6, but it seems unlikely. Regardless, I will check if there's a sane way to do this generally for the targeted platforms, any perhaps the default value for the new bind address parameter can just be blank? That way we could detect and use the right address space for a given platform. This would also mean that people that have no interest in selecting the correct bind port can just clear the parameter and not have to think about it.