UPnP Server module implementation
This MR follows discussions about implementation that can be found here: https://mailman.videolan.org/pipermail/vlc-devel/2021-March/142703.html
These changes implement a fully functional UPNP server exposing the medialibrary content on the network.
Version 3
The recent changes on the medialibrary introduced a way to whitelist media in a generic manner to all the network interface.
The UPnP server now only list media that have been explicitly marked as public
by the user. This mitigates a lot of the privacy issues talked earlier.
To be clear, this merge request is only the start of the usage of the public media sharing, I plan to port this medialibrary capability to at least the HTTP interface. The MR does not provide a way to mark media as public/private, this will be done later.
Additionally, I tried to lighten up the merge request:
- The transcoding output of the server will be submitted in a subsequent MR
- The Unit tests will also be for a subsequent merge request as I'm introducing a small C++ helper that I would like to make available across all the codebase
- The commits have been cleaned up, and re-arranged
- The submitted code has been reformatted and cleaned up and updated to C++17 capabilities.
Original Description
Extracted from the former commit message:
This is the first implementation of a vlc upnp server module.
The module behave as an interface, it works in pair with the
medialibrary API to expose most of its content.
Here is a list of the server main features:
- Very straightforward to deploy, you start vlc with `-I upnp` and it
simply exposes the medialibrary on the local network.
- The server automatically exposes downscaled versions of your videos
- While the DLNA spec is far from being fully supported, a lot of DLNA
clients are supported.
- DLNA's "time based seeking" during transcoding is supported.
- Special transcoding profiles depending on client's user-agent are
suported, for the moment only the PLAYSTATION3 has differents
profiles hardcoded in the source code but it should be extensible to
other specific clients eventually using config files.
The module is split into different parts, here's a quick overview:
- upnp_server.cpp: The core of the module, all the interactions with
libupnp are done here, this file brings all the module parts
together.
- cds/*: The "ContentDirectory Service", represents and implements the
server file hierarchy, in our case, it mostly reflect the
medialibrary content.
- sout.cpp: The access out upnp server submodule internals. Used in
media transcoding to make the connection between the vlc transcode
pipeline output and the upnp HTTP callbacks.
- xml_wrapper: Wrap the xmli library with modern c++ code to fit the
codebase better.
- test/*: Unit tests
The upnp_server module is based on Hamza Parnica's great proof of
concept.
Merge request reports
Activity
added MRStatus::Accepted label
MR Acceptance result
This MergeRequest has been Accepted! Congratulations.MR acceptance checks details:
-
MR should be considered mergeable by Gitlab -
Last pipeline should be successful -
MergeRequest should have at least one external review and/or vote -
All threads should be resolved, have votes and score > 0 -
MergeRequest should have no activity (threads/votes) for (72h/72h)
-
Depends on !295 (merged) and !291 (merged).
Also some comments on the
cds/
subpart which is not completely reviewed (because C++) would be highly appreciated !Edited by Alaric Senat- Resolved by Thomas Guillem
Opening a thread because I need to do more tests before merging it. (but I'm OK with the code review)
added MRStatus::InReview label and removed MRStatus::Accepted label
- Resolved by Alaric Senat
- Resolved by Thomas Guillem
- Resolved by Alaric Senat
- Resolved by Thomas Guillem
In my opinion, the workflow you're suggesting (upnp server startup dialog) is not really that friendly with non-interactive use case and cumbersome for interactive use case. Maybe a location in the web and Qt UI can be reserved to showing that the UPnP server is enabled and let the user disable it instead?
The UPnP client connection dialog looks like a great addition though! One future question might be how to manage an allow list manually (CLI, file update) and where it is documented though.
added 41 commits
-
c6ca54b4...2fc48e91 - 31 commits from branch
videolan:master
- ea2f400e - avcodec: fill audio frame channel field
- 4eaf15fb - avcodec: fix assignation indent
- 35603b36 - renderer_common: use vlc_object instead of vlc_stream
- b3c24f93 - stream_out: dlna: publicly expose protocol info function
- ac5cd26d - stream_out: dlna: constify the profile list
- ea02a3a3 - upnp-wrapper: remove pointless `#if` guards
- 0b3e9017 - upnp: prioritize `UpnpInit2()` over `UpnpInit()`
- 8604aedb - contrib: bump libupnp to 1.14.7
- 1d3a633b - upnp_server: add the upnp server module
- 57ee0343 - upnp_server: add upnp server to intf creation callback
Toggle commit list-
c6ca54b4...2fc48e91 - 31 commits from branch
added MRStatus::NotCompliant label and removed MRStatus::InReview label
added 74 commits
-
57ee0343...2497347f - 66 commits from branch
videolan:master
- 8f4ed2f6 - renderer_common: use vlc_object instead of vlc_stream
- 3413c11c - stream_out: dlna: publicly expose protocol info function
- 7e82830e - stream_out: dlna: constify the profile list
- 8edcc62d - upnp-wrapper: remove pointless `#if` guards
- 9069f769 - upnp: prioritize `UpnpInit2()` over `UpnpInit()`
- 178fa2a8 - contrib: bump libupnp to 1.14.7
- 266ecd71 - upnp_server: add the upnp server module
- c93ee88d - upnp_server: add upnp server to intf creation callback
Toggle commit list-
57ee0343...2497347f - 66 commits from branch
added MRStatus::InReview label and removed MRStatus::NotCompliant label
added 80 commits
-
c0b75704...c882e2c8 - 71 commits from branch
videolan:master
- bd3b2a30 - renderer_common: use vlc_object instead of vlc_stream
- 1b27b800 - stream_out: dlna: publicly expose protocol info function
- 5241f24a - stream_out: dlna: constify the profile list
- 3ce1f91b - upnp-wrapper: remove pointless `#if` guards
- ddab8494 - upnp: prioritize `UpnpInit2()` over `UpnpInit()`
- b41702d2 - contrib: bump libupnp to 1.14.7
- caf20f2e - upnp_server: add the upnp server module
- 514492c5 - upnp_server: add upnp server to intf creation callback
- 55271bb3 - upnp_server: add startup dialog confirmation
Toggle commit list-
c0b75704...c882e2c8 - 71 commits from branch
- Resolved by Thomas Guillem
Hi,
Here's some update on what's going on these last branch update.
After trying the server on real scenarios and getting the point of view of j-b, we decided to drop the dialogs showing at each new connection. These are a great solution to be transparent on when the server is running and to what the clients are requesting but, in practice, are way too annoying to use for the standard user.
Instead we decided to go back to display a clear and concise warning at module startup. This warning is displayed by a dialog in a separate thread so it doesn't block
Open()
as discussed previously. If the warning dialog is cancelled or unavailable, the server does not start and the module essentially does nothing.There is still a volatile option available to disable the dialog for advanced CLI users. The option has been renamed
--upnp-server-accept-risk
for better transparency.I've put these changes in the latest commit so they are easily reviewable, the commit will eventually be squashed before merge.
Edited by Alaric Senat