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

[Pierre d'Herbemont]

On Thu, Feb 25, 2010 at 10:03 PM, Laurent Aimar <fenrir at via.ecp.fr> wrote:

> 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.
>

*sigh*


> >      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.


Simply didn't see it.


> > 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.
>

That's what I was asking on IRC.


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

Well, trailing spaces, it's ok to have it in the patch I guess.

>
> > @@ -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.
>

okay. So that's what you meant the first time.


> > +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.


What level do you want? There are plenty of coding style... And we have
none.


>

> +/* 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.
>

It's not.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20100225/46192e8c/attachment.html>