onvif: Major overhaul of ONVIF support (discovery, reconnect, focus, imaging, presets fix) - #312
onvif: Major overhaul of ONVIF support (discovery, reconnect, focus, imaging, presets fix)#312jonata wants to merge 2 commits into
Conversation
|
Amazing! Thanks for working on this. Yes, please split the change into logical, bisectable commits. That will make the changes a lot easier to review. I'm also in the midst of some major rework to the device management, and it will be easy to reconcile with that work if the changes are in smaller pieces. |
326d124 to
56a05e4
Compare
|
Done — force-pushed the branch as 15 focused, bisectable commits, each with a Signed-off-by:
Each commit builds on its own; the final tree is identical to what was in the previous squashed version. Happy to reorder, split further, or drop any of these to make merging easier alongside your device-management rework. |
glikely
left a comment
glikely
left a comment
There was a problem hiding this comment.
Great series, thank you! I've merged the first three commits as they were flawless. I've got a nitpick on where formatting changes to keep clang-format happy are applied, and there is a change to add the override keyword that is a coding fix that should be committed separately from the feature. Fix those things up and I'll merge the rest of the commits minus the last one.
I really appreciate the detailed commit messages. It made it a lot easier to review and understand the code.
The last commit I want to think about a bit more. I'm experimenting with a major overhaul on how PTZ instances are created by making them source filters, which would conflict with your change to auto-create the ONVIF source. You can leave that feature in the series, and I'll make a decision later on whether I merge it.
56a05e4 to
f2edecc
Compare
|
Pushed the requested changes:
Branch is rebased on top of the new |
|
About the last commit, sure. By now it's just adding a media source with the RTSP url, based on the ONVIF discovery. |
f2edecc to
b683c28
Compare
Have you published the simulator code? I'd like to make use of it. |
|
It's a silly simulator, that would print PTZ values in a colorbar on rstp. |
|
That's pretty useful. Do you want to add it to the repo? Perhaps in /scripts. I've got a really poor VISCA simulator in there too. They can be friends |
|
Thanks for adding the script. I've pulled it into main. I've left the last two patches adding the discovery features from mainline for the moment. I'm experimenting with making PTZDevices be source filters instead of being managed independently, which would mess with the discover code for ONVIF. If the filters approach works the way I hope it will, then I'll get you to look at the tree and see how the ONVIF discovery can be adapted for the new model. Give me a week or two to mess with it some more. In the mean time, please take a look at the proc_handler api pull request. I would appreciate your review and testing to make sure I haven't broken anything. I'm attending a conference at the moment and away from my test hardware. |
There was a problem hiding this comment.
Pull request overview
This PR significantly expands the experimental ONVIF support by adding WS-Discovery-based camera discovery + a selection dialog, plus wiring the new discovery flow into the “Add PTZ device” UI and build system.
Changes:
- Add a new ONVIF discovery / media-probe implementation (
OnvifDiscovery,OnvifMediaProbe) and a Qt selection dialog (OnvifDiscoveryDialog). - Update the settings “Add ONVIF” flow to use the discovery dialog and optionally auto-create an OBS Media Source from a probed RTSP URI.
- Add supporting build / localization / tooling updates (CMake sources, locale strings, scripts unignored, ONVIF emulator script).
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/settings.cpp | Routes “Add ONVIF” through the discovery dialog and optionally creates an OBS media source from the selected RTSP URI. |
| src/onvif-discovery.hpp | Declares the WS-Discovery probe, media probe, and selection dialog APIs. |
| src/onvif-discovery.cpp | Implements WS-Discovery probing, async SOAP media probing, and the camera selection dialog UI. |
| scripts/onvifemu.py | Adds a minimal ONVIF emulator for manual end-to-end testing. |
| data/locale/en-GB.ini | Adds new UI strings for the ONVIF discovery dialog and related settings. |
| CMakeLists.txt | Adds the new ONVIF discovery sources under ENABLE_ONVIF. |
| .gitignore | Un-ignores /scripts so the new emulator/tooling script is tracked. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| QUrl s(scope); | ||
| QString path = s.path(); | ||
| QString value = QUrl::fromPercentEncoding(s.path().toUtf8()); |
| lines << QString("Host: %1").arg(m_selected.host); | ||
| lines << QString("Port: %1").arg(m_selected.port); | ||
| if (!m_selected.manufacturer.isEmpty()) | ||
| lines << QString("Manufacturer: %1").arg(m_selected.manufacturer); | ||
| if (!m_selected.model.isEmpty()) | ||
| lines << QString("Model: %1").arg(m_selected.model); | ||
| if (!m_selected.location.isEmpty()) |
|
I've made progress on the plugin refactoring. Your remaining patches still applies and builds cleanly, but you'll probably want to check to make sure I haven't broken anything. Please take a look at the main branch. I've also allowed Copilot to do a review. It's AI, so take the results with a grain of salt, but there are some valid comments in there. I'm still looking at making PTZ devices filters, so that PTZ devices are always associated with a source, instead of something that is loosly associated afterwards. That change will impact both of your remaining patches. The autodiscovery patch will need to be reworked because there won't necessarily be an 'add' button in the settings dialog anymore. However, what you could do is add an autodetect button or a discovered cameras combo box to the ONVIF get_properties() function. That would give you the hook for opening the dialog to choose a discovered camera. That is a change that could be made now. However, I'm happy if you want to make that change in a follow up patch. The auto-create OBS Media Source patch is more difficult. When PTZ devices are setup as filters then the OBS Source needs to be in place first, so you'll need to trigger the source creation somewhere else. It could be a button in the settings dialog I suppose. Needs some thought. Regardless, please rebase onto latest mainline, make sure all is still working, and refresh this PR |
Adds a first-class entry path that replaces manual host/port typing. Selecting "ONVIF (experimental)" from the Add Device menu now opens an OnvifDiscoveryDialog that: - Sends a SOAP-over-UDP Probe to the WS-Discovery multicast group (239.255.255.250:3702) from every up + multicast-capable IPv4 interface, so multi-homed hosts find cameras on every LAN, not just the default-route one. - Lists discovered cameras in a table with host/port/manufacturer/model parsed from the response's Scopes section. - Picks the responder's source address when a camera's XAddrs all point at hosts we can't reach — a common Xiongmai/budget-cam quirk where the device advertises its DHCP-assigned IP even when actually reached via NAT or a different subnet. This mirrors what ODM does. - Has a "Don't see your camera? Add manually" row so cameras the multicast probe can't reach (a different routed network, a multicast-blocked switch, etc.) can be added by hand. - Fetches per-profile RTSP stream URIs via an OnvifMediaProbe helper (GetSystemDateAndTime for clock sync, GetCapabilities, GetProfiles, GetStreamUri-per-profile), surfacing SOAP faults so credential or configuration failures produce a readable error instead of "Failed to parse SOAP response". Empty password ⇒ probe without a WS-Security header at all, matching the way ODM treats anonymous cameras. - Sanitizes unescaped `&` in SOAP responses the same way PTZOnvif does, so Xiongmai-style GetStreamUri URLs with `&channel=` / `&protocol=` don't blow up Qt's QDomDocument parser. - Has a 10s transferTimeout on every probe request, so the dialog doesn't sit on the OS TCP timeout when a camera goes offline. - Saves the offending response body to /tmp/onvif-probe-bad-response.xml when a SOAP parse fails, so the failure is debuggable from a single user report. On accept, settings.cpp creates the PTZ device pre-filled with the discovered host / port / credentials. Auto-creating an OBS Media Source from the picked stream URI is intentionally deferred to a follow-up patch — this commit is just the discovery + selection flow. Signed-off-by: Jonatã Bolzan Loss <jonata@jonata.org>
Builds on the previous WS-Discovery dialog patch: once the user picks a camera, also offer to drop an FFmpeg-based Media Source into the current scene pointing at the camera's RTSP stream. The dialog grows a "Also create an OBS Media Source for this camera's stream" checkbox (default checked) and a stream picker that lists every RTSP URI returned by GetStreamUri so the user can choose Main vs Sub before clicking "Use Selected Camera". On accept, settings.cpp: - Takes the URI from the picker (falls back to the first usable URI), - Embeds the dialog's username/password as Basic auth into the URL so the source still works after the per-session token in the camera's reply expires, - Creates an "ffmpeg_source" with restart_on_activate and hw_decode, - Adds it to the currently active scene. If the user unchecks the box, only the PTZ device is created. If GetStreamUri came back empty (anonymous probe was rejected, or the camera has no streamable profile), the Media Source step silently no-ops. Signed-off-by: Jonatã Bolzan Loss <jonata@jonata.org>
0c750c0 to
2c49994
Compare
|
Refreshed:
|
Summary
This PR makes the experimental ONVIF driver actually usable end-to-end against real-world cameras (notably budget Xiongmai/XM-chipset firmwares that are loose with the spec), and adds WS-Discovery as the primary entry path so users no longer have to type host/port by hand.
It's a single commit on purpose for ease of pulling, but I'm happy to split into smaller patches if that's preferred — the changes group cleanly along the section boundaries below.
Bug fixes in the existing driver
absoluteMove()/relativeMove(). Both tookintpan/tilt/zoom, silently casting the normalized doubles to 0 or 1. Every absolute/relative move was effectively (0, 0, 0).sendRequest()unconditionally set anAuthorization: Basic ...header even though the WS-UsernameToken inside the SOAP envelope already authenticates. Some firmwares rejected the duplicate as ambiguous.8899→80— the more common ONVIF default.sendRequest()no longer pushes a request with an empty URL (which can happen when an operation fires beforeGetCapabilitieshas populated the per-service XAddrs). Avoids spammingProtocol "" is unknownonce per click.&in SOAP body text sanitized before parsing. Many cameras emit&protocol=/&channel=in stream URIs. Python'sElementTreetolerates it; Qt'sQDomDocumentrejects the whole document. We replace any¬ already starting a known entity with&.SetPresetResponsecarries the camera-assignedPresetToken, but the old code threw it away — somemory_recall()later had no token and silently did nothing. Now we remember which slot triggered theSetPresetand store the returned token there.memory_reset()also clears the stale local token afterRemovePreset.Reliability improvements
connectCamera()now begins withGetSystemDateAndTime(an unauthenticated call per spec), computes the offset between camera and host clocks, and adjusts every WS-SecurityCreatedtimestamp by it. Cameras with bad NTP no longer reject our calls as out-of-window timestamps. If the time probe errors out,ensureCapabilitiesRequested()makes sureGetCapabilitiesstill runs so the device doesn't stay un-initialized.hsoapis the prolific offender) advertise their static/DHCP IP in WS-Discovery XAddrs and inGetCapabilitiesresponses even when reached via a different address (NAT, multi-homed, IP changed since boot). We now override the host of any XAddr that doesn't match the responder/configured host — both in the WS-Discovery parse and inhandleGetCapabilitiesResponse. This is what ODM and most other ONVIF clients do.PTZOnvifnow callssetConnected()on every success/failure, so the dock's red/green indicator reflects reality. After 3 consecutive failures (~15s), the device is marked disconnected and the next status-timer tick restarts the full connect chain fromGetSystemDateAndTime— so a camera reboot or a network blip recovers without user intervention.GetStatuspolling keeps the cached pan/tilt/zoom position in sync. The same timer drives the reconnect retry when disconnected.New features
WS-Discovery + selection dialog
Adding an ONVIF device now pops a dialog that:
Probeto239.255.255.250:3702from every suitable interface (multi-homed hosts find cameras on every LAN, not just the default-route one).Scopes.GetCapabilities→GetProfiles→GetStreamUri, with the same clock-skew handling as the runtime driver and a clear SOAP-fault surface so auth/credential failures produce readable errors instead of "Failed to parse SOAP response".Per-device settings
ProfileTokenPTZ commands target. Persists across reloads.Imaging service
AutoFocusModetoggle (MANUAL/AUTO), wired throughPTZDevice::focus_changed. Only fires on cameras that advertise an Imaging XAddr.AUTO/MANUAL).PTZ service
SetHomePositionvia right-click on the dock's Home button. Gated by a new virtualsupportsSetHome()(defaultfalse), so VISCA/Pelco/USB devices show no menu and their behavior is unchanged — only ONVIF overrides it totrue.Files
src/onvif-discovery.{cpp,hpp}OnvifDiscovery(WS-Discovery probe),OnvifMediaProbe(stream-URI fetcher),OnvifDiscoveryDialog(UI)src/ptz-onvif.{cpp,hpp}src/settings.cppsrc/ptz-device.hpppantilt_set_home()andsupportsSetHome()(default no-op/false)src/ptz-controls.{cpp,hpp}data/locale/en-GB.iniCMakeLists.txtENABLE_ONVIFTest plan
XM530_RF50X30_8M,hsoap/2.8, ONVIF 17.12) camera: WS-Discovery finds it, manual entry works for cross-subnet, RTSP URIs come back, Media Source auto-creates and plays, pan/tilt/zoom respond, presets save and recall, status dot turns green, auto-reconnect recovers after the camera reboots.Set/GotoHomePosition.SetHomePosition: not all firmwares implement it (the spec lets cameras silently accept-but-not-store). When the camera supports it, the saved position is recalled by a subsequent single-click on Home.supportsSetHome()default keeps the new menu hidden for those protocols.