[vlc-devel] [vlc-commits] demux: ttml: replace timings on output

[Filip Roséen]

Hi Francois,

On 2017-01-12 15:51, Francois Cartegnie wrote:

> vlc | branch: master | Francois Cartegnie <fcvlcdev at free.fr> |
> Thu Jan 12 15:46:36 2017 +0100| [d9d6893b17c83b7d92c57a3d1ed966023f74a8fb] |
> committer: Francois Cartegnie
> 
> demux: ttml: replace timings on output
> 
> timings must be relative to document, which is no
> longer true after splitting document.
> 
> > http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=d9d6893b17c83b7d92c57a3d1ed966023f74a8fb
> ---
> 
>  modules/demux/ttml.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 51 insertions(+), 4 deletions(-)
> 
> diff --git a/modules/demux/ttml.c b/modules/demux/ttml.c
> index d887f02..32a3090 100644
> --- a/modules/demux/ttml.c
> +++ b/modules/demux/ttml.c
> @@ -66,7 +66,27 @@ struct demux_sys_t
>      } times;
>  };
>  
> -static void tt_node_AttributesToText( struct vlc_memstream *p_stream, const tt_node_t* p_node )
> +static char *tt_genTiming( int64_t i_time )
> +{
> +    if( i_time < 0 )
> +        i_time = 0;
> +    char *psz;
> +    unsigned h, m, s, f;
> +    f = (i_time % CLOCK_FREQ) / 10000;
> +    i_time /= CLOCK_FREQ;

Why is `CLOCK_FREQ` used above?

> +    h = i_time / 3600;
> +    m = (i_time - h) / 3600;
> +    s = (i_time - h - m);

The above looks very very odd. Given that it looks like you are trying
to extract hours, minutes, and seconds, something like the below would
be correct:

    h = i_time / 3600;
    m = i_time % 3600 / 60;
    s = i_time % 60;

It is also worth noting that I personally feel that readability is
harmed when variable declarations are separated from their
initialization for no apparent reason.

> +    if( asprintf( &psz, "%2.2u:%2.2u:%2.2u.%2.2u",
> +                        h, m, s, f ) < 0 )
> +        psz = NULL;
> +
> +    return psz;
> +}
> +
> +static void tt_node_AttributesToText( struct vlc_memstream *p_stream, const tt_node_t* p_node,
> +                                      int64_t i_splicetime )
>  {
>      const vlc_dictionary_t* p_attr_dict = &p_node->attr_dict;
>      for( int i = 0; i < p_attr_dict->i_size; ++i )
> @@ -74,9 +94,36 @@ static void tt_node_AttributesToText( struct vlc_memstream *p_stream, const tt_n
>          for ( vlc_dictionary_entry_t* p_entry = p_attr_dict->p_entries[i];
>                                        p_entry != NULL; p_entry = p_entry->p_next )
>          {
> +            const char *psz_value = NULL;
> +            char *psz_alloc = NULL;
> +
> +            if( !strcmp(p_entry->psz_key, "begin") )
> +            {
> +                if( p_node->timings.i_begin != -1 )
> +                    psz_value = psz_alloc = tt_genTiming( p_node->timings.i_begin - i_splicetime );
> +            }
> +            else if( !strcmp(p_entry->psz_key, "end") )
> +            {
> +                if( p_node->timings.i_end != -1 )
> +                    psz_value = psz_alloc = tt_genTiming( p_node->timings.i_end - i_splicetime );
> +            }
> +            else if( !strcmp(p_entry->psz_key, "dur") )
> +            {
> +                /* remove */
> +                continue;
> +            }

For all cases where `strcmp` is used above, `strcasecmp` might make
more sense (given that the case of the attributes are not strictly
limited to lower-case, afaik)?

> +            else
> +            {
> +                psz_value = (char const*)p_entry->p_value;
> +            }
> +
> +            if( psz_value == NULL )
> +                continue;
> +
>              vlc_memstream_printf( p_stream, " %s=\"%s\"",
> -                                  p_entry->psz_key,
> -                                  (char const*)p_entry->p_value );
> +                                  p_entry->psz_key, psz_value );
> +
> +            free( psz_alloc );
>          }
>      }
>  }
> @@ -95,7 +142,7 @@ static void tt_node_ToText( struct vlc_memstream *p_stream, const tt_basenode_t
>          vlc_memstream_putc( p_stream, '<' );
>          vlc_memstream_puts( p_stream, p_node->psz_node_name );
>  
> -        tt_node_AttributesToText( p_stream, p_node );
> +        tt_node_AttributesToText( p_stream, p_node, i_playbacktime );
>  
>          if( tt_node_HasChild( p_node ) )
>          {
> 
> _______________________________________________
> vlc-commits mailing list
> vlc-commits at videolan.org
> https://mailman.videolan.org/listinfo/vlc-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20170112/478cd508/attachment.html>