[vlc-commits] [Git][videolan/vlc][master] 7 commits: mtp: fix device name

Hugo Beauzée-Luyssen (@chouquette) gitlab at videolan.org
Fri Nov 26 08:45:01 UTC 2021



Hugo Beauzée-Luyssen pushed to branch master at VideoLAN / VLC


Commits:
20b82357 by Romain Vimont at 2021-11-26T08:30:39+00:00
mtp: fix device name

LIBMTP_Get_Friendlyname() may return an empty string. Since it is not
NULL, this name was used as the device name.

Instead, in that case, fallback to the model name.

- - - - -
d91e3439 by Romain Vimont at 2021-11-26T08:30:39+00:00
mtp: fix leak on error

- - - - -
d1ca62de by Romain Vimont at 2021-11-26T08:30:39+00:00
mtp: reorder to avoid forward declarations

No functional changes.

- - - - -
e5e20a5e by Romain Vimont at 2021-11-26T08:30:39+00:00
mtp: remove remaining items on close

Do not leak the remaining items.

- - - - -
8a9d595a by Romain Vimont at 2021-11-26T08:30:39+00:00
mtp: store items in vlc_vector

Allocating the pp_items array required to know the number of items in
advance.

To do so, the mtp module hijacked the progression callback to know the
number of items on listing (which was called once for each item, but in
the end the total value was correct).

This callback function accepts a "userdata" having type `void const *`,
so it not intended to write to it, but it still worked without ugly cast
due to an additional redirection (p_sys).

To prepare further refactors (multi-device support), remove this
callback hack and use a vlc_vector to append items on-the-fly.

- - - - -
1533bef8 by Romain Vimont at 2021-11-26T08:30:39+00:00
mtp: add a parent device node

Refactor to use a list of devices, and use a device node, parent of all
discovered media for the device.

This also prepares to add support for several MTP devices at the same
time.

Fixes #26085

- - - - -
ea53570b by Romain Vimont at 2021-11-26T08:30:39+00:00
mtp: add support for multiple devices

The implementation of the MTP service discovery could only support one
device at a time.

Now that the structures to store a list of devices is in place, perform
a diff between the known devices and the detected devices to update the
list accordingly.

- - - - -


1 changed file:

- modules/services_discovery/mtp.c


Changes:

=====================================
modules/services_discovery/mtp.c
=====================================
@@ -26,56 +26,29 @@
 
 #define VLC_MODULE_LICENSE VLC_LICENSE_GPL_2_PLUS
 #include <vlc_common.h>
+#include <vlc_list.h>
 #include <vlc_plugin.h>
 #include <vlc_services_discovery.h>
+#include <vlc_vector.h>
 
 #include <libmtp.h>
 
-/*****************************************************************************
- * Module descriptor
- *****************************************************************************/
-static int Open( vlc_object_t * );
-static void Close( vlc_object_t * );
-
-VLC_SD_PROBE_HELPER("mtp", N_("MTP devices"), SD_CAT_DEVICES)
-
-vlc_module_begin()
-    set_shortname( "MTP" )
-    set_description( N_( "MTP devices" ) )
-    set_category( CAT_PLAYLIST )
-    set_subcategory( SUBCAT_PLAYLIST_SD )
-    set_capability( "services_discovery", 0 )
-    set_callbacks( Open, Close )
-    cannot_unload_broken_library()
-
-    VLC_SD_PROBE_SUBMODULE
-vlc_module_end()
-
-
-/*****************************************************************************
- * Local prototypes
- *****************************************************************************/
-
-static void *Run( void * );
-
-static int AddDevice( services_discovery_t *, LIBMTP_raw_device_t * );
-static void AddTrack( services_discovery_t *, LIBMTP_track_t *);
-static void CloseDevice( services_discovery_t * );
-static int CountTracks( uint64_t const, uint64_t const, void const * const );
-
-/*****************************************************************************
- * Local structures
- *****************************************************************************/
+typedef struct VLC_VECTOR(input_item_t *) vec_items_t;
 
 typedef struct
 {
-    int i_tracks_num;
-    input_item_t **pp_items;
-    int i_count;
+    struct vlc_list node;
+    input_item_t *root;
+    vec_items_t items;
     char *psz_name;
     uint32_t i_bus;
     uint8_t i_dev;
     uint16_t i_product_id;
+} mtp_device_t;
+
+typedef struct
+{
+    struct vlc_list devices; /**< list of mtp_device_t */
     vlc_thread_t thread;
 } services_discovery_sys_t;
 
@@ -85,98 +58,105 @@ static void vlc_libmtp_init(void *data)
     LIBMTP_Init();
 }
 
-/*****************************************************************************
- * Open: initialize and create stuff
- *****************************************************************************/
-static int Open( vlc_object_t *p_this )
+/* Takes ownership of the name */
+static mtp_device_t *
+DeviceNew( char *name )
 {
-    services_discovery_t *p_sd = ( services_discovery_t * )p_this;
-    services_discovery_sys_t *p_sys;
+    mtp_device_t *device = malloc( sizeof( *device ) );
+    if ( !device )
+        return NULL;
 
-    if( !( p_sys = malloc( sizeof( services_discovery_sys_t ) ) ) )
-        return VLC_ENOMEM;
-    p_sd->p_sys = p_sys;
-    p_sd->description = _("MTP devices");
-    p_sys->psz_name = NULL;
+    device->root = input_item_NewExt( "vlc://nop", name,
+                                      INPUT_DURATION_INDEFINITE,
+                                      ITEM_TYPE_NODE, ITEM_LOCAL );
+    if ( !device->root )
+    {
+        free( device );
+        return NULL;
+    }
 
-    static vlc_once_t mtp_init_once = VLC_STATIC_ONCE;
+    device->psz_name = name;
 
-    vlc_once(&mtp_init_once, vlc_libmtp_init, NULL);
+    vlc_vector_init( &device->items );
 
-    if (vlc_clone (&p_sys->thread, Run, p_sd, VLC_THREAD_PRIORITY_LOW))
-    {
-        free (p_sys);
-        return VLC_EGENERIC;
-    }
-    return VLC_SUCCESS;
+    return device;
 }
 
-/*****************************************************************************
- * Close: cleanup
- *****************************************************************************/
-static void Close( vlc_object_t *p_this )
+static void
+DeviceDelete( mtp_device_t *device )
 {
-    services_discovery_t *p_sd = ( services_discovery_t * )p_this;
-    services_discovery_sys_t *p_sys = p_sd->p_sys;
+    for (size_t i = 0; i < device->items.size; ++i )
+        input_item_Release( device->items.data[i] );
+    vlc_vector_destroy( &device->items );
 
-    free( p_sys->psz_name );
-    vlc_cancel( p_sys->thread );
-    vlc_join( p_sys->thread, NULL );
-    free( p_sys );
+    input_item_Release( device->root );
+    free( device->psz_name );
+    free( device );
 }
 
-/*****************************************************************************
- * Run: main thread
- *****************************************************************************/
-static void *Run( void *data )
+static char *GetDeviceName( LIBMTP_mtpdevice_t *p_device )
 {
-    LIBMTP_raw_device_t *p_rawdevices;
-    int i_numrawdevices;
-    int i_ret;
-    int i_status = 0;
-    services_discovery_t *p_sd = data;
+    char *name = LIBMTP_Get_Friendlyname( p_device );
+    if ( !EMPTY_STR( name ) )
+        return name;
 
-    for(;;)
+    name = LIBMTP_Get_Modelname( p_device );
+    if ( !EMPTY_STR( name ) )
+        return name;
+
+    return strdup( "MTP Device" );
+}
+
+static void AddTrack( services_discovery_t *p_sd, mtp_device_t *device,
+                      LIBMTP_track_t *p_track )
+{
+    input_item_t *p_input;
+    char *psz_string;
+    char *extension;
+
+    extension = rindex( p_track->filename, '.' );
+    if( asprintf( &psz_string, "mtp://%"PRIu32":%"PRIu8":%"PRIu16":%d%s",
+                  device->i_bus, device->i_dev,
+                  device->i_product_id, p_track->item_id,
+                  extension ) == -1 )
     {
-        int canc = vlc_savecancel();
-        i_ret = LIBMTP_Detect_Raw_Devices( &p_rawdevices, &i_numrawdevices );
-        if ( i_ret == 0 && i_numrawdevices > 0 && p_rawdevices != NULL &&
-             i_status == 0 )
-        {
-            /* Found a new device, add it */
-            msg_Dbg( p_sd, "New device found" );
-            if( AddDevice( p_sd, &p_rawdevices[0] ) == VLC_SUCCESS )
-                i_status = 1;
-            else
-                i_status = 2;
-        }
-        else
-        {
-            if ( ( i_ret != 0 || i_numrawdevices == 0 || p_rawdevices == NULL )
-                 && i_status == 1)
-            {
-                /* The device is not connected anymore, delete it */
-                msg_Info( p_sd, "Device disconnected" );
-                CloseDevice( p_sd );
-                i_status = 0;
-            }
-        }
-        free( p_rawdevices );
-        vlc_restorecancel(canc);
-        if( i_status == 2 )
-        {
-            vlc_tick_sleep( VLC_TICK_FROM_SEC(5) );
-            i_status = 0;
-        }
-        else
-            vlc_tick_sleep( VLC_TICK_FROM_MS(500) );
+        msg_Err( p_sd, "Error adding %s, skipping it", p_track->filename );
+        return;
     }
-    return NULL;
+    if( ( p_input = input_item_New( psz_string, p_track->title ) ) == NULL )
+    {
+        msg_Err( p_sd, "Error adding %s, skipping it", p_track->filename );
+        free( psz_string );
+        return;
+    }
+    free( psz_string );
+
+    input_item_SetArtist( p_input, p_track->artist );
+    input_item_SetGenre( p_input, p_track->genre );
+    input_item_SetAlbum( p_input, p_track->album );
+    if( asprintf( &psz_string, "%d", p_track->tracknumber ) != -1 )
+    {
+        input_item_SetTrackNum( p_input, psz_string );
+        free( psz_string );
+    }
+    if( asprintf( &psz_string, "%d", p_track->rating ) != -1 )
+    {
+        input_item_SetRating( p_input, psz_string );
+        free( psz_string );
+    }
+    input_item_SetDate( p_input, p_track->date );
+    p_input->i_duration = VLC_TICK_FROM_MS(p_track->duration);
+
+    if ( !vlc_vector_push( &device->items, p_input ) )
+    {
+        msg_Err( p_sd, "Error adding %s, skipping it", p_track->filename );
+        input_item_Release( p_input );
+        return;
+    }
+
+    services_discovery_AddSubItem( p_sd, device->root, p_input );
 }
 
-/*****************************************************************************
- * Everything else
- *****************************************************************************/
 static int AddDevice( services_discovery_t *p_sd,
                       LIBMTP_raw_device_t *p_raw_device )
 {
@@ -187,41 +167,46 @@ static int AddDevice( services_discovery_t *p_sd,
 
     if( ( p_device = LIBMTP_Open_Raw_Device( p_raw_device ) ) != NULL )
     {
-        if( !( psz_name = LIBMTP_Get_Friendlyname( p_device ) ) )
-            if( !( psz_name = LIBMTP_Get_Modelname( p_device ) ) )
-                if( !( psz_name = strdup( N_( "MTP Device" ) ) ) )
-                    return VLC_ENOMEM;
+        psz_name = GetDeviceName( p_device );
+        if ( !psz_name )
+            return VLC_ENOMEM;
         msg_Info( p_sd, "Found device: %s", psz_name );
-        p_sys->i_bus = p_raw_device->bus_location;
-        p_sys->i_dev = p_raw_device->devnum;
-        p_sys->i_product_id = p_raw_device->device_entry.product_id;
+
+        /* The device takes ownership of the name */
+        mtp_device_t *mtp_device = DeviceNew( psz_name );
+        if ( !mtp_device )
+        {
+            free( psz_name );
+            return VLC_ENOMEM;
+        }
+
+        mtp_device->i_bus = p_raw_device->bus_location;
+        mtp_device->i_dev = p_raw_device->devnum;
+        mtp_device->i_product_id = p_raw_device->device_entry.product_id;
+
+        vlc_list_append( &mtp_device->node, &p_sys->devices );
+
+        services_discovery_AddItem( p_sd, mtp_device->root );
+
         if( ( p_track = LIBMTP_Get_Tracklisting_With_Callback( p_device,
-                            CountTracks, p_sd ) ) == NULL )
+                            NULL, NULL ) ) == NULL )
         {
             msg_Warn( p_sd, "No tracks on the device" );
-            p_sys->pp_items = NULL;
         }
         else
         {
-            if( !( p_sys->pp_items = calloc( p_sys->i_tracks_num,
-                                                   sizeof( input_item_t * ) ) ) )
-            {
-                free( psz_name );
-                return VLC_ENOMEM;
-            }
-            p_sys->i_count = 0;
             while( p_track != NULL )
             {
                 msg_Dbg( p_sd, "Track found: %s - %s", p_track->artist,
                          p_track->title );
-                AddTrack( p_sd, p_track );
+                AddTrack( p_sd, mtp_device, p_track );
                 p_tmp = p_track;
                 p_track = p_track->next;
                 LIBMTP_destroy_track_t( p_tmp );
             }
         }
-        p_sys->psz_name = psz_name;
         LIBMTP_Release_Device( p_device );
+
         return VLC_SUCCESS;
     }
     else
@@ -231,74 +216,151 @@ static int AddDevice( services_discovery_t *p_sd,
     }
 }
 
-static void AddTrack( services_discovery_t *p_sd, LIBMTP_track_t *p_track )
+static void CloseDevice( services_discovery_t *p_sd, mtp_device_t *device )
+{
+    /* Notify the removal of the whole node and its children */
+    services_discovery_RemoveItem( p_sd, device->root );
+
+    vlc_list_remove( &device->node );
+    DeviceDelete( device );
+}
+
+static int
+FindRawDevice( const LIBMTP_raw_device_t *raw_devices, int count,
+               uint32_t bus_location, uint8_t devnum )
+{
+    for ( int i = 0; i < count; ++i )
+        if ( raw_devices[i].bus_location == bus_location &&
+             raw_devices[i].devnum == devnum )
+            return i;
+    return -1;
+}
+
+static mtp_device_t *
+FindDevice( struct vlc_list *devices, uint32_t bus_location, uint8_t devnum )
+{
+    mtp_device_t *device;
+    vlc_list_foreach( device, devices, node )
+        if ( device->i_bus == bus_location &&
+             device->i_dev == devnum )
+            return device;
+    return NULL;
+}
+
+static void
+UpdateDevices( services_discovery_t *p_sd,
+               LIBMTP_raw_device_t *raw_devices, int count )
 {
     services_discovery_sys_t *p_sys = p_sd->p_sys;
-    input_item_t *p_input;
-    char *psz_string;
-    char *extension;
 
-    extension = rindex( p_track->filename, '.' );
-    if( asprintf( &psz_string, "mtp://%"PRIu32":%"PRIu8":%"PRIu16":%d%s",
-                  p_sys->i_bus, p_sys->i_dev,
-                  p_sys->i_product_id, p_track->item_id,
-                  extension ) == -1 )
-    {
-        msg_Err( p_sd, "Error adding %s, skipping it", p_track->filename );
-        return;
-    }
-    if( ( p_input = input_item_New( psz_string, p_track->title ) ) == NULL )
-    {
-        msg_Err( p_sd, "Error adding %s, skipping it", p_track->filename );
-        free( psz_string );
-        return;
-    }
-    free( psz_string );
+    /* Remove devices which have disappeared */
 
-    input_item_SetArtist( p_input, p_track->artist );
-    input_item_SetGenre( p_input, p_track->genre );
-    input_item_SetAlbum( p_input, p_track->album );
-    if( asprintf( &psz_string, "%d", p_track->tracknumber ) != -1 )
+    mtp_device_t *device;
+    vlc_list_foreach( device, &p_sys->devices, node )
     {
-        input_item_SetTrackNum( p_input, psz_string );
-        free( psz_string );
+        int idx = FindRawDevice( raw_devices, count, device->i_bus,
+                                 device->i_dev );
+        if ( idx == -1 )
+        {
+            /* Not found */
+            msg_Info( p_sd, "Device disconnected" );
+            CloseDevice( p_sd, device );
+        }
     }
-    if( asprintf( &psz_string, "%d", p_track->rating ) != -1 )
+
+    /* Add new detected devices */
+
+    for ( int i = 0; i < count; ++i )
     {
-        input_item_SetRating( p_input, psz_string );
-        free( psz_string );
+        LIBMTP_raw_device_t *raw_device = &raw_devices[i];
+        device = FindDevice( &p_sys->devices, raw_device->bus_location,
+                             raw_device->devnum );
+        if ( !device )
+        {
+            /* Device not found in the list, it's a new one */
+            msg_Dbg( p_sd, "New device detected" );
+            if ( AddDevice( p_sd, raw_device ) != VLC_SUCCESS )
+                msg_Err( p_sd, "Could not add MTP device" );
+        }
     }
-    input_item_SetDate( p_input, p_track->date );
-    p_input->i_duration = VLC_TICK_FROM_MS(p_track->duration);
-    services_discovery_AddItem( p_sd, p_input );
-    p_sys->pp_items[p_sys->i_count++] = p_input;
 }
 
-static void CloseDevice( services_discovery_t *p_sd )
+/*****************************************************************************
+ * Run: main thread
+ *****************************************************************************/
+static void *Run( void *data )
 {
-    services_discovery_sys_t *p_sys = p_sd->p_sys;
-    input_item_t **pp_items = p_sys->pp_items;
+    LIBMTP_raw_device_t *p_rawdevices;
+    int i_numrawdevices;
+    services_discovery_t *p_sd = data;
 
-    if( pp_items != NULL )
+    for(;;)
     {
-        for( int i_i = 0; i_i < p_sys->i_count; i_i++ )
+        int canc = vlc_savecancel();
+        int ret = LIBMTP_Detect_Raw_Devices( &p_rawdevices, &i_numrawdevices );
+        if ( ret == LIBMTP_ERROR_NONE ||
+             ret == LIBMTP_ERROR_NO_DEVICE_ATTACHED )
         {
-            if( pp_items[i_i] != NULL )
-            {
-                services_discovery_RemoveItem( p_sd, pp_items[i_i] );
-                input_item_Release( pp_items[i_i] );
-            }
+            UpdateDevices( p_sd, p_rawdevices, i_numrawdevices );
+            if ( ret == LIBMTP_ERROR_NONE )
+                free( p_rawdevices );
         }
-        free( pp_items );
+        vlc_restorecancel(canc);
+
+        vlc_tick_sleep( VLC_TICK_FROM_MS(500) );
+    }
+    return NULL;
+}
+
+static int Open( vlc_object_t *p_this )
+{
+    services_discovery_t *p_sd = ( services_discovery_t * )p_this;
+    services_discovery_sys_t *p_sys;
+
+    if( !( p_sys = malloc( sizeof( services_discovery_sys_t ) ) ) )
+        return VLC_ENOMEM;
+    p_sd->p_sys = p_sys;
+    p_sd->description = _("MTP devices");
+
+    vlc_list_init( &p_sys->devices );
+
+    static vlc_once_t mtp_init_once = VLC_STATIC_ONCE;
+
+    vlc_once(&mtp_init_once, vlc_libmtp_init, NULL);
+
+    if (vlc_clone (&p_sys->thread, Run, p_sd, VLC_THREAD_PRIORITY_LOW))
+    {
+        free (p_sys);
+        return VLC_EGENERIC;
     }
+    return VLC_SUCCESS;
 }
 
-static int CountTracks( uint64_t const sent, uint64_t const total,
-                        void const * const data )
+static void Close( vlc_object_t *p_this )
 {
-    VLC_UNUSED( sent );
-    services_discovery_t *p_sd = (services_discovery_t *)data;
+    services_discovery_t *p_sd = ( services_discovery_t * )p_this;
     services_discovery_sys_t *p_sys = p_sd->p_sys;
-    p_sys->i_tracks_num = total;
-    return 0;
+
+    vlc_cancel( p_sys->thread );
+    vlc_join( p_sys->thread, NULL );
+
+    mtp_device_t *device;
+    vlc_list_foreach( device, &p_sys->devices, node )
+        DeviceDelete( device );
+
+    free( p_sys );
 }
+
+VLC_SD_PROBE_HELPER("mtp", N_("MTP devices"), SD_CAT_DEVICES)
+
+vlc_module_begin()
+    set_shortname( "MTP" )
+    set_description( N_( "MTP devices" ) )
+    set_category( CAT_PLAYLIST )
+    set_subcategory( SUBCAT_PLAYLIST_SD )
+    set_capability( "services_discovery", 0 )
+    set_callbacks( Open, Close )
+    cannot_unload_broken_library()
+
+    VLC_SD_PROBE_SUBMODULE
+vlc_module_end()



View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/7195529927c75d021a4427becd5119677f8fe0d7...ea53570b29c115defa2bcc948919c9cdd0a0cff2

-- 
View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/7195529927c75d021a4427becd5119677f8fe0d7...ea53570b29c115defa2bcc948919c9cdd0a0cff2
You're receiving this email because of your account on code.videolan.org.




More information about the vlc-commits mailing list