[vlc-devel] [PATCH] libvlc: expand media player API to retrieve full information about available chapter of a given title
Felix Paul Kühne
fkuehne at videolan.org
Sun Jun 14 16:27:00 CEST 2015
Hello Rémi,
The patch in question was merged on June 9 after 5 days without feedback. I’ll make the changes you requested in a subsequent commit this week.
> On 12 Jun 2015, at 21:29, Rémi Denis-Courmont <remi at remlab.net> wrote:
>
> Summary description is too long.
Sorry, I can’t fix that one anymore.
> Le jeudi 04 juin 2015, 14:08:38 Felix Paul Kühne a écrit :
>> ---
>> include/vlc/libvlc_media_player.h | 40 ++++++++++++++++++++
>> include/vlc_input.h | 3 ++
>> lib/libvlc.sym | 2 +
>> lib/media_player.c | 78
>> +++++++++++++++++++++++++++++++++++++++ src/input/control.c |
>> 44 ++++++++++++++++++++++
>> 5 files changed, 167 insertions(+)
>>
>> diff --git a/include/vlc/libvlc_media_player.h
>> b/include/vlc/libvlc_media_player.h index 8bb7c29..48ce461 100644
>> --- a/include/vlc/libvlc_media_player.h
>> +++ b/include/vlc/libvlc_media_player.h
>> @@ -73,6 +73,17 @@ typedef struct libvlc_title_description_t
>> } libvlc_title_description_t;
>>
>> /**
>> + * Description for chapters.
>> + * It contains information about time set as well as
>> + * name (description string).
>
> This is incomplete documentation (unit?), and documented the fields in Doxygen
> would be better.
The unit is correctly documented in the pushed variant. I’ll doxygenise the fields.
>> + */
>> +typedef struct libvlc_chapter_description_t
>> +{
>> + int64_t i_time_offset;
>> + char *psz_name;
>> +} libvlc_chapter_description_t;
>> +
>> +/**
>> * Description for audio output. It contains
>> * name, description and pointer to next record.
>> */
>> @@ -1170,6 +1181,35 @@ LIBVLC_API
>> unsigned i_count );
>>
>> /**
>> + * Get the full description of available chapters
>> + *
>> + * \version LibVLC 3.0.0 and later.
>> + *
>> + * \param p_mi the media player
>> + * \param index of the title to query for chapters
>> + * \param address to store an allocated array of chapter descriptions
>> + * descriptions (must be freed with
>> libvlc_chapter_descriptions_release
>
> Missing () for Doxygen.
This is fixed in the pushed variant.
>> +
>> + /* fill array */
>> + for( int i = 0; i < ci_chapter_count; i++)
>> + {
>> + libvlc_chapter_description_t *p_chapter = calloc( 1,
>> sizeof(*p_chapter) );
>
> Why calloc?
Mainly to match similar implementations in the same file. I can switch to malloc if you prefer.
>> + if( unlikely(p_chapter == NULL) )
>> + {
>> + libvlc_chapter_descriptions_release( *pp_chapters,
>> ci_chapter_count );
>> + free( p_chapter );
>
> ??
ehm, will fix :D
>> + case INPUT_GET_SEEKPOINTS:
>> + {
>> + seekpoint_t ***array = (seekpoint_t ***)va_arg( args,
>> seekpoint_t *** );
>> + int *pi_title_to_fetch = (int *) va_arg(
>> args, int * );
>
> Useless casts.
Idem.
>> + *array = calloc( p_title->i_seekpoint, sizeof(**array) );
>> + if( !array )
>
> Wrong and missing unlikely().
Idem.
Thanks for the thorough review!
Cheers,
Felix
More information about the vlc-devel
mailing list