[vlc-devel] [PATCH 1/8] Fix Metadata marshalling when sending the PropertiesChanged signal

[Alex Merry]

VLC was getting kicked from the D-Bus when a track was stopped, because
it was generating invalid data on the wire when sending the
PropertiesChanged signal for the Metadata property.

The issue was that if there was now no current track, GetInputMeta would
never be called and the the variant would never be populated with the
"a{sv}" structure that the call to dbus_message_iter_open_container
claimed it would be.

We now share the code that GetProperties used, which dealt with this
correctly, although now both use CurrentInput (which is what the signal
previously used) instead of CurrentPlayingItem (which is what
GetProperties previously used).
---
 modules/control/dbus/dbus_player.c | 58 ++++++++++++++++++++++----------------
 1 file changed, 33 insertions(+), 25 deletions(-)

diff --git a/modules/control/dbus/dbus_player.c b/modules/control/dbus/dbus_player.c
index 5d24af1..ef40688 100644
--- a/modules/control/dbus/dbus_player.c
+++ b/modules/control/dbus/dbus_player.c
@@ -4,10 +4,12 @@
  * Copyright © 2006-2011 Rafaël Carré
  * Copyright © 2007-2011 Mirsal Ennaime
  * Copyright © 2009-2011 The VideoLAN team
+ * Copyright © 2013      Alex Merry
  * $Id$
  *
  * Authors:    Mirsal Ennaime <mirsal at mirsal fr>
  *             Rafaël Carré <funman at videolanorg>
+ *             Alex Merry <dev at randomguy3 me uk>
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -673,30 +675,46 @@ DBUS_METHOD( LoopStatusSet )
     REPLY_SEND;
 }
 
+static int
+MarshalMetadata( intf_thread_t *p_intf, DBusMessageIter *container )
+{
+    DBusMessageIter a;
+    playlist_t *p_playlist = p_intf->p_sys->p_playlist;
+    input_item_t *p_item = 0;
+
+    input_thread_t *p_input;
+    if( ( p_input = playlist_CurrentInput( p_intf->p_sys->p_playlist ) ) ) {
+        PL_LOCK;
+        p_item = input_GetItem( p_input );
+        if( p_item )
+            GetInputMeta( p_item, container );
+        PL_UNLOCK;
+        vlc_object_release( (vlc_object_t*) p_input );
+    }
+    if (!p_item) {
+        // avoid breaking the type marshalling
+        if( !dbus_message_iter_open_container( container, DBUS_TYPE_ARRAY, "{sv}", &a ) ||
+              !dbus_message_iter_close_container( container, &a ) ) {
+            PL_UNLOCK;
+            return DBUS_HANDLER_RESULT_NEED_MEMORY;
+        }
+    }
+
+    return VLC_SUCCESS;
+}
+
 DBUS_METHOD( Metadata )
 {
     REPLY_INIT;
     OUT_ARGUMENTS;
 
-    DBusMessageIter v, a;
-    playlist_t *p_playlist = PL;
+    DBusMessageIter v;
 
     if( !dbus_message_iter_open_container( &args, DBUS_TYPE_VARIANT,
                                            "a{sv}", &v ) )
         return DBUS_HANDLER_RESULT_NEED_MEMORY;
 
-    PL_LOCK;
-    playlist_item_t* p_item =  playlist_CurrentPlayingItem( p_playlist );
-
-    if( p_item )
-        GetInputMeta( p_item->p_input, &v );
-
-    PL_UNLOCK;
-
-    if( ( !p_item &&
-        ( !dbus_message_iter_open_container( &v, DBUS_TYPE_ARRAY, "{sv}", &a ) ||
-          !dbus_message_iter_close_container( &v, &a ) ) ) ||
-
+    if( MarshalMetadata( p_this, &v ) != VLC_SUCCESS ||
         !dbus_message_iter_close_container( &args, &v ) ) {
         return DBUS_HANDLER_RESULT_NEED_MEMORY;
     }
@@ -888,20 +906,10 @@ PropertiesChangedSignal( intf_thread_t    *p_intf,
 
         if( !strcmp( ppsz_properties[i], "Metadata" ) )
         {
-            input_thread_t *p_input;
-            p_input = playlist_CurrentInput( p_intf->p_sys->p_playlist );
-
             dbus_message_iter_open_container( &entry,
                                               DBUS_TYPE_VARIANT, "a{sv}",
                                               &variant );
-
-            if( p_input )
-            {
-                input_item_t *p_item = input_GetItem( p_input );
-                GetInputMeta( p_item, &variant );
-                vlc_object_release( p_input );
-            }
-
+            MarshalMetadata( p_intf, &variant );
             dbus_message_iter_close_container( &entry, &variant );
         }
         else if( !strcmp( ppsz_properties[i], "PlaybackStatus" ) )
-- 
1.8.1.1