[vlc-devel] [PATCH 02/10] MED Support - Core controls and definitons

[Roiy Shpaner]

Hey,

Thanks for the review.

> -----Original Message-----
> From: vlc-devel [mailto:vlc-devel-bounces at videolan.org] On Behalf Of
> Francois Cartegnie
> Sent: April-06-14 2:19 AM
> To: Mailing list for VLC media player developers
> Subject: Re: [vlc-devel] [PATCH 02/10] MED Support - Core controls and
> definitons
> 
> Le 04/04/2014 20:07, Roiy Shpaner a écrit :
> 
> Just about code style.
> 
> > +    int64_t i_time_duration;
> > +    char    *psz_layerName;
> > +    int     eventNum;
> > +    char    *psz_text;
> > +    int     i_layer_id;
> 
> missing code convention for int.

Ok, fixed.
> 
> > +static inline void vlc_seeksegment_Clear( seeksegment_t *segment ) {
> > +    if( !segment ) return;
> 
> p_segment
> many many
> 

For the sake of consistency it seems better to keep it in the same format as
the existing seekpoint.

> > +
> > +    free( segment->psz_layerName );
> > +    free( segment->psz_text );
> > +
> > +    segment->psz_layerName = NULL;
> > +    segment->psz_text = NULL;
> > +}
> 
> FREENULL macro
> 

I agree with Rémi, I think this is preferable over the macro.

> > +static inline seeksegment_t *vlc_seeksegment_Duplicate( const
> > +seeksegment_t *src ) {
> > +    seeksegment_t *segment = vlc_seeksegment_New();
> > +    if( src->psz_layerName ) segment->psz_layerName = strdup( src-
> >psz_layerName );
> > +    if( src->psz_text ) segment->psz_text = strdup( src->psz_text );
> > +    segment->i_time_offset = src->i_time_offset;
> > +    segment->i_byte_offset = src->i_byte_offset;
> > +    segment->i_time_duration = src->i_time_duration;
> > +    segment->i_byte_duration = src->i_byte_duration;
> > +    segment->eventNum = src->eventNum;
> > +    segment->i_layer_id = src->i_layer_id;
> > +    return segment;
> > +}
> 
> or *new = *src + new string pointers

Again, this is more clear about the variables being set, and is consistent
with seekpoint.

> 
> >  static void UpdateBookmarksOption( input_thread_t * ); @@ -52,7 +55,6
> > @@ int input_Control( input_thread_t *p_input, int i_query, ...  )  {
> >      va_list args;
> >      int     i_result;
> > -
> >      va_start( args, i_query );
> >      i_result = input_vaControl( p_input, i_query, args );
> >      va_end( args );
> > @@ -66,14 +68,17 @@ int input_vaControl( input_thread_t *p_input, int
> i_query, va_list args )
> >      int i_bkmk = 0;
> >      int *pi_bkmk;
> >
> > -    int i_int, *pi_int;
> > +    seeksegment_t *p_segment, ***p_segments;
> 
> *** == ppp. Notation again ! many places.

Ok, fixed.
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel