[vlc-devel] [PATCH 7/7] libvlc: add libvlc_media_get_type

[Thomas Guillem]

On Wed, Mar 25, 2015, at 15:57, Rémi Denis-Courmont wrote:
> Le 2015-03-24 20:22, Thomas Guillem a écrit :
> > Get the type of the media.
> > ---
> >  NEWS                       |  1 +
> >  include/vlc/libvlc_media.h | 35 +++++++++++++++++++++++++++++++++++
> >  lib/libvlc.sym             |  1 +
> >  lib/media.c                | 32 ++++++++++++++++++++++++++++++++
> >  4 files changed, 69 insertions(+)
> >
> > diff --git a/NEWS b/NEWS
> > index 3daee6e..b2e443b 100644
> > --- a/NEWS
> > +++ b/NEWS
> > @@ -105,6 +105,7 @@ libVLC:
> >   * Add libvlc_media_parse_with_options that uses a flag to specify
> > parse options
> >   * Add libvlc_audio_output_device_get to get the currently selected
> > audio output device
> >     identifier (if there is one available)
> > + * Add libvlc_media_get_type to get the type of the media
> >
> >  Logging
> >   * Support for the SystemD Journal
> > diff --git a/include/vlc/libvlc_media.h b/include/vlc/libvlc_media.h
> > index 7fc718a..18cb500 100644
> > --- a/include/vlc/libvlc_media.h
> > +++ b/include/vlc/libvlc_media.h
> > @@ -223,6 +223,27 @@ typedef struct libvlc_media_track_t
> >
> >  } libvlc_media_track_t;
> >
> > +/** defgroup libvlc_media_type LibVLC media type
> > + * \ingroup libvlc_media
> > + * @{
> > + */
> > +
> > +/**
> > + * Media type
> > + *
> > + * \see libvlc_media_get_type
> > + */
> > +typedef enum libvlc_media_type_t {
> > +    libvlc_media_type_unknown,
> > +    libvlc_media_type_file,
> > +    libvlc_media_type_directory,
> > +    libvlc_media_type_disc,
> > +    libvlc_media_type_stream,
> > +    libvlc_media_type_playlist,
> > +} libvlc_media_type_t;
> > +
> > +/** @}*/
> > +
> >  /**
> >   * Parse flags used by libvlc_media_parse_with_options()
> >   *
> > @@ -669,6 +690,20 @@ LIBVLC_API
> >  void libvlc_media_tracks_release( libvlc_media_track_t **p_tracks,
> >                                    unsigned i_count );
> >
> > +/**
> > + * Get the media type of the media descriptor object
> > + *
> > + * \version LibVLC 3.0.0 and later.
> > + *
> > + * \see libvlc_media_type_t
> > + *
> > + * \param p_md media descriptor object
> > + *
> > + * \return media type
> > + */
> > +LIBVLC_API
> > +libvlc_media_type_t libvlc_media_get_type( libvlc_media_t *p_md );
> > +
> >  /** @}*/
> >
> >  # ifdef __cplusplus
> > diff --git a/lib/libvlc.sym b/lib/libvlc.sym
> > index 4fd2378..1e241e7 100644
> > --- a/lib/libvlc.sym
> > +++ b/lib/libvlc.sym
> > @@ -90,6 +90,7 @@ libvlc_media_get_meta
> >  libvlc_media_get_mrl
> >  libvlc_media_get_state
> >  libvlc_media_get_stats
> > +libvlc_media_get_type
> >  libvlc_media_get_user_data
> >  libvlc_media_get_tracks_info
> >  libvlc_media_is_parsed
> > diff --git a/lib/media.c b/lib/media.c
> > index 9ccfb25..1d01c11 100644
> > --- a/lib/media.c
> > +++ b/lib/media.c
> > @@ -999,3 +999,35 @@ void libvlc_media_tracks_release(
> > libvlc_media_track_t **p_tracks, unsigned i_co
> >      }
> >      free( p_tracks );
> >  }
> > +
> > 
> > +/**************************************************************************
> > + * Get the media type of the media descriptor object
> > + 
> > **************************************************************************/
> > +libvlc_media_type_t libvlc_media_get_type( libvlc_media_t *p_md )
> 
> Not very future proof ABI-wise...

Ah, Why ? Maybe it is better to return an int for error/success, and set
libvlc_media_type_t in argument.

> 
> > +{
> > +    assert( p_md );
> > +
> > +    int i_type;
> > +    input_item_t *p_input_item = p_md->p_input_item;
> > +
> > +    vlc_mutex_lock( &p_input_item->lock );
> > +    i_type = p_md->p_input_item->i_type;
> > +    vlc_mutex_unlock( &p_input_item->lock );
> 
> Can the type of an item even change? If it can't, there is no need to 
> lock, and the item should be const-qualified. And if it can, then this 
> does not really work.

i_type is set when we create the item, but there is one place that
change the type to a ITEM_TYPE_NODE, in ChangeToNode from
src/playlist/item.c.
Maybe I should replace ITEM_TYPE_NODE with a new variable in item,
b_node. That way, I will be able to set i_type const. Furthermore, it's
more coherent since a file, a playlist or directory can be a node too.


> > +
> > +    switch( i_type )
> > +    {
> > +    case ITEM_TYPE_FILE:
> > +        return libvlc_media_type_file;
> > +    case ITEM_TYPE_NODE:
> > +    case ITEM_TYPE_DIRECTORY:
> > +        return libvlc_media_type_directory;
> > +    case ITEM_TYPE_DISC:
> > +        return libvlc_media_type_disc;
> > +    case ITEM_TYPE_STREAM:
> > +        return libvlc_media_type_stream;
> > +    case ITEM_TYPE_PLAYLIST:
> > +        return libvlc_media_type_playlist;
> > +    default:
> > +        return libvlc_media_type_unknown;
> > +    }
> > +}
> 
> -- 
> Rémi Denis-Courmont
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel