[vlc-devel] [PATCH] PnP discovery: Evaluate "TotalMatches" and "NumberReturned"

Hugo Beauzée-Luyssen hugo at beauzee.fr
Wed Apr 22 09:07:01 CEST 2020


On Thu, Apr 16, 2020, at 3:22 PM, Andreas Krug wrote:
> Hi All,
> 
> I was asked again by a user for this patch because VLC-Player Version 
> 3.0.9.2 Vetinari does not have a solution.
> Who can review the patch?
> Is there a process to set priority?
> 
> Next ticket created around that 4 months ago:
> https://trac.videolan.org/vlc/ticket/23554
> 
> and theses are still open:
> https://trac.videolan.org/vlc/ticket/15876
> https://trac.videolan.org/vlc/ticket/21381
> 
> Thanks
> 
> Andreas
> 
> On 18.11.19 23:07, Andreas Krug wrote:
> > Hi All,
> >
> > ping again for review.
> >
> > Seems that there some more user than me facing this issue with 
> > Panasonic, received an e-mail on Friday from a user.
> >
> > Affected tickets:
> >
> > https://trac.videolan.org/vlc/ticket/15876
> > https://trac.videolan.org/vlc/ticket/22496
> > https://trac.videolan.org/vlc/ticket/21381
> >
> > Thanks
> >
> > Andreas
> >
> >
> > On 16.07.19 16:14, Andreas Krug wrote:
> >>      Browse again with increased "StartingIndex" and adapted 
> >> "RequestCount".
> >>      Solves
> >>      #21381 Panasonic Viera returns maximal 20 items on uPnP
> >>      #22496 DLNA/UPnP - Panasonic recorder 12 records limit
> >> ---
> >>   modules/services_discovery/upnp.cpp | 60 +++++++++++++++++++----------
> >>   modules/services_discovery/upnp.hpp |  2 +-
> >>   2 files changed, 41 insertions(+), 21 deletions(-)
> >>
> >> diff --git a/modules/services_discovery/upnp.cpp 
> >> b/modules/services_discovery/upnp.cpp
> >> index f1037c82c8..26d62c25e6 100644
> >> --- a/modules/services_discovery/upnp.cpp
> >> +++ b/modules/services_discovery/upnp.cpp
> >> @@ -1142,6 +1142,7 @@ int MediaServer::sendActionCb( Upnp_EventType 
> >> eventType,
> >>   IXML_Document* MediaServer::_browseAction( const char* psz_object_id_,
> >>                                              const char* 
> >> psz_browser_flag_,
> >>                                              const char* psz_filter_,
> >> +                                           const char* 
> >> psz_starting_index,
> >>                                              const char* 
> >> psz_requested_count_,
> >>                                              const char* 
> >> psz_sort_criteria_ )
> >>   {
> >> @@ -1186,7 +1187,7 @@ IXML_Document* MediaServer::_browseAction( 
> >> const char* psz_object_id_,
> >>       }
> >>         i_res = UpnpAddToAction( &p_action, "Browse",
> >> -            CONTENT_DIRECTORY_SERVICE_TYPE, "StartingIndex", "0" );
> >> +            CONTENT_DIRECTORY_SERVICE_TYPE, "StartingIndex", 
> >> psz_starting_index );
> >>       if ( i_res != UPNP_E_SUCCESS )
> >>       {
> >>           msg_Dbg( m_access, "AddToAction 'StartingIndex' failed: %s",
> >> @@ -1242,53 +1243,72 @@ browseActionCleanup:
> >>    */
> >>   bool MediaServer::fetchContents()
> >>   {
> >> -    IXML_Document* p_response = _browseAction( m_psz_objectId,
> >> +    std::string StartingIndex = "0";
> >> +    std::string RequestedCount = "5000";
> >> +    const char* psz_TotalMatches = "0";
> >> +    const char* psz_NumberReturned = "0";
> >> +    long  l_reqCount = 0;
> >> +
> >> +    do
> >> +    {
> >> +      IXML_Document* p_response = _browseAction( m_psz_objectId,
> >>                                         "BrowseDirectChildren",
> >>                                         "*",
> >> +                                      StartingIndex.c_str(),
> >>                                         // Some servers don't 
> >> understand "0" as "no-limit"
> >> -                                      "5000", /* RequestedCount */
> >> +                                      RequestedCount.c_str(), /* 
> >> RequestedCount */
> >>                                         "" /* SortCriteria */
> >>                                         );
> >> -    if ( !p_response )
> >> -    {
> >> +      if ( !p_response )
> >> +      {
> >>           msg_Err( m_access, "No response from browse() action" );
> >>           return false;
> >> -    }
> >> +      }
> >>   -    IXML_Document* p_result = parseBrowseResult( p_response );
> >> +      psz_TotalMatches = xml_getChildElementValue( 
> >> (IXML_Element*)p_response, "TotalMatches" );
> >> +      psz_NumberReturned = xml_getChildElementValue( 
> >> (IXML_Element*)p_response, "NumberReturned" );
> >>   -    ixmlDocument_free( p_response );
> >> +      StartingIndex = std::to_string( std::stol(psz_NumberReturned) 
> >> + std::stol(StartingIndex) );
> >> +      l_reqCount = std::stol(psz_TotalMatches) - 
> >> std::stol(StartingIndex) ;
> >> +      RequestedCount = std::to_string(l_reqCount);
> >>   -    if ( !p_result )
> >> -    {
> >> +      IXML_Document* p_result = parseBrowseResult( p_response );
> >> +
> >> +      ixmlDocument_free( p_response );
> >> +
> >> +      if ( !p_result )
> >> +      {
> >>           msg_Err( m_access, "browse() response parsing failed" );
> >>           return false;
> >> -    }
> >> +      }
> >>     #ifndef NDEBUG
> >> -    msg_Dbg( m_access, "Got DIDL document: %s", ixmlPrintDocument( 
> >> p_result ) );
> >> +     msg_Dbg( m_access, "Got DIDL document: %s", ixmlPrintDocument( 
> >> p_result ) );
> >>   #endif
> >>   -    IXML_NodeList* containerNodeList =
> >> +     IXML_NodeList* containerNodeList =
> >>                   ixmlDocument_getElementsByTagName( p_result, 
> >> "container" );
> >>   -    if ( containerNodeList )
> >> -    {
> >> +     if ( containerNodeList )
> >> +     {
> >>           for ( unsigned int i = 0; i < ixmlNodeList_length( 
> >> containerNodeList ); i++ )
> >>               addContainer( (IXML_Element*)ixmlNodeList_item( 
> >> containerNodeList, i ) );
> >>           ixmlNodeList_free( containerNodeList );
> >> -    }
> >> +     }
> >>   -    IXML_NodeList* itemNodeList = 
> >> ixmlDocument_getElementsByTagName( p_result,
> >> +     IXML_NodeList* itemNodeList = 
> >> ixmlDocument_getElementsByTagName( p_result,
> >> "item" );
> >> -    if ( itemNodeList )
> >> -    {
> >> +     if ( itemNodeList )
> >> +     {
> >>           for ( unsigned int i = 0; i < ixmlNodeList_length( 
> >> itemNodeList ); i++ )
> >>               addItem( (IXML_Element*)ixmlNodeList_item( 
> >> itemNodeList, i ) );
> >>           ixmlNodeList_free( itemNodeList );
> >> +      }
> >> +
> >> +      ixmlDocument_free( p_result );
> >>       }
> >> +    while( l_reqCount );
> >>   -    ixmlDocument_free( p_result );
> >>       return true;
> >>   }
> >>   diff --git a/modules/services_discovery/upnp.hpp 
> >> b/modules/services_discovery/upnp.hpp
> >> index 9c128905d0..ab3e9c999b 100644
> >> --- a/modules/services_discovery/upnp.hpp
> >> +++ b/modules/services_discovery/upnp.hpp
> >> @@ -157,7 +157,7 @@ private:
> >>       bool addContainer( IXML_Element* containerElement );
> >>       bool addItem( IXML_Element* itemElement );
> >>   -    IXML_Document* _browseAction(const char*, const char*,
> >> +    IXML_Document* _browseAction(const char*, const char*, const char*,
> >>               const char*, const char*, const char* );
> >>       static int sendActionCb( Upnp_EventType, UpnpEventPtr, void *);
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel

Hi,

Sorry about the delay...
The patch looks good to me, however it doesn't apply anymore. Could you rebase it on top of the current master?
Additionally, could you use 4 spaces instead of 2, and ideally split the indentation of existing code in the new loop in a 2nd commit?

Thanks a lot,

Regards,

-- 
  Hugo Beauzée-Luyssen
  hugo at beauzee.fr


More information about the vlc-devel mailing list