[vlc-devel] [PATCH v3 2/6] upnp: Add basic support for registering MediaRenderer service
Hugo Beauzée-Luyssen
hugo at beauzee.fr
Mon Aug 12 11:17:48 CEST 2019
On Sat, Aug 10, 2019, at 1:30 PM, Johan Gunnarsson wrote:
> Den ons 7 aug. 2019 kl 15:42 skrev Hugo Beauzée-Luyssen <hugo at beauzee.fr>:
> >
> > Hi,
> >
> > On Thu, Jul 25, 2019, at 11:26 PM, Johan Gunnarsson wrote:
> > > ---
> > > modules/services_discovery/upnp-wrapper.cpp | 109 +++++++++++++++++++-
> > > modules/services_discovery/upnp-wrapper.hpp | 7 ++
> > > 2 files changed, 115 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/modules/services_discovery/upnp-wrapper.cpp
> > > b/modules/services_discovery/upnp-wrapper.cpp
> > > index 38f18a1192..27c3b9595d 100644
> > > --- a/modules/services_discovery/upnp-wrapper.cpp
> > > +++ b/modules/services_discovery/upnp-wrapper.cpp
> > > @@ -36,19 +36,65 @@
> > > #include "upnp-wrapper.hpp"
> > > #include <vlc_cxx_helpers.hpp>
> > >
> > > +static const char *mediarenderer_desc =
> > > + "<?xml version=\"1.0\" encoding=\"utf-8\"?>"
> > > + "<root xmlns=\"urn:schemas-upnp-org:device-1-0\">"
> > > + "<specVersion>"
> > > + "<major>1</major>"
> > > + "<minor>0</minor>"
> > > + "</specVersion>"
> > > + "<device>"
> > > +
> > > "<deviceType>urn:schemas-upnp-org:device:MediaRenderer:1</deviceType>"
> > > + "<friendlyName>VLC media player</friendlyName>" /* TODO:
> > > include hostname */
> > > + "<manufacturer>VideoLAN</manufacturer>"
> > > + "<modelName>" PACKAGE_NAME "</modelName>"
> > > + "<modelNumber>" PACKAGE_VERSION "</modelNumber>"
> > > + "<modelURL>https://www.videolan.org/vlc/</modelURL>"
> > > + "<UDN>" UPNP_UDN "</UDN>" /* TODO: generate at each startup */
> > > + "<serviceList>"
> > > + "<service>"
> > > +
> > > "<serviceType>urn:schemas-upnp-org:service:RenderingControl:1</serviceType>"
> > > +
> > > "<serviceId>urn:upnp-org:serviceId:RenderingControl</serviceId>"
> > > + "<SCPDURL>/RenderingControlSCPD.xml</SCPDURL>"
> > > + "<controlURL>/upnp/control/RenderingControl</controlURL>"
> > > + "<eventSubURL>/upnp/event/RenderingControl</eventSubURL>"
> > > + "</service>"
> > > + "<service>"
> > > +
> > > "<serviceType>urn:schemas-upnp-org:service:ConnectionManager:1</serviceType>"
> > > +
> > > "<serviceId>urn:upnp-org:serviceId:ConnectionManager</serviceId>"
> > > + "<SCPDURL>/ConnectionManagerSCPD.xml</SCPDURL>"
> > > + "<controlURL>/upnp/control/ConnectionManager</controlURL>"
> > > + "<eventSubURL>/upnp/event/ConnectionManager</eventSubURL>"
> > > + "</service>"
> > > + "<service>"
> > > +
> > > "<serviceType>urn:schemas-upnp-org:service:AVTransport:1</serviceType>"
> > > + "<serviceId>urn:upnp-org:serviceId:AVTransport</serviceId>"
> > > + "<SCPDURL>/AVTransportSCPD.xml</SCPDURL>"
> > > + "<controlURL>/upnp/control/AVTransport</controlURL>"
> > > + "<eventSubURL>/upnp/event/AVTransport</eventSubURL>"
> > > + "</service>"
> > > + "</serviceList>"
> > > + "</device>"
> > > + "</root>";
> > > +
> > > UpnpInstanceWrapper* UpnpInstanceWrapper::s_instance;
> > > UpnpInstanceWrapper::Listeners UpnpInstanceWrapper::s_listeners;
> > > vlc_mutex_t UpnpInstanceWrapper::s_lock = VLC_STATIC_MUTEX;
> > >
> > > UpnpInstanceWrapper::UpnpInstanceWrapper()
> > > : m_client_handle( -1 )
> > > + , m_device_handle( -1 )
> > > , m_refcount( 0 )
> > > + , m_mediarenderer_refcount( 0 )
> > > {
> > > }
> > >
> > > UpnpInstanceWrapper::~UpnpInstanceWrapper()
> > > {
> > > - UpnpUnRegisterClient( m_client_handle );
> > > + if( m_client_handle != -1 )
> > > + UpnpUnRegisterClient( m_client_handle );
> > > + if( m_device_handle != -1 )
> > > + UpnpUnRegisterRootDevice( m_device_handle );
> > > UpnpFinish();
> > > }
> > >
> > > @@ -104,6 +150,15 @@ UpnpInstanceWrapper
> > > *UpnpInstanceWrapper::get(vlc_object_t *p_obj)
> > > delete instance;
> > > return NULL;
> > > }
> > > +
> > > + char *root = config_GetSysPath( VLC_PKG_DATA_DIR, "upnp" );
> >
> > If config_GetSysPath fails, the later call will disable the webserver and won't return an error, which is probably not the behavior you want
>
> If root is NULL UpnpSetWebServerRootDir will return an error and print
> a warning. When this happens the XML files in the
> VLC_PKG_DATA_DIR/upnp directory will not be available, but the SOAP
> calls are still working fine. I think this is an acceptable best
> effort fallback.
>
Fair enough
> >
> > > + if( (i_res = UpnpSetWebServerRootDir( root )) !=
> > > UPNP_E_SUCCESS)
> > > + {
> > > + msg_Warn( p_obj, "UpnpSetWebServerRootDir failed: %s",
> > > + UpnpGetErrorMessage( i_res ));
> > > + }
> > > + free( root );
> > > +
> > > s_instance = instance;
> > > }
> > > s_instance->m_refcount++;
> > > @@ -128,6 +183,11 @@ UpnpClient_Handle
> > > UpnpInstanceWrapper::client_handle() const
> > > return m_client_handle;
> > > }
> > >
> > > +UpnpDevice_Handle UpnpInstanceWrapper::device_handle() const
> > > +{
> > > + return m_device_handle;
> > > +}
> > > +
> > > int UpnpInstanceWrapper::Callback(Upnp_EventType event_type,
> > > UpnpEventPtr p_event, void *p_user_data)
> > > {
> > > vlc::threads::mutex_locker lock( &s_lock );
> > > @@ -153,3 +213,50 @@ void
> > > UpnpInstanceWrapper::removeListener(ListenerPtr listener)
> > > if ( iter != s_listeners.end() )
> > > s_listeners.erase( iter );
> > > }
> > > +
> > > +void UpnpInstanceWrapper::startMediaRenderer( vlc_object_t *p_obj )
> > > +{
> > > + vlc::threads::mutex_locker lock( &s_lock );
> > > + if( m_mediarenderer_refcount == 0 )
> > > + {
> > > + int i_res;
> > > + if( (i_res = UpnpEnableWebserver( TRUE )) != UPNP_E_SUCCESS)
> > > + {
> > > + msg_Err( p_obj, "Failed to enable webserver: %s",
> > > UpnpGetErrorMessage( i_res ) );
> > > + return;
> > > + }
> > > + i_res = UpnpRegisterRootDevice2( UPNPREG_BUF_DESC,
> > > + mediarenderer_desc,
> > > + strlen(mediarenderer_desc),
> > > + 1,
> > > + Callback,
> > > + NULL,
> >
> > Please use nullptr :)
>
> Feels a bit silly to have nullptr here but NULL in all other places in
> this file. If you really want me to do it I can fix it in v4 of this
> patch.
>
Oh yeah I meant for all new code this patch introduces, but granted there are probably a lot of NULL everywhere in UPnP (and other C++ modules).
At some point I think we'll need to convert the existing code.
> >
> > > + &m_device_handle );
> > > + if( i_res != UPNP_E_SUCCESS )
> > > + {
> > > + msg_Err( p_obj, "Device registration failed: %s",
> > > UpnpGetErrorMessage( i_res ) );
> > > + }
> > > + }
> > > + m_mediarenderer_refcount++;
> > > +}
> > > +
> > > +void UpnpInstanceWrapper::stopMediaRenderer( vlc_object_t *p_obj )
> > > +{
> > > + vlc::threads::mutex_locker lock( &s_lock );
> > > + m_mediarenderer_refcount--;
> > > + if( m_mediarenderer_refcount == 0 )
> > > + {
> > > + int i_res;
> > > + i_res = UpnpUnRegisterRootDevice( m_device_handle );
> > > + if( i_res != UPNP_E_SUCCESS )
> > > + {
> > > + msg_Err( p_obj, "Device unregistration failed: %s",
> > > UpnpGetErrorMessage( i_res ) );
> > > + }
> > > + m_device_handle = -1;
> > > + i_res = UpnpEnableWebserver( FALSE );
> > > + if( i_res != UPNP_E_SUCCESS )
> > > + {
> > > + msg_Warn( p_obj, "Failed to disable webserver: %s",
> > > UpnpGetErrorMessage( i_res ) );
> > > + }
> > > + }
> > > +}
> > > diff --git a/modules/services_discovery/upnp-wrapper.hpp
> > > b/modules/services_discovery/upnp-wrapper.hpp
> > > index c59592b0b3..c9f3e152d4 100644
> > > --- a/modules/services_discovery/upnp-wrapper.hpp
> > > +++ b/modules/services_discovery/upnp-wrapper.hpp
> > > @@ -41,6 +41,8 @@
> > > #include <upnp.h>
> > > #include <upnptools.h>
> > >
> > > +#define UPNP_UDN "uuid:034fc8dc-ec22-44e5-a79b-38c935f11663"
> > > +
> > > #if UPNP_VERSION < 10800
> > > typedef void* UpnpEventPtr;
> > > #else
> > > @@ -71,7 +73,9 @@ private:
> > > static UpnpInstanceWrapper* s_instance;
> > > static vlc_mutex_t s_lock;
> > > UpnpClient_Handle m_client_handle;
> > > + UpnpDevice_Handle m_device_handle;
> > > int m_refcount;
> > > + int m_mediarenderer_refcount;
> > > typedef std::shared_ptr<Listener> ListenerPtr;
> > > typedef std::vector<ListenerPtr> Listeners;
> > > static Listeners s_listeners;
> > > @@ -81,8 +85,11 @@ public:
> > > static UpnpInstanceWrapper* get( vlc_object_t* p_obj );
> > > void release();
> > > UpnpClient_Handle client_handle() const;
> > > + UpnpDevice_Handle device_handle() const;
> > > void addListener(ListenerPtr listener);
> > > void removeListener(ListenerPtr listener);
> > > + void startMediaRenderer( vlc_object_t *p_obj );
> > > + void stopMediaRenderer( vlc_object_t *p_obj );
> > >
> > > private:
> > > static int Callback( Upnp_EventType event_type, UpnpEventPtr
> > > p_event, void* p_user_data );
> > > --
> > > 2.17.1
> > >
> > > _______________________________________________
> > > vlc-devel mailing list
> > > To unsubscribe or modify your subscription options:
> > > https://mailman.videolan.org/listinfo/vlc-devel
> >
> > LGTM otherwise
> >
> > --
> > Hugo Beauzée-Luyssen
> > hugo at beauzee.fr
> > _______________________________________________
> > vlc-devel mailing list
> > To unsubscribe or modify your subscription options:
> > https://mailman.videolan.org/listinfo/vlc-devel
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
--
Hugo Beauzée-Luyssen
hugo at beauzee.fr
More information about the vlc-devel
mailing list