[vlc-devel] [PATCH] GSoC: MTP Devices initial release
rem at videolan.org
Sat Jan 24 21:05:25 CET 2009
Le samedi 24 janvier 2009 21:36:08 Fabio Ritrovato, vous avez écrit :
> Hi there, due to some changes in the code in the last months, most of
> my code is not working anymore (and to get it working i think I'll
> need to have some talk with the big guys...), but I wanted to let you
> have at least the basic part of my work before the feature freeze.
> What's there is the first part of my project, the basic read-only
> service discovery and access modules, to let VLC play files from an
> MTP device.
> If there are problems, let me know and I'll try to fix them ASAP.
> Best regards,
> Fabio Ritrovato
+ set_capability( "access", 50 )
Is MTP using normal file paths? I would have thought it only accepts mtp://...
locations. In the latter case, priority should be zero, if I'm not mistaken.
+ bool b_seekable;
This was meant for the file access to handle FIFOs properly. I assume libmtp
only creates regular files, so you should not need this.
+ bool b_pace_control;
This is always true, hence the variable should be removed completely.
+ sscanf( p_access->psz_path, "%"SCNu32":%"SCNu8":%"SCNu16":%d", &i_bus,
+ &i_dev, &i_product_id, &i_track_id );
I wouldn't hurt to check for formatting errors here.
+ if( asprintf( &psz_to_path, "%s/mtp%d",
+ config_GetCacheDir(), i_track_id ) == -1 )
Maybe you should use a normal temporary file, instead of going to .cache ?
+ i_ret = net_Read (p_access, fd, NULL, p_buffer, i_len, false);
Regular files don't do non-blocking I/O, so net_Read() is wasteful here. You
could probably call read() directly.
+ if( p_access->info.i_size != 0 &&
+ (p_sys->i_nb_reads % INPUT_FSTAT_NB_READS) == 0 )
+ struct stat st;
+ if ((fstat (fd, &st) == 0)
+ && (p_access->info.i_size != st.st_size))
+ p_access->info.i_size = st.st_size;
+ p_access->info.i_update |= INPUT_UPDATE_SIZE;
That's for the file access to detect asynchronous file size changes. I assume
you don't need it here.
Regarding the SD:
+ while( !p_sd->b_die )
Direct access to b_die is not safe outside the object lock. Anyway, you don't
need to check b_die at all as you use vlc_cancel().
Unless LIBMTP is cancellation safe, you should add
`int canc = vlc_savecancel();` here...
+ i_ret = LIBMTP_Detect_Raw_Devices( &p_rawdevices, &i_numrawdevices );
+ if ( i_ret == 0 && i_numrawdevices > 0 && p_rawdevices != NULL &&
+ i_status == 0 )
/* ... */
+ if ( ( i_ret != 0 || i_numrawdevices == 0 || p_rawdevices ==
+ && i_status == 1)
/* ... */
+ CloseDevice( p_sd );
+ i_status = 0;
And `vlc_restorecancel (canc);` here.
+ free( p_rawdevices );
+ msleep( 5000 );
5 ms is waaaay toooo short. It's not only harmful for power consumption, it's
so fast it's harmful for scheduling. If you really cannot handle events
properly, please use something orders longer than that.
+ p_sd->p_sys->p_device = p_device;
You don't use this variable outside of the block, so there is no point in
storing it in the plugin instance data.
+static int CountTracks( uint64_t const sent, uint64_t const total,
+ void const * const data )
+ VLC_UNUSED( sent );
+ VLC_UNUSED( total );
+ VLC_UNUSED( data );
+ p_sd_global->p_sys->i_tracks_num = total;
+ return 0;
What is `data`? It looks like an opaque pointer. If it is, then you should
really use that instead of the ugly p_sd_global.
More information about the vlc-devel