[vlc-devel] [PATCH] libvlc: Add a new more extensible struct libvlc_media_track_t

[Martin Storsjö]

On Tue, 12 Feb 2013, Martin Storsjö wrote:

> On Mon, 11 Feb 2013, Rémi Denis-Courmont wrote:
>
>>> diff --git a/lib/media.c b/lib/media.c
>>> index 600e10a..e96caae 100644
>>> --- a/lib/media.c
>>> +++ b/lib/media.c
>>> @@ -734,3 +734,105 @@ libvlc_media_get_tracks_info( libvlc_media_t *p_md,
>>> libvlc_media_track_info_t ** vlc_mutex_unlock( &p_input_item->lock );
>>>      return i_es;
>>>  }
>>> +
>>> +int
>>> +libvlc_media_get_tracks( libvlc_media_t *p_md, libvlc_media_track_t ***
>>> pp_es ) +{
>>> +    assert( p_md );
>>> +
>>> +    input_item_t *p_input_item = p_md->p_input_item;
>>> +    vlc_mutex_lock( &p_input_item->lock );
>>> +
>>> +    const int i_es = p_input_item->i_es;
>>> +    *pp_es = (i_es > 0) ? calloc( i_es, sizeof(**pp_es) ) : NULL;
>> 
>> Why calloc()? This does not need to be initialized twice. Same afterward.
>
> Only for the case of some of the later allocations fail, to allow calling the 
> normal release function in that case.
>
>>> +
>>> +    if( !*pp_es ) /* no ES, or OOM */
>>> +    {
>>> +        vlc_mutex_unlock( &p_input_item->lock );
>>> +        return 0;
>>> +    }
>>> +
>>> +    /* Fill array */
>>> +    for( int i = 0; i < i_es; i++ )
>>> +    {
>>> +        libvlc_media_track_t *p_mes = calloc( 1, sizeof(*p_mes) );
>>> +        if ( p_mes )
>>> +        {
>>> +            p_mes->audio = calloc( 1, __MAX(__MAX(sizeof(*p_mes->audio),
>>> +                                                  sizeof(*p_mes->video)),
>>> +
>>> sizeof(*p_mes->subtitle)) ); +        }
>>> +        if ( !p_mes || !p_mes->audio )
>>> +        {
>>> +            libvlc_media_tracks_release( *pp_es, i_es );
>> 
>> (except for this nearly impossible error.)
>
> Yes, it's pretty unlikely. I think I could safely switch the other two to 
> malloc at least (I used calloc to have to think less about which ones 
> actually are needed), while still keeping the toplevel array initialized with 
> calloc and keeping the error checking and cleanup.
>
> Or would you prefer me to remove the error checking here completely, 
> switching all the allocations to malloc?

Hmm, the libvlc_media_track_t needs to be initialized as well, otherwise 
freeing e.g. psz_language or psz_description could use uninitialized data, 
if only the media-specific struct allocation failed. But that one can use 
malloc at least.

// Martin