[vlc-devel] Media library Include file and Core
Srikanth Raju
srikiraju at gmail.com
Sun Jun 27 16:45:28 CEST 2010
Hello,
On Sun, Jun 27, 2010 at 12:37 AM, Laurent Aimar <fenrir at elivagar.org> wrote:
> Hi,
>
> On Mon, Jun 21, 2010 at 11:23:35PM +0530, Srikanth Raju wrote:
> > Hello,
> > I have attached two patches. I will merge these into master by Wednesday
> if
> > nobody has any problems with them. These are the include files and some
> minor
> > changes to the core.
> It seems that I am too late but here is a first pass (I probably didn't
> see all
> issues).
>
>
>
> > +typedef struct ml_gc_object_t
> > +{
> > + vlc_spinlock_t spin;
> > + bool pool;
> > + uintptr_t refs;
> > + void (*pf_destructor) (struct ml_gc_object_t *);
> > +} ml_gc_object_t;
> > +
> > +#define ML_GC_MEMBERS ml_gc_object_t ml_gc_data;
> Why gc_object_t isn't enough?
>
> I want to be able to ensure that only one of each ml_media_t exists per URI
at runtime across all threads, so that we don't have to keep querying the
database to keep it updated across the threads. The solution for this was a
runtime pool of media objects, however there are cases where the ml_media_t
object will not be in the database(such as when items aren't really stored
in the database), hence we need an additional pool variable.
> > +/** Main structure of the media library. VLC object. */
> > +struct media_library_t
> > +{
> > + VLC_COMMON_MEMBERS
> > +
> > + module_t *p_module; /**< the media library module */
> > + media_library_sys_t *p_sys; /**< internal struture */
> > +
> > + /** Member functions */
> > + struct
> > + {
> > + /**< Search in the database */
> > + int ( * pf_Find ) ( media_library_t *p_media_library,
> > + vlc_array_t *p_result_array,
> > + va_list args );
> > +
> > + /**< Search in the database using an array of arguments */
> > + int ( * pf_FindAdv ) ( media_library_t *p_media_library,
> > + vlc_array_t *p_result_array,
> > + ml_select_e selected_type,
> > + const char *psz_lvalue,
> > + ml_ftree_t *tree );
> > +
> > + /**< Update the database using an array of arguments */
> > + int ( * pf_Update ) ( media_library_t *p_media_library,
> > + ml_select_e selected_type,
> > + const char *psz_lvalue,
> > + ml_ftree_t *where,
> > + vlc_array_t *changes );
> > +
> > + /**< Delete many medias in the database */
> > + int ( * pf_Delete ) ( media_library_t *p_media_library,
> > + vlc_array_t *p_array );
> > +
> > + /**< Control the media library */
> > + int ( * pf_Control ) ( media_library_t *p_media_library,
> > + int i_query, va_list args );
> > +
> > + /**< Create associated input item */
> > + input_item_t* ( * pf_InputItemFromMedia ) (
> > + media_library_t *p_media_library, int i_media );
> > +
> > + /**< Get a media */
> > + ml_media_t* ( * pf_GetMedia ) (
> > + media_library_t *p_media_library, int i_media,
> > + ml_select_e select, bool reload );
> > + } functions;
> > +};
> Why is media_library_t a module? Do you plan to have multiple
> implementations of it?
>
Would you suggest that it should be part of the core? There is a lot of code
that is incoming, around 7000 lines, and we don't want to make the core
heavy.
It would also be nicer to not use camel case for function pointers (as
> we do for every other module in vlc).
>
Will fix.
Why pf_Find and pf_FindAdv? Isn't pf_Find a special case of pf_FindAdv? If
> so,
> a wrapper could reuse it instead of having the module defines 2 functions.
>
>
Well yes. Hmm, again, at the expense of making the include larger?
> + * @brief Structure to describe a media
> > + *
> > + * This is the main structure holding the meta data in ML.
> > + * @see b_sparse indicates whether the media struct has valid values
> > + * in its Extra fields. Otherwise, it must be loaded with the API
> > + * function.
> > + * @see i_id indicates whether this struct is saved in the ML if i_id >
> 0
> > + * Otherwise, it can be added to the database
> > + */
> > +struct ml_media_t
> > +{
> > + ML_GC_MEMBERS
> > + vlc_mutex_t lock; /**< Mutex for multithreaded
> access */
> > + bool b_sparse; /**< Specifies if media is
> loaded fully */
> > + ml_type_e i_type; /**< Type of the media
> (ml_type_e) */
> Looking at ml_type_e, it seems intended to be used to store a 'or' of
> multiple values. If so, it is illegal to use ml_type_e.
>
Ah, I suppose certain compilers don't like enum memory taking up multiple
values?
> > +/**
> > + * @brief Create a Media Library VLC object.
> > + * @param p_this Parent to attach the ML object to.
> > + * @param psz_name Name for the module
> > + * @return The ML object.
> > + */
> > +VLC_EXPORT( media_library_t*, __ml_Create, ( vlc_object_t *p_this, char*
> psz_name ) );
> > +
> > +/**
> > + * @brief Destructor for the Media library singleton
> > + * @param p_this Parent the ML object is attached to
> > + */
> > +VLC_EXPORT( void, __ml_Destroy, ( vlc_object_t* p_this ) );
> Do not add more functions with __ prefix.
> You can easily do (for example):
>
> VLC_EXPORT( void, ml_Release, ( vlc_object_t* p_this ) );
> #define ml_Release(a) ml_Release( VLC_OBJECT(a))
>
> and then simply do
>
> void (ml_Release)( vlc_object_t* p_this )
> {
> code;
> }
>
I can't see the change, except for naming, which I will fix.
> > +/**
> > + * @brief Checks if a given find tree has leaf nodes
> > + * @param Find tree
> > + * @return Number of leaf nodes
> > + */
> > +static inline int ml_FtreeHasOp( ml_ftree_t* tree )
> > +{
> > + if( tree == NULL )
> > + return 0;
> > + if( tree->criteria > 0 && tree->op == ML_OP_NONE )
> > + return 1;
> > + else
> > + return ml_FtreeHasOp( tree->left ) + ml_FtreeHasOp( tree->right
> );
> > +}
>
> It seems to me that a lot of theses functions have no need to be defines
> as
> inline functions, and it would be way better to move them out of the .h
>
>
Yes. They can be moved to the core( too many symbols in core ) or into the
module ( too many module functions ).
In general, this is the problem, where to place functions. What would be a
good principle of where to move a particular function?
> > +/**
> > + * @brief Connect up a find tree
> > + * @param op operator to connect with
> > + * If op = ML_OP_NONE, then you are connecting to a tree consisting of
> > + * only SPECIAL nodes.
> > + * If op = ML_OP_NOT, then right MUST be NULL
> > + * op must not be ML_OP_SPECIAL, @see __ml_FtreeSpec
> > + * @param left part of the tree
> > + * @param right part of the tree
> > + * @return Pointer to new tree
> > + * @note Use the helpers!
> > + */
> > +VLC_EXPORT( ml_ftree_t*, ml_OpConnectChilds, ( ml_op_e op, ml_ftree_t*
> left,
> > + ml_ftree_t* right ) );
> > +
> > +/**
> > + * @brief Attaches a special node to a tree
> > + * @param tree Tree to attach special node to
> > + * @param crit Criteria may be SORT_ASC, SORT_DESC, LIMIT or DISTINCT
> > + * @param limit Limit used if LIMIT criteria used
> > + * @param Sort string used if SORT criteria is used
> > + * @return Pointer to new tree
> > + * @note Use the helpers
> > + */
> > +VLC_EXPORT( ml_ftree_t*, __ml_FtreeSpec, ( ml_ftree_t* tree,
> > + ml_select_e crit,
> > + int limit,
> > + char* sort ) );
> > +
> > +/**
> > + * @brief This function gives quick sequential adding capability
> > + * @param left Tree to add to. This may be NULL
> > + * @param right Tree to append. May not be NULL
> > + * @return Pointer to new tree.*/
> > +static inline ml_ftree_t* ml_FtreeFastAnd( ml_ftree_t* left,
> > + ml_ftree_t* right )
> > +{
> > + if( ml_FtreeHasOp( left ) == 0 )
> > + {
> > + return ml_OpConnectChilds( ML_OP_NONE, left, right );
> > + }
> > + else
> > + {
> > + return ml_OpConnectChilds( ML_OP_AND, left, right );
> > + }
> > +}
> > +#define ml_FtreeAnd( left, right ) ml_OpConnectChilds( ML_OP_AND, left,
> right )
> > +#define ml_FtreeOr( left, right ) ml_OpConnectChilds( ML_OP_OR, left,
> right )
> > +#define ml_FtreeNot( left ) ml_OpConnectChilds( ML_OP_NOT, left,
> NULL )
> > +
> > +#define ml_FtreeSpecAsc( tree, str ) __ml_FtreeSpec( tree,
> ML_SORT_ASC, 0, str )
> > +#define ml_FtreeSpecDesc( tree, str ) __ml_FtreeSpec( tree,
> ML_SORT_DESC, 0, str )
> > +#define ml_FtreeSpecLimit( tree, limit ) __ml_FtreeSpec( tree,
> ML_LIMIT, limit, NULL )
> > +#define ml_FtreeSpecDistinct( tree ) __ml_FtreeSpec( tree,
> ML_DISTINCT, 0, NULL )
> > +
> > +
> >
> +/*****************************************************************************
> > + * ML Core Functions
> > +
> *****************************************************************************/
> > +
> > +/**
> > + * @brief Create input item from media
> > + * @param p_media_library This ML instance.
> > + * @param i_media_id ID of the media to use to create an input_item.
> > + * @return The media item.
> > + */
> > +static inline input_item_t* ml_CreateInputItem(
> > + media_library_t *p_media_library, int i_media_id )
> > +{
> > + return p_media_library->functions.pf_InputItemFromMedia(
> p_media_library,
> > + i_media_id
> );
> > +}
> > +
> > +/**
> > + * @brief Search in the database according some criterias
> > + *
> > + * @param p_media_library the media library object
> > + * @param result a pointer to a result array
> > + * @param ... parameters to select the data
> > + * @return VLC_SUCCESS or an error
> > + */
> > +static inline int __ml_Find( media_library_t *p_media_library,
> > + vlc_array_t *p_result_array, ... )
> > +{
> > + va_list args;
> > + int returned;
> > +
> > + va_start( args, p_result_array );
> > + returned = p_media_library->functions.pf_Find( p_media_library,
> > + p_result_array, args
> );
> > + va_end( args );
> > +
> > + return returned;
> > +}
> > +
> > +
> > +/**
> > + * @brief Search in the database according some criterias (threaded)
> > + * @param p_media_library the media library object
> > + * @param result_array a pointer to a result array
> > + * @param result_type type of data to retrieve
> > + * @param psz_lvalue This should contain any necessary lvalue/key
> > + * for the given result_type. Used for ML_PEOPLE. Otherwise NULL
> > + * @param args parameters to select the data
> > + * @return VLC_SUCCESS or an error
> > + */
> > +static inline int ml_FindAdv( media_library_t *p_media_library,
> > + vlc_array_t *p_result_array,
> > + ml_select_e result_type,
> > + char* psz_lvalue,
> > + ml_ftree_t *tree )
> > +{
> > + return p_media_library->functions.pf_FindAdv( p_media_library,
> > + p_result_array,
> > + result_type,
> > + psz_lvalue,
> > + tree );
> > +}
> > +
> > +
> > +/**
> > + * @brief Find a value in the ML database, fill p_result with it.
> > + * @param p_media_library Media library object
> > + * @param p_result Object to put result into
> > + * @param Args [ SelectType [ PersonType ] Value ] ... ML_END
> > + * @note Do not use this function directly.
> > + */
> > +static inline int __ml_GetValue( media_library_t *p_media_library,
> > + ml_result_t *p_result,
> > + va_list args )
> > +{
> > + vlc_array_t *p_result_array = vlc_array_new();
> > + int i_ret = p_media_library->functions.pf_Find( p_media_library,
> > + p_result_array,
> > + args );
> > + if( i_ret != VLC_SUCCESS )
> > + goto exit;
> > + if( vlc_array_count( p_result_array ) > 0 )
> > + memcpy( p_result,
> > + ( ml_result_t* ) vlc_array_item_at_index(
> p_result_array, 0 ),
> > + sizeof( ml_result_t) );
> > + else
> > + i_ret = VLC_EGENERIC;
> > +
> > +exit:
> > + /* Note: Do not free the results, because of memcpy */
> > + vlc_array_destroy( p_result_array );
> > + return i_ret;
> > +}
> > +
> > +/**
> > + * @brief Search an INTEGER in the database
> > + * This uses a Query but returns only one integer (>0), or an error
> code.
> > + *
> > + * @param p_media_library the media library object
> > + * @param va_args parameters to select the data
> > + * @return Found INTEGER >= 0 or an error
> > + */
> > +#define ml_GetInt( ml, ... ) __ml_GetInt( ml, __VA_ARGS__, ML_LIMIT, 1,
> ML_END )
> > +static inline int __ml_GetInt( media_library_t *p_media_library, ... )
> > +{
> > + va_list args;
> > + va_start( args, p_media_library );
> > + ml_result_t result;
> > + int i_ret = __ml_GetValue( p_media_library, &result, args );
> > + va_end( args );
> > + if( i_ret != VLC_SUCCESS )
> > + return i_ret;
> > + else
> > + return result.value.i;
> > +}
> > +
> > +
> > +/**
> > + * @brief Search a string (VARCHAR) in the database
> > + * This uses a Query but returns only one integer (>0), or an error
> code.
> > + *
> > + * @param p_media_library the media library object
> > + * @param va_args parameters to select the data
> > + * @return Found string, or NULL if not found or in case of error
> > + */
> > +#define ml_FindPsz( ml, ... ) __ml_GetPsz( ml, __VA_ARGS__, ML_LIMIT, 1,
> ML_END )
> > +static inline char* __ml_GetPsz( media_library_t *p_media_library, ... )
> > +{
> > + va_list args;
> > + va_start( args, p_media_library );
> > + ml_result_t result;
> > + int i_ret = __ml_GetValue( p_media_library, &result, args );
> > + va_end( args );
> > + if( i_ret != VLC_SUCCESS )
> > + return NULL;
> > + else
> > + return result.value.psz; // no need to duplicate
> > +}
> > +
> > +/**
> > + * @brief Generic update in Media Library database
> > + *
> > + * @param p_media_library the media library object
> > + * @param selected_type the type of the element we're selecting
> > + * @param where list of ids/uris to be changed
> > + * @param changes list of changes to make in the entries
> > + * @return VLC_SUCCESS or VLC_EGENERIC
> > + */
> > +static inline int ml_Update( media_library_t *p_media_library,
> > + ml_select_e selected_type,
> > + const char* psz_lvalue,
> > + ml_ftree_t *where,
> > + vlc_array_t *changes )
> > +{
> > + return p_media_library->functions.pf_Update( p_media_library,
> > + selected_type,
> psz_lvalue,
> > + where, changes );
> > +}
> > +
> > +/**
> > + * @brief Update a given table
> > + * @param p_media_library The media library object
> > + * @param selected_type The table to update
> > + * @param psz_lvalue The role of the person if selected_type = ML_PEOPLE
> > + * @param id The id of the row to update
> > + * @param ... The update data. [SelectType [RoleType] Value]
> > + */
> > +#define ml_UpdateSimple( ml, sel, lval, id, ... ) \
> > + __ml_UpdateSimple( ml, sel, lval, id, __VA_ARGS__, ML_END )
> > +VLC_EXPORT( int, __ml_UpdateSimple, ( media_library_t *p_media_library,
> > + ml_select_e selected_type,
> > + const char* psz_lvalue,
> > + int id, ... ) );
> > +
> > +/**
> > + * @brief Generic DELETE function
> > + * Delete a media and all its references which don't point
> > + * to anything else.
> > + *
> > + * @param p_media_library This media_library_t object
> > + * @param id the id of the media to delete
> > + * @return VLC_SUCCESS or VLC_EGENERIC
> > + */
> > +static inline int
> > +ml_DeleteSimple( media_library_t *p_media_library, int id )
> > +{
> > + vlc_array_t* p_where = vlc_array_new();
> > + ml_element_t* p_find = (ml_element_t *) calloc( 1, sizeof(
> ml_element_t ) );
> > + p_find->criteria = ML_ID;
> > + p_find->value.i = id;
> > + vlc_array_append( p_where, p_find );
> > + int i_return = p_media_library->functions.pf_Delete(
> p_media_library,
> > + p_where );
> > + free( p_find );
> > + vlc_array_destroy( p_where );
> > + return i_return;
> > +}
> > +
> > +/**
> > + * @brief Delete many medias in the media library
> > + * @param p_media_library Media library object
> > + * @param p_array Array of ids to delete
> > + * @return VLC_SUCCESS or VLC_EGENERIC
> > + */
> > +static inline int
> > +ml_Delete( media_library_t *p_media_library, vlc_array_t* p_array )
> > +{
> > + return p_media_library->functions.pf_Delete( p_media_library,
> > + p_array );
> > +}
> > +
> > +
> >
> +/*****************************************************************************
> > + * ML Person Related Functions
> > +
> *****************************************************************************/
> > +
> > +/**
> > + * @brief Create and append a person object to the given list
> > + * @param pp_person pointer to person list. Set the address to null to
> create new list
> > + * @param i_role The role of the person
> > + * @param psz_name The name string. Will be strdup'd
> > + * @param i_id The id in the database
> > + * @note This function is NOT thread safe. Please lock any associated
> media
> > + */
> > +static inline int ml_CreateAppendPersonAdv( ml_person_t **pp_person,
> > + const char* psz_role, const char* psz_name, int i_id )
> > +{
> > + assert( i_id || ( psz_name && *psz_name && psz_role && *psz_role )
> );
> > + if( !pp_person )
> > + return VLC_EGENERIC;
> > + if( *pp_person != NULL )
> > + return ml_CreateAppendPersonAdv( &((**pp_person).p_next),
> > + psz_role, psz_name, i_id);
> > + *pp_person = ( ml_person_t * ) calloc( 1, sizeof( ml_person_t ) );
> > + (*pp_person)->psz_name = (psz_name && *psz_name) ? strdup( psz_name
> ): NULL;
> > + (*pp_person)->psz_role = (psz_role && *psz_role) ? strdup( psz_role
> ): NULL;
> > + (*pp_person)->i_id = i_id;
> > + (*pp_person)->p_next = NULL;
> > + return VLC_SUCCESS;
> > +}
> > +
> > +/**
> > + * @brief Create and append a person object to the given list
> > + * @param pp_person pointer to person list.
> > + * Set the address to NULL to create a new list
> > + * @param personfrom Person object to copy from
> > + * @note Ignores the next variable and copies only the variables.
> > + * Uses ml_CreateAppendPersonAdv
> > + * @note This function is NOT threadsafe
> > + */
> > +static inline int ml_CreateAppendPerson( ml_person_t **pp_person,
> > + ml_person_t *p_personfrom )
> > +{
> > + return ml_CreateAppendPersonAdv( pp_person,
> > + p_personfrom->psz_role,
> > + p_personfrom->psz_name,
> > + p_personfrom->i_id );
> > +}
> > +
> > +/**
> > + * @brief Copy one person list into another
> > + * @param a To list
> > + * @param b From list
> > + * @note On errors, you have to free any allocated persons yourself
> > + * @note This function is NOT threadsafe. Please ensure your medias are
> locked
> > + */
> > +static inline int ml_CopyPersons( ml_person_t** a, ml_person_t* b )
> > +{
> > + int i_ret;
> > + while( b )
> > + {
> > + i_ret = ml_CreateAppendPerson( a, b );
> > + if( i_ret != VLC_SUCCESS )
> > + return i_ret;
> > + b = b->p_next;
> > + }
> > + return VLC_SUCCESS;
> > +}
> > +
> > +
> > +/**
> > + * @brief Returns a person list of given type
> > + * @param p_ml The ML object
> > + * @param p_media The Media object
> > + * @param i_type The person type
> > + * @note This function is thread safe
> > + */
> > +VLC_EXPORT( ml_person_t*, ml_GetPersonsFromMedia, ( media_library_t*
> p_ml,
> > + ml_media_t* p_media,
> > + const char *psz_role
> ) );
> > +
> > +
> > +#define ml_GetAlbumArtistsFromMedia( a, b ) ml_GetPersonsFromMedia( a,
> b, ML_PERSON_ALBUM_ARTIST );
> > +#define ml_GetArtistsFromMedia( a, b ) ml_GetPersonsFromMedia( a,
> b, ML_PERSON_ARTIST );
> > +#define ml_GetEncodersFromMedia( a, b ) ml_GetPersonsFromMedia( a,
> b, ML_PERSON_ENCODER );
> > +#define ml_GetPublishersFromMedia( a, b ) ml_GetPersonsFromMedia( a,
> b, ML_PERSON_PUBLISHER );
> > +
> > +/**
> > + * @brief Delete a certain type of people from a media
> > + * @param p_media Media to delete from
> > + * @param i_type Type of person to delete
> > + * @note This function is threadsafe
> > + */
> > +VLC_EXPORT( void, ml_DeletePersonTypeFromMedia, ( ml_media_t* p_media,
> > + const char *psz_role )
> );
> > +
> > +
> > +/**
> > + * @brief Creates and adds the playlist based on a given find tree
> > + * @param p_ml Media library object
> > + * @param p_tree Find tree to create SELECT
> > + */
> > +
> > +VLC_EXPORT( void, ml_PlaySmartPlaylistBasedOn, ( media_library_t* p_ml,
> > + ml_ftree_t* p_tree ) );
> > +
> > +
> > +/**
> > + * Convenience Macros
> > + */
> > +
> > +/**
> > + * Get information using the *media* ID. This returns only 1
> information.
> > + * @note You have to free the string returned (if that's a string!).
> > + */
> > +#define ml_GetAlbumById( a, id ) ml_GetPsz( a, ML_ALBUM,
> ML_ID, id )
> > +#define ml_GetArtistById( a, id ) ml_GetPsz( a, ML_PEOPLE,
> ML_PERSON_ARTIST, ML_ID, id )
> > +#define ml_GetCoverUriById( a, id ) ml_GetPsz( a, ML_COVER,
> ML_ID, id )
> > +#define ml_GetEncoderById( a, id ) ml_GetPsz( a, ML_PEOPLE,
> ML_PERSON_ENCODER, ML_ID, id )
> > +#define ml_GetExtraById( a, id ) ml_GetPsz( a, ML_EXTRA,
> ML_ID, id )
> > +#define ml_GetGenreById( a, id ) ml_GetPsz( a, ML_GENRE,
> ML_ID, id )
> > +#define ml_GetOriginalTitleById( a, id ) ml_GetPsz( a,
> ML_ORIGINAL_TITLE, ML_ID, id )
> > +#define ml_GetPublisherById( a, id ) ml_GetPsz( a, ML_PEOPLE,
> ML_PERSON_PUBLISHER, ML_ID, id )
> > +#define ml_GetTitleById( a, id ) ml_GetPsz( a, ML_TITLE,
> ML_ID, id )
> > +#define ml_GetUriById( a, id ) ml_GetPsz( a, ML_URI, ML_ID,
> id )
> > +
> > +#define ml_GetAlbumIdById( a, id ) ml_GetInt( a, ML_ALBUM_ID,
> ML_ID, id )
> > +#define ml_GetArtistIdById( a, id ) ml_GetInt( a, ML_PEOPLE_ID,
> ML_PERSON_ARTIST, ML_ID, id )
> > +#define ml_GetDurationById( a, id ) ml_GetInt( a, ML_DURATION,
> ML_ID, id )
> > +#define ml_GetEncoderIdById( a, id ) ml_GetInt( a, ML_PEOPLE_ID,
> ML_PERSON_ENCODER, ML_ID, id )
> > +#define ml_GetLastPlayedById( a, id ) ml_GetInt( a,
> ML_LAST_PLAYED, ML_ID, id )
> > +#define ml_GetPlayedCountById( a, id ) ml_GetInt( a,
> ML_PLAYED_COUNT, ML_ID, id )
> > +#define ml_GetPublisherIdById( a, id ) ml_GetInt( a, ML_PEOPLE_ID,
> ML_PERSON_PUBLISHER, ML_ID, id )
> > +#define ml_GetScoreById( a, id ) ml_GetInt( a, ML_SCORE,
> ML_ID, id )
> > +#define ml_GetTrackNumberById( a, id ) ml_GetInt( a,
> ML_TRACK_NUMBER, ML_ID, id )
> > +#define ml_GetTypeById( a, id ) ml_GetInt( a, ML_TYPE,
> ML_ID, id )
> > +#define ml_GetYearById( a, id ) ml_GetInt( a, ML_YEAR,
> ML_ID, id )
> > +#define ml_GetVoteById( a, id ) ml_GetInt( a, ML_VOTE,
> ML_ID, id )
> > +
> > +/** Albums handling */
> > +#define ml_GetAlbumId( a, b ) ml_GetInt( a, ML_ALBUM_ID,
> ML_ALBUM, b )
> > +
> > +/** People handling */
> > +#define ml_GetArtistId( a, b ) ml_GetInt( a, ML_PERSON_ID,
> ML_PERSON_ARTIST, ML_PERSON, ML_PERSON_ARTIST, b )
> > +#define ml_GetEncoderId( a, b ) ml_GetInt( a, ML_PERSON_ID,
> ML_PERSON_ENCODER, ML_PERSON, ML_PERSON_ENCODER, b )
> > +#define ml_GetPublisherId( a, b ) ml_GetInt( a, ML_PERSON_ID,
> ML_PERSON_PUBLISHER, ML_PERSON, ML_PERSON_PUBLISHER, b )
> > +
> > +/** Counts handling */
> > +#define ml_GetMediaCount( a, ... ) __ml_GetInt( a,
> ML_COUNT_MEDIA, __VA_ARGS__, ML_END )
> > +#define ml_GetAlbumCount( a, ... ) __ml_GetInt( a,
> ML_COUNT_ALBUM, __VA_ARGS__, ML_END )
> > +#define ml_GetPeopleCount( a, ... ) __ml_GetInt( a,
> ML_COUNT_PEOPLE, __VA_ARGS__, ML_END )
> > +
> > +#define ml_Find( a, b, ... ) __ml_Find( a, b,
> __VA_ARGS__, ML_END )
> > +
> > +#define ml_FindAlbum( a, b, ... ) __ml_Find( a, b, ML_ALBUM,
> __VA_ARGS__, ML_END )
> > +#define ml_FindArtist( a, b, ... ) __ml_Find( a, b, ML_PERSON,
> ML_PERSON_ARTIST, __VA_ARGS__, ML_END )
> > +#define ml_FindEncoder( a, b, ... ) __ml_Find( a, b, ML_PERSON,
> ML_PERSON_ENCODER, __VA_ARGS__, ML_END )
> > +#define ml_FindGenre( a, b, ... ) __ml_Find( a, b, ML_GENRE,
> __VA_ARGS__, ML_END )
> > +#define ml_FindMedia( a, b, ... ) __ml_Find( a, b, ML_MEDIA,
> __VA_ARGS__, ML_END )
> > +#define ml_FindOriginalTitle( a, b, ... ) __ml_Find( a, b,
> ML_ORIGINAL_TITLE, __VA_ARGS__, ML_END )
> > +#define ml_FindPublisher( a, b, ... ) __ml_Find( a, b, ML_PERSON,
> ML_PERSON_PUBLISHER, __VA_ARGS__, ML_END )
> > +#define ml_FindTitle( a, b, ... ) __ml_Find( a, b, ML_TITLE,
> __VA_ARGS__, ML_END )
> > +#define ml_FindType( a, b, ... ) __ml_Find( a, b, ML_TYPE,
> __VA_ARGS__, ML_END )
> > +#define ml_FindUri( a, b, ... ) __ml_Find( a, b, ML_URI,
> __VA_ARGS__, ML_END )
> > +#define ml_FindYear( a, b, ... ) __ml_Find( a, b, ML_YEAR,
> __VA_ARGS__, ML_END )
> > +
> > +#define ml_FindAllAlbums( a, b ) ml_FindAlbum( a, b,
> ML_DISTINCT )
> > +#define ml_FindAllArtists( a, b ) ml_FindArtist( a, b,
> ML_DISTINCT )
> > +#define ml_FindAllGenres( a, b ) ml_FindGenre( a, b,
> ML_DISTINCT )
> > +#define ml_FindAllMedias( a, b ) ml_FindMedia( a, b,
> ML_DISTINCT )
> > +#define ml_FindAllOriginalTitles( a, b ) ml_FindOriginalTitle( a, b,
> ML_DISTINCT )
> > +#define ml_FindAllPublishers( a, b, ... ) ml_FindPublisher( a, b,
> ML_DISTINCT )
> > +#define ml_FindAllTitles( a, b ) ml_FindTitle( a, b,
> ML_DISTINCT )
> > +#define ml_FindAllTypes( a, b ) ml_FindType( a, b,
> ML_DISTINCT )
> > +#define ml_FindAllUris( a, b ) ml_FindUri( a, b,
> ML_DISTINCT )
> > +#define ml_FindAllYears( a, b ) ml_FindYear( a, b,
> ML_DISTINCT )
> > +
> > +#define ml_FindAlbumAdv( a, b, c ) ml_FindAdv( a, b, ML_ALBUM,
> NULL, c )
> > +#define ml_FindArtistAdv( a, b, c ) ml_FindAdv( a, b, ML_PERSON,
> ML_PERSON_ARTIST, c )
> > +#define ml_FindEncoderAdv( a, b, c ) ml_FindAdv( a, b, ML_PERSON,
> ML_PERSON_ENCODER, c )
> > +#define ml_FindGenreAdv( a, b, c ) ml_FindAdv( a, b, ML_GENRE,
> NULL, c )
> > +#define ml_FindMediaAdv( a, b, c ) ml_FindAdv( a, b, ML_MEDIA,
> NULL, c )
> > +#define ml_FindOriginalTitleAdv( a, b, c ) ml_FindAdv( a, b,
> ML_ORIGINAL_TITLE,NULL, c )
> > +#define ml_FindPublisherAdv( a, b, c ) ml_FindAdv( a, b,
> ML_PUBLISHER, ML_PERSON_PUBLISHER, c )
> > +#define ml_FindTitleAdv( a, b, c ) ml_FindAdv( a, b, ML_TITLE,
> NULL, c )
> > +#define ml_FindTypeAdv( a, b, c ) ml_FindAdv( a, b, ML_TYPE,
> NULL, c )
> > +#define ml_FindUriAdv( a, b, c ) ml_FindAdv( a, b, ML_URI,
> NULL, c )
> > +#define ml_FindYearAdv( a, b, c ) ml_FindAdv( a, b, ML_YEAR,
> NULL, c )
>
> Btw, are all thoses functions really needed by a ML users ? Aren't some of
> them private to the core ? If so, pleases move them there.
>
> No, all these functions are needed by users.
> Why are you using i_id ? With proper ref counting you should be able to
> not use it (playlist is a bad example, and it is plane to remove it).
>
>
The id is the primary key in the database. I'm not sure what "problem" you
have in mind when you are referring to the playlist. I suppose that the
playlist had a constantly varying structure and it would be difficult to
keep track of old stray ids. However, this isn't the case here, as the
database ensures that the primary key in unique. This is also a performance
thing, where it's much faster to search by index( i_id ), than by a direct
search. Could you emphasize what you are trying to avoid?
> +/**
> > + * @brief Create an instance of the media library
> > + * @param p_this Parent object
> > + * @param psz_name Name which is passed to module_need (not needed)
> > + * @return p_ml created and attached, module loaded. NULL if
> > + * not able to load
> > + */
> > +media_library_t *__ml_Create( vlc_object_t *p_this, char *psz_name )
> > +{
> > + media_library_t *p_ml = NULL;
> Useless assignation.
>
> +
> > + p_ml = ( media_library_t * ) vlc_custom_create(
> > + p_this, sizeof( media_library_t ),
> > + VLC_OBJECT_GENERIC, "media-library" );
> > + if( !p_ml )
> > + {
> > + msg_Err( p_this, "unable to create media library object" );
> > + return NULL;
> > + }
> > + vlc_object_attach( p_ml, p_this );
> > +
> > + p_ml->p_module = module_need( p_ml, "media-library", psz_name, false
> );
> > + if( !p_ml->p_module )
> > + {
> > + vlc_object_release( p_ml );
> > + msg_Err( p_this, "Media Library provider not found" );
> > + return NULL;
> > + }
> > +
> > + return p_ml;
> > +}
>
> > +/**
> > + * @brief Acquire a reference to the media library singleton
> > + * @param p_this Object that holds the reference
> > + * @return media_library_t The ml object. NULL if not compiled with
> > + * media library or if unable to load
> > + */
> > +media_library_t* __ml_Hold( vlc_object_t* p_this )
> > +{
> > + media_library_t* p_ml = NULL;
> Same (apply to other cases).
>
Will fix.
> > + p_ml = libvlc_priv (p_this->p_libvlc)->p_ml;
> > + assert( VLC_OBJECT( p_ml ) != p_this );
> > + if( p_ml == NULL &&
> > + var_GetBool( p_this->p_libvlc,
> "load-media-library-on-startup" ) == false )
> No == false or true or ...
>
> If the ML didn't load at startup, start it up now, when the user has asked
for it.
> > + {
> > + libvlc_priv (p_this->p_libvlc)->p_ml
> > + = __ml_Create( VLC_OBJECT( p_this->p_libvlc ), NULL );
> > + p_ml = libvlc_priv (p_this->p_libvlc)->p_ml;
> > + }
> I don't see how that can be thread safe.
>
> You're right. It's not. Will fix.
> + if( p_ml )
> > + vlc_object_hold( p_ml );
> > + return p_ml;
> > +}
> > +
> > +/**
> > + * @brief Release a reference to the media library singleton
> > + * @param p_this Object that holds the reference
> > + */
> > +void __ml_Release( vlc_object_t* p_this )
> > +{
> > + media_library_t* p_ml;
> > + p_ml = libvlc_priv (p_this->p_libvlc)->p_ml;
> > + if( p_ml == NULL )
> > + {
> > + msg_Warn( p_this->p_libvlc , "Spurious release ML called");
> > + return;
> > + }
> > + assert( VLC_OBJECT( p_ml ) != p_this );
> > + vlc_object_release( p_ml );
> > +}
> Wouldn't it be simpler and better to handle it as the playlist do ? ie
> a single ml_Get() function.
>
> Hmm yes. There is an added downside that we don't know if stray threads are
holding references they shouldn't be. But then again, all threads should
have joined by that time anyway. Will do.
>
> --
> fenrir
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> http://mailman.videolan.org/listinfo/vlc-devel
>
--
Regards,
Srikanth Raju
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20100627/9a594e47/attachment.html>
More information about the vlc-devel
mailing list