[vlc-devel] [Patch] Correctly fill out input_item's es info, and slim down input_item_es_t.

[Laurent Aimar]

Hi,

On Thu, Feb 25, 2010, Pierre d'Herbemont wrote:
>    Updated patch attached.
>    Pierre.

>  struct input_item_t
>  {
>      VLC_GC_MEMBERS
> @@ -73,6 +102,9 @@ struct input_item_t
>      int         i_es;                /**< Number of es format descriptions */
>      es_format_t **es;                /**< Es formats */
>  
> +    int i_tracks;               /**< Number of track info descriptions */
> +    input_item_track_t **tracks;/**< Tracks Info */
 Vertical align would look nicer.

>      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) ? malloc( i_es * sizeof(libvlc_media_track_info_t) ) : NULL;
> +    const int i_tracks = p_input_item->i_tracks;
> +    *pp_tracks = (i_tracks > 0) ? malloc( i_tracks * sizeof(libvlc_media_track_info_t) ) : NULL;
 Any reason why you don't want to use sizeof(*variable) ? But as I 
don't maintain libvlc, I don't really care.

> diff --git a/src/input/es_out.c b/src/input/es_out.c
> index 3062d8e..aa0ffbb 100644
> --- a/src/input/es_out.c
> +++ b/src/input/es_out.c
> @@ -45,6 +45,7 @@
>  #include "es_out.h"
>  #include "event.h"
>  #include "info.h"
> +#include "item.h"
>  
>  #include "../stream_output/stream_output.h"
>  
> @@ -2041,6 +2042,8 @@ static void EsOutDel( es_out_t *out, es_out_id_t *es )
>      if( es->p_pgrm == p_sys->p_pgrm )
>          EsOutESVarUpdate( out, es, true );
>  
> +    input_item_DeleteTrack( input_GetItem(p_sys->p_input), &es->fmt );
 I don't think you want it here otherwise you will end up with an empty
track list once the item is played.

> @@ -2223,7 +2226,7 @@ static int EsOutControlLocked( es_out_t *out, int i_query, va_list args )
>              }
>              return VLC_SUCCESS;
>          }
> - 
> +
 Cosmetic.

> @@ -99,6 +100,10 @@ static inline void input_item_Clean( input_item_t *p_i )
>      }
>      TAB_CLEAN( p_i->i_es, p_i->es );
>  
> +    for( i = 0; i < p_i->i_tracks; i++ )
> +        free( p_i->tracks[i] );
 I would really like an input_item_track_Delete instead of a direct free.

> +static input_item_track_t *input_item_track_NewFromESFormat(const es_format_t *fmt)
> +{
> +    input_item_track_t *ret = malloc(sizeof(*ret));
> +    ret->i_codec = fmt->i_codec;
> +    ret->i_id = fmt->i_id;
> +    ret->i_cat = fmt->i_cat;
> +
> +    ret->i_profile = fmt->i_profile;
> +    ret->i_level = fmt->i_level;
> +
> +    switch(fmt->i_cat)
> +    {
> +        case VIDEO_ES:
> +            ret->u.video.i_height = fmt->video.i_height;
> +            ret->u.video.i_width = fmt->video.i_width;
> +            break;
> +        case AUDIO_ES:
> +            ret->u.audio.i_channels = fmt->audio.i_channels;
> +            ret->u.audio.i_rate = fmt->audio.i_rate;
> +            break;
> +        default:
> +            break;
 Bad indentation level. Vertical align would also help.

> +/* Called by es_out when a new Elementary Stream is added or updated. */
> +void input_item_UpdateTrack(input_item_t *item, const es_format_t *fmt)
> +{
> +    input_item_track_t *track = input_item_track_NewFromESFormat(fmt);
Missing:
if( !track)
    return;

> +
> +    vlc_mutex_lock( &item->lock );
> +
> +    for( int i = 0; i < item->i_tracks; i++ )
> +    {
> +        if( item->tracks[i]->i_id == track->i_id )
> +        {
> +            /* We've found the right ES, replace it */
> +            free(item->tracks[i]);
input_item_track_Delete.

> +            item->tracks[i] = track;
> +            vlc_mutex_unlock( &item->lock );
> +            return;
> +        }
> +    }
> +
> +    /* ES not found, create a new one. */
> +    TAB_APPEND(item->i_tracks, item->tracks, track);
> +
> +    vlc_mutex_unlock( &item->lock );
> +}
> +
> +void input_item_DeleteTrack(input_item_t *item, const es_format_t *fmt)
> +{
> +    vlc_mutex_lock( &item->lock );
> +
> +    for( int i = 0; i < item->i_tracks; i++ )
> +    {
> +        if( item->tracks[i]->i_id == fmt->i_id )
> +        {
> +            /* We've found the right track, remove it */
> +            free(item->tracks[i]);
> +            TAB_REMOVE(item->i_tracks, item->tracks, item->tracks[i]);
> +            vlc_mutex_unlock( &item->lock );
> +            return;
> +        }
> +    }
> +
> +#ifndef NDEBUG
> +    // This should not happen, it means that someone removed a non existent track.
> +    abort();
> +#endif
> +
> +    vlc_mutex_unlock( &item->lock );
> +}
 I don't think this function is used once removed from EsOutDel.

Regards,

-- 
fenrir