[vlc-devel] [PATCH 01/19] ttml demux: add style inheritence support

[Francois Cartegnie]

Le 29/08/2016 à 23:00, Stanislas Plessia a écrit :

>  
> +static void FreeDictAttrValues( void* p_attr_value, void* p_obj )
> +{
> +    VLC_UNUSED( p_obj );
> +    free( p_attr_value );
> +}

Plural ?

> +static void ClearNode( node_t* p_node )
> +{
> +    free( p_node->psz_styleid );
> +    free( p_node->psz_node_name );
> +    free( p_node->psz_begin );
> +    free( p_node->psz_end );
> +    vlc_dictionary_clear( &p_node->attr_dict, FreeDictAttrValues, NULL );
> +    free( p_node );
> +}

If you make use of release functions, you need init ones as well.

> +static int MergeStyles(char** pp_dest, char* p_src)

inconsistency

> +static int MergeNodeWithParents( node_t* p_node )
> +{
> +    node_t *parent = p_node->p_parent;
> +    while( parent != NULL)
> +    {
> +        if( MergeAttrDict( &p_node->attr_dict, &parent->attr_dict ) != VLC_SUCCESS )
> +            return VLC_EGENERIC;

||

> +        if( MergeStyles( &p_node->psz_styleid, parent->psz_styleid ) != VLC_SUCCESS )
> +            return VLC_EGENERIC;
> +
> +        parent = parent->p_parent;
> +    }
> +    return VLC_SUCCESS;
> +}

> +
> +    if( !strcasecmp( p_node->psz_node_name, "br" ) || !strcasecmp( p_node->psz_node_name, "tt:br" ) )
> +        psz_text = Append( psz_text, "/>" );
> +    else
> +        psz_text = Append( psz_text, ">" );
> +
> +    return psz_text;
> +}

IMHO that part is dubious, as nodes should be closed after processing
childs.
Won't prevent unclosed dom with self closing tags or regular such as
empty paragraph.

> -        if ( i_type == XML_READER_STARTELEM && ( !strcasecmp( psz_name, "head" ) || !strcasecmp( psz_name, "tt:head" ) ) )
> +        if( i_type == XML_READER_STARTELEM && ( !strcasecmp( psz_node_name, "head" ) || !strcasecmp( psz_node_name, "tt:head" ) ) )
>          {

I hope for you someone has not the idea to bind the TTML namespace to
another prefix..



Francois