feat(VideoManager): add HTTP MJPEG and WebSocket JPEG sources - #13594
feat(VideoManager): add HTTP MJPEG and WebSocket JPEG sources#13594alireza787b wants to merge 9 commits into
Conversation
|
Pretty cool, although I think you're missing some more places where the WebSockets lib needs to be added. One example being the Vagrantfile |
Thanks for catching that! You're absolutely right - I've now added qtwebsockets to all the additional Qt installation locations:
This ensures consistency across all development environments (CI workflows, Vagrant, and manual setups). The changes have been pushed - |
|
How can this be tested without needing to buy some sort of camera that supports this. With other gstreamer based feed we can simulate streams using gstreamer to validate things work. |
@DonLakeFlyer, I've included synthetic test servers that follow the ADSB simulator pattern and don't require a camera or video files to run. How to Test Without a Camera
The test servers will generate synthetic test patterns for validation. The project's README also contains GStreamer CLI alternatives if you prefer command-line tools for simulation. Real-World TestingFor more comprehensive, real-world testing, you can use PixEagle, which works well with webcams, video files, or the simulator sources mentioned above: |
|
@alireza787b Interesting feature, we'll get back to it when one of us has more time to test it out |
c4b0c05 to
295a1c1
Compare
|
See the Build Results workflow run for details. |
There was a problem hiding this comment.
Pull request overview
Adds new network-based video sources to QGroundControl’s video pipeline (HTTP/HTTPS MJPEG and WebSocket-fed JPEG via GStreamer appsrc), along with new Video settings and test servers to validate the streams.
Changes:
- Introduces HTTP MJPEG (
souphttpsrc → multipartdemux → jpegparse) and WebSocket (appsrc → jpegdec) source creation in the GStreamer receiver. - Adds new Video settings/Facts and updates the Video settings UI to configure HTTP/WebSocket URLs and some connection parameters.
- Adds Python HTTP MJPEG and WebSocket test servers plus a small README/requirements set.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| test/VideoStreaming/http_mjpeg_server.py | Adds a local Flask-based MJPEG test server for manual validation. |
| test/VideoStreaming/websocket_video_server.py | Adds a local WebSocket JPEG-frame test server for manual validation. |
| test/VideoStreaming/requirements.txt | Adds Python dependencies for the test servers. |
| test/VideoStreaming/README.md | Documents how to run the test servers and expected protocol. |
| src/VideoManager/VideoReceiver/GStreamer/QGCWebSocketVideoSource.h | Declares a Qt WebSocket-to-GStreamer appsrc bridge. |
| src/VideoManager/VideoReceiver/GStreamer/QGCWebSocketVideoSource.cc | Implements WebSocket connection/reconnect/heartbeat and pushing frames to appsrc. |
| src/VideoManager/VideoReceiver/GStreamer/GstVideoReceiver.h | Adds HTTP/WebSocket source helpers and stream settings structs. |
| src/VideoManager/VideoReceiver/GStreamer/GstVideoReceiver.cc | Routes http(s)/ws(s) URIs to new source builders and captures settings. |
| src/VideoManager/VideoReceiver/GStreamer/CMakeLists.txt | Adds optional gstapp + Qt6 WebSockets build wiring for WebSocket support. |
| src/VideoManager/VideoManager.cc | Wires new video sources into URI selection and timeout behavior. |
| src/UI/AppSettings/VideoSettings.qml | Adds URL inputs and basic HTTP/WebSocket settings groups to the UI. |
| src/Settings/VideoSettings.h | Adds new setting Facts and new source string constants. |
| src/Settings/VideoSettings.cc | Registers new sources and setting Facts; adds streamConfigured logic for new URLs. |
| src/Settings/Video.SettingsGroup.json | Adds new HTTP/WebSocket settings metadata entries. |
| signals: | ||
| void connected(); | ||
| void disconnected(); | ||
| void errorOccurred(const QString &error); | ||
|
|
||
| private slots: | ||
| void _onConnected(); | ||
| void _onDisconnected(); | ||
| void _onBinaryMessageReceived(const QByteArray &message); | ||
| void _onTextMessageReceived(const QString &message); | ||
| void _onError(); | ||
| void _onSslErrors(const QList<QSslError> &errors); | ||
| void _sendHeartbeat(); |
There was a problem hiding this comment.
QGCWebSocketVideoSource.h uses QSslError/QString/QByteArray/QList in signals/slots but doesn’t include the required headers (e.g., <QtNetwork/QSslError>, <QtCore/QString>, <QtCore/QByteArray>, <QtCore/QList>). This can break compilation depending on include order; add the proper includes (or a complete-type include for QSslError).
There was a problem hiding this comment.
Fixed. Added the required includes (QSslError, QString, QByteArray, QList) and sorted them alphabetically per QGC convention.
| GST_BUFFER_PTS(buffer) = gst_util_uint64_scale(_framesReceived, GST_SECOND, 30); | ||
| GST_BUFFER_DTS(buffer) = GST_BUFFER_PTS(buffer); | ||
| GST_BUFFER_DURATION(buffer) = gst_util_uint64_scale(1, GST_SECOND, 30); |
There was a problem hiding this comment.
Appsrc is configured with do-timestamp=TRUE, but _pushFrameToAppsrc sets PTS/DTS/duration using a hard-coded 30 FPS. This will produce incorrect timestamps whenever the server FPS differs and can conflict with appsrc’s timestamping. Prefer leaving timestamps unset (GST_CLOCK_TIME_NONE) and letting appsrc timestamp, or compute timestamps from actual arrival time / negotiated FPS.
| GST_BUFFER_PTS(buffer) = gst_util_uint64_scale(_framesReceived, GST_SECOND, 30); | |
| GST_BUFFER_DTS(buffer) = GST_BUFFER_PTS(buffer); | |
| GST_BUFFER_DURATION(buffer) = gst_util_uint64_scale(1, GST_SECOND, 30); | |
| // Let appsrc timestamp the buffer (do-timestamp=TRUE) instead of forcing a hard-coded 30 FPS. | |
| GST_BUFFER_PTS(buffer) = GST_CLOCK_TIME_NONE; | |
| GST_BUFFER_DTS(buffer) = GST_CLOCK_TIME_NONE; | |
| GST_BUFFER_DURATION(buffer) = GST_CLOCK_TIME_NONE; |
There was a problem hiding this comment.
Agreed. Switched to GST_CLOCK_TIME_NONE for PTS/DTS/DURATION and relying on do-timestamp=TRUE to timestamp from the pipeline clock. This is the correct pattern for a live source with variable frame rate.
| void QGCWebSocketVideoSource::_onBinaryMessageReceived(const QByteArray &message) | ||
| { | ||
| if (message.isEmpty()) { | ||
| return; | ||
| } | ||
|
|
||
| _pushFrameToAppsrc(message); | ||
| _expectingBinaryFrame = false; | ||
| } | ||
|
|
||
| void QGCWebSocketVideoSource::_onTextMessageReceived(const QString &message) | ||
| { | ||
| // Text messages are JSON metadata from PixEagle-compatible servers | ||
| // Format: {"type":"frame","size":N,"quality":Q} | ||
| if (message.contains(QStringLiteral("frame"))) { | ||
| _expectingBinaryFrame = true; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
_onTextMessageReceived sets _expectingBinaryFrame but _onBinaryMessageReceived ignores it and always pushes any binary message into jpegdec. Since protocol metadata (including declared size) is effectively ignored (_expectedFrameSize is never used), non-frame binary messages or size mismatches can be fed to the decoder. Consider enforcing the expected state/size before pushing to appsrc.
There was a problem hiding this comment.
Removed the dead protocol state (_expectingBinaryFrame, _expectedFrameSize). Replaced with a lightweight JPEG SOI marker check (0xFFD8) on incoming binary messages — protocol-agnostic and prevents non-JPEG data from reaching jpegdec.
| find_package(PkgConfig QUIET) | ||
| if(PKG_CONFIG_FOUND) | ||
| pkg_check_modules(GSTREAMER_APP QUIET IMPORTED_TARGET gstreamer-app-1.0) | ||
| endif() | ||
| if(TARGET PkgConfig::GSTREAMER_APP) | ||
| set_target_properties(PkgConfig::GSTREAMER_APP PROPERTIES IMPORTED_GLOBAL TRUE) | ||
| target_link_libraries(${CMAKE_PROJECT_NAME} PRIVATE PkgConfig::GSTREAMER_APP) | ||
| target_compile_definitions(${CMAKE_PROJECT_NAME} PRIVATE QGC_GST_APP_AVAILABLE) | ||
|
|
||
| target_sources(${CMAKE_PROJECT_NAME} | ||
| PRIVATE | ||
| QGCWebSocketVideoSource.cc | ||
| QGCWebSocketVideoSource.h | ||
| ) | ||
|
|
||
| find_package(Qt6 ${QGC_QT_MINIMUM_VERSION}...${QGC_QT_MAXIMUM_VERSION} | ||
| COMPONENTS WebSockets | ||
| ) | ||
| if(TARGET Qt6::WebSockets) | ||
| target_link_libraries(${CMAKE_PROJECT_NAME} PRIVATE Qt6::WebSockets) | ||
| else() | ||
| message(WARNING "Qt6::WebSockets not found - WebSocket video streaming will not be available") | ||
| endif() | ||
| endif() |
There was a problem hiding this comment.
CMake adds QGCWebSocketVideoSource.* and defines QGC_GST_APP_AVAILABLE based only on gstreamer-app being found, but the source unconditionally includes QtWebSockets headers. If gstapp is present but Qt6::WebSockets is not, this will fail to compile/link. Gate adding these sources/defines on BOTH dependencies (gstapp + Qt6::WebSockets), or otherwise disable WebSocket support cleanly.
There was a problem hiding this comment.
Fixed. WebSocket sources and QGC_GST_APP_AVAILABLE are now only added when both gstreamer-app-1.0 and Qt6::WebSockets are found. If either is missing, a status message explains which dependency is unavailable.
| SettingsGroupLayout { | ||
| Layout.fillWidth: true | ||
| heading: qsTr("HTTP Stream Settings") | ||
| visible: _isHTTP | ||
|
|
||
| LabelledFactTextField { | ||
| Layout.fillWidth: true | ||
| label: qsTr("Connection Timeout") | ||
| fact: _videoSettings.httpTimeout | ||
| } | ||
|
|
||
| LabelledFactTextField { | ||
| Layout.fillWidth: true | ||
| label: qsTr("Retry Attempts") | ||
| fact: _videoSettings.httpRetryAttempts | ||
| } | ||
|
|
||
| FactCheckBoxSlider { | ||
| Layout.fillWidth: true | ||
| text: qsTr("Keep-Alive") | ||
| fact: _videoSettings.httpKeepAlive | ||
| } | ||
| } | ||
|
|
||
| SettingsGroupLayout { | ||
| Layout.fillWidth: true | ||
| heading: qsTr("WebSocket Stream Settings") | ||
| visible: _isWebSocket | ||
|
|
||
| LabelledFactTextField { | ||
| Layout.fillWidth: true | ||
| label: qsTr("Connection Timeout") | ||
| fact: _videoSettings.websocketTimeout | ||
| } | ||
|
|
||
| LabelledFactTextField { | ||
| Layout.fillWidth: true | ||
| label: qsTr("Reconnect Delay") | ||
| fact: _videoSettings.websocketReconnectDelay | ||
| } | ||
|
|
||
| LabelledFactTextField { | ||
| Layout.fillWidth: true | ||
| label: qsTr("Heartbeat Interval") | ||
| fact: _videoSettings.websocketHeartbeat | ||
| } | ||
| } |
There was a problem hiding this comment.
VideoSettings.qml exposes only a subset of the newly-added HTTP/WebSocket tuning Facts (e.g., httpBufferSize/httpUserAgent/adaptiveQuality/minQuality/maxQuality/websocketBufferFrames are defined in settings but have no UI controls here). Either add the missing controls or remove/keep them internal to avoid confusing “hidden” settings.
There was a problem hiding this comment.
Removed the unused settings (httpBufferSize, httpUserAgent, adaptiveQuality, minQuality, maxQuality, websocketBufferFrames). PixEagle's adaptive quality runs server-side (bandwidth EWMA + encoding time + CPU monitoring via AdaptiveQualityEngine), so no client-side quality controls are needed. Buffer size and user-agent remain as hardcoded struct defaults in the GStreamer receiver. Will re-add as user-facing settings if/when client-initiated quality negotiation is implemented.
| async def video_handler(websocket): | ||
| """Handle a single WebSocket video client.""" | ||
| print(f"Client connected: {websocket.remote_address}") | ||
| frame_number = 0 | ||
| fps = 30 | ||
| quality = 85 | ||
| frame_interval = 1.0 / fps | ||
|
|
||
| try: | ||
| while True: | ||
| start = time.monotonic() | ||
|
|
||
| frame = generate_test_frame(640, 480, frame_number, fps) | ||
| _, jpeg = cv2.imencode('.jpg', frame, [cv2.IMWRITE_JPEG_QUALITY, quality]) | ||
| jpeg_bytes = jpeg.tobytes() | ||
|
|
||
| # Send JSON metadata first | ||
| metadata = json.dumps({ | ||
| "type": "frame", | ||
| "size": len(jpeg_bytes), | ||
| "quality": quality, | ||
| "frame": frame_number, | ||
| "timestamp": time.time() | ||
| }) | ||
| await websocket.send(metadata) | ||
|
|
||
| # Send binary JPEG frame | ||
| await websocket.send(jpeg_bytes) | ||
|
|
||
| frame_number += 1 | ||
|
|
||
| elapsed = time.monotonic() - start | ||
| remaining = frame_interval - elapsed | ||
| if remaining > 0: | ||
| await asyncio.sleep(remaining) | ||
|
|
||
| except websockets.exceptions.ConnectionClosed: | ||
| print(f"Client disconnected: {websocket.remote_address}") | ||
|
|
||
|
|
||
| async def main(port): | ||
| print(f"Starting WebSocket video server on ws://0.0.0.0:{port}/ws/video_feed") | ||
| async with websockets.serve(video_handler, "0.0.0.0", port): | ||
| await asyncio.Future() | ||
|
|
||
|
|
||
| if __name__ == '__main__': | ||
| parser = argparse.ArgumentParser(description='WebSocket Video Test Server') | ||
| parser.add_argument('--port', type=int, default=5078, help='Server port (default: 5078)') | ||
| parser.add_argument('--fps', type=int, default=30, help='Frames per second (default: 30)') | ||
| args = parser.parse_args() | ||
|
|
||
| asyncio.run(main(args.port)) |
There was a problem hiding this comment.
--fps is accepted on the command line but ignored: video_handler hard-codes fps=30 and main() doesn’t pass args.fps through. This makes the test server behavior not match the documented usage. Thread the selected FPS into the handler (e.g., via a closure/global) and use it for frame_interval and frame generation.
There was a problem hiding this comment.
Fixed. The --fps argument is now threaded through to the handler via functools.partial.
| DEFINE_SETTINGFACT(adaptiveQuality) | ||
| DEFINE_SETTINGFACT(minQuality) | ||
| DEFINE_SETTINGFACT(maxQuality) | ||
| DEFINE_SETTINGFACT(websocketBufferFrames) |
There was a problem hiding this comment.
These WebSocket adaptive-quality related Facts are introduced but aren’t referenced anywhere outside settings (no implementation or UI). If adaptive quality isn’t implemented yet, consider removing these Facts until they’re wired up, or add the missing logic/UI so they have an effect.
| DEFINE_SETTINGFACT(adaptiveQuality) | |
| DEFINE_SETTINGFACT(minQuality) | |
| DEFINE_SETTINGFACT(maxQuality) | |
| DEFINE_SETTINGFACT(websocketBufferFrames) |
There was a problem hiding this comment.
Removed. Adaptive quality is handled entirely server-side by PixEagle's AdaptiveQualityEngine — no client-side settings needed.
| @@ -131,6 +139,8 @@ void GstVideoReceiver::start(uint32_t timeout) | |||
| "message-forward", TRUE, | |||
| nullptr); | |||
|
|
|||
| _captureStreamSettings(); | |||
|
|
|||
| _source = _makeSource(_uri); | |||
There was a problem hiding this comment.
_captureStreamSettings() reads SettingsManager/Fact values from the GStreamer worker thread (start() runs on the worker when _needDispatch() is true). Settings/Facts are QObjects owned by the main thread, so this is unsafe cross-thread access. Capture these settings on the caller (main) thread and pass plain values into the worker, or fetch them via a blocking queued invoke to the main thread.
There was a problem hiding this comment.
Fixed. _captureStreamSettings() now executes before _needDispatch() dispatches to the worker thread, ensuring Fact values are read on the main thread. The worker then uses the captured plain-value structs (_httpSettings, _wsSettings) safely.
| { | ||
| "name": "httpUrl", | ||
| "shortDesc": "HTTP Video URL", | ||
| "longDesc": "HTTP/HTTPS URL for MJPEG video stream. Format: http://host:port/path (e.g., http://192.168.1.100:5077/video_feed for PixEagle, or http://camera-ip/mjpeg for IP cameras).", | ||
| "type": "string", | ||
| "default": "" | ||
| }, |
There was a problem hiding this comment.
PR description says the default “Video Display Fit” is changed to “Fit Width”, but the metadata here still indicates enumValues 0=Fit Width and default is 1 (Fit Height). If the default is intended to be Fit Width, update the default accordingly (and ensure any related code/UI matches).
There was a problem hiding this comment.
The JSON default for videoFit was never changed in this PR — it remains 1 (Fit Height) as before. Corrected the PR description to remove the incorrect claim. No code change needed.
|
Addressing all 9 review comments plus additional improvements: Thread safety:
Build correctness:
A/V correctness:
Code quality:
YAGNI removals:
Note: |
|
@HTRamsey Not sure what you want to do with this... |
|
Compatibility status update from the PixEagle side: I audited this PR against PixEagle current media/security defaults. The local test paths still make sense: same-host QGC/PixEagle usage with http://127.0.0.1:5077/video_feed and ws://127.0.0.1:5077/ws/video_feed is the supported direct-development path. PixEagle now permits a native same-host WebSocket client with no browser Origin only when both the socket peer and Host authority are loopback. The remote PixEagle examples such as 192.168.x.x:5077 should not be treated as plug-and-play anymore. PixEagle media endpoints are local-first, reject query-string tokens, and require scoped media:read auth for non-loopback clients. This PR currently exposes URL/timeouts only, with no configurable Origin/Authorization/header support for HTTP souphttpsrc or QWebSocket, so direct remote PixEagle HTTP/WS needs follow-up design before it is safe to advertise. For field/ground-station PixEagle video today, the recommended path remains H.264/RTP/UDP output to QGC. Future QGC work should likely add reviewed optional Origin/auth header support plus credential redaction before claiming authenticated remote PixEagle HTTP/WS compatibility. I did not build or mutate the QGC branch in this audit; GitHub currently reports this PR as dirty/not rebaseable, so rebase/build cleanup is still needed separately. |
|
Follow-up clarification from the PixEagle side after the remote-host media/security review: This QGC feature should remain generic. Normal HTTP/HTTPS MJPEG and WebSocket sources such as IP cameras, lab MJPEG servers, and custom WebSocket video servers should continue to work as URL-only sources when those sources do not require authentication. PixEagle should be treated as one stricter source profile on top of that generic capability, not hard-coded into QGC core behavior. Same-host PixEagle development remains the loopback path: http://127.0.0.1:5077/video_feed and ws://127.0.0.1:5077/ws/video_feed. Remote PixEagle running on a companion computer should not be advertised as plug-and-play anonymous HTTP/WS; it needs future optional generic Authorization/Origin/TLS settings plus credential redaction before direct remote PixEagle HTTP/WS is claimed. Field QGC video from PixEagle still uses H.264/RTP/UDP today. No QGC branch changes are included in this comment; this is just the compatibility boundary I will use for the follow-up implementation work. |
Reconcile PR mavlink#13594 with current QGroundControl and add generic authenticated HTTP MJPEG/WebSocket JPEG support with strict TLS, credential redaction, bounded input, recording guards, and focused tests.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (24.30%) is below the target coverage (30.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## master #13594 +/- ##
==========================================
+ Coverage 25.47% 30.21% +4.74%
==========================================
Files 769 775 +6
Lines 65912 67674 +1762
Branches 30495 31552 +1057
==========================================
+ Hits 16788 20450 +3662
+ Misses 37285 33220 -4065
- Partials 11839 14004 +2165
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 375 files with indirect coverage changes Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
b98848b to
ab5213f
Compare
github-actions
Bot
added
the
github_actions
alireza787b
changed the title
|
Draft refresh is now available at The PR description has been replaced to match the implementation and test evidence. The receiver is generic:
The camera-free fixtures remain under All Windows build variants passed, and CI created, installed, and verified the AMD64 installer with the build SDK removed from Upstream PR CI is now running on the promoted head. I am keeping the PR in draft until manual Windows playback, reconnect, source-switch, and MKV/MOV recording evidence is attached. Architecture, security, and testability review is welcome in the meantime. |
|
Thank you for the earlier feedback and testing direction on this draft. I have replaced this broad proposal with smaller, independently reviewable
The current QGC build configuration already provisions Qt WebSockets through The transport PRs intentionally support unauthenticated endpoints only. The I am closing this draft as superseded so review can continue on the focused |
|
Superseded by the focused PR sequence linked above. |
Description
Adds two generic network JPEG video sources to QGroundControl:
The existing video settings surface now exposes source-specific URLs plus optional None, Basic, or Bearer authentication, an in-memory session credential, an owner-only credential file on supported Unix desktop systems, an exact Origin header, and a custom CA certificate. Credentials are accepted only over HTTPS or WSS. URL user-info and common secret query parameters are rejected, diagnostics are redacted, authenticated HTTP redirects are disabled, and WebSocket redirects are not followed.
The receiver validates JPEG framing and dimensions before decode, applies bounded frame and decoded-image limits, and keeps source failures on the existing QGC reconnect path. HTTP and WebSocket JPEG streams can be recorded as MKV or MOV. MP4 is rejected for these sources because the parsed JPEG stream is not compatible with that container. Source disconnect while recording waits for splitmuxsink fragment finalization before teardown.
The change also includes user documentation and camera-free synthetic HTTP/WebSocket test servers under
test/VideoStreaming/. It is generic and does not contain source-product-specific behavior. WebRTC and codec negotiation are out of scope.Type of Change
Testing
Exact pre-promotion candidate:
ab5213f4f69c5b07494f101226db384b48af1e4f.This PR remains draft pending manual Windows playback, reconnect, source-switch, and MKV/MOV recording evidence from the exact installer.
Platforms Tested
Windows build/install/launch is automated above; the network-video receiver workflow is still awaiting manual Windows validation. Other platform build checks are tracked in CI and are not presented as runtime tests.
Flight Stacks Tested
Not applicable; this change does not exercise vehicle control.
Screenshots
Not applicable. The controls use QGroundControl's existing settings components.
Checklist
The complete unit and integration suites passed in the exact-head Linux CI run linked above; no local full QGC build is claimed.
Related Issues
No linked issue. This update also addresses the maintainer request in this PR for a reproducible test path that does not require a camera.
By submitting this pull request, I confirm that my contribution is made under the terms of the project's dual license (Apache 2.0 and GPL v3).