[vlc-devel] [PATCH 11/11] demux/mkv: Reimplemented MkvTree and moved it to Util.hpp

[Denis Charmet]

On 2016-03-02 18:04, Filip Roséen wrote:
> `MkvTree` can be used all across the module, meaning that it better
> belongs in `Util.hpp` than with static linkage in 
> `matroska_segment_parse.cpp`.

I fail to see the point of that since its only purpose is to debug the 
parsing of elements... but the rest is nice.

> 
>  - A helper function `MkvTree_va` has been introduced so that one can
>    easily call the function from functions that accepts a variadic
>    number of arguments (using va_list).
> 
>  - The implementation has been optimized so that we do not need to
>    allocate storage for the format string on every invocation if it
>    would fit inside a fixed length buffer.
> 
>  - An error check has been added to signal if we run out of memory.
> ---
>  modules/demux/mkv/matroska_segment_parse.cpp | 25 -----------------
>  modules/demux/mkv/mkv.cpp                    |  1 -
>  modules/demux/mkv/util.cpp                   | 42 
> ++++++++++++++++++++++++++++
>  modules/demux/mkv/util.hpp                   |  4 +++
>  4 files changed, 46 insertions(+), 26 deletions(-)
> 
> diff --git a/modules/demux/mkv/matroska_segment_parse.cpp
> b/modules/demux/mkv/matroska_segment_parse.cpp
> index be1d967..bb590b5 100644
> --- a/modules/demux/mkv/matroska_segment_parse.cpp
> +++ b/modules/demux/mkv/matroska_segment_parse.cpp
> @@ -187,31 +187,6 @@ void matroska_segment_c::ParseSeekHead(
> KaxSeekHead *seekhead )
>      }
>  }
> 
> -
> -/**
> - * Helper function to print the mkv parse tree
> - */
> -static void MkvTree( demux_t & demuxer, int i_level, const char
> *psz_format, ... )
> -{
> -    va_list args;
> -    if( i_level > 9 )
> -    {
> -        msg_Err( &demuxer, "MKV tree is too deep" );
> -        return;
> -    }
> -    va_start( args, psz_format );
> -    static const char psz_foo[] = "|   |   |   |   |   |   |   |   |  
> |";
> -    char *psz_foo2 = (char*)malloc( i_level * 4 + 3 + strlen( 
> psz_format ) );
> -    strncpy( psz_foo2, psz_foo, 4 * i_level );
> -    psz_foo2[ 4 * i_level ] = '+';
> -    psz_foo2[ 4 * i_level + 1 ] = ' ';
> -    strcpy( &psz_foo2[ 4 * i_level + 2 ], psz_format );
> -    msg_GenericVa( &demuxer,VLC_MSG_DBG, psz_foo2, args );
> -    free( psz_foo2 );
> -    va_end( args );
> -}
> -
> -
>  
> /*****************************************************************************
>   * ParseTrackEntry:
>   
> *****************************************************************************/
> diff --git a/modules/demux/mkv/mkv.cpp b/modules/demux/mkv/mkv.cpp
> index bb55ad7..0459176 100644
> --- a/modules/demux/mkv/mkv.cpp
> +++ b/modules/demux/mkv/mkv.cpp
> @@ -794,4 +794,3 @@ static int Demux( demux_t *p_demux)
> 
>      return i_return;
>  }
> -
> diff --git a/modules/demux/mkv/util.cpp b/modules/demux/mkv/util.cpp
> index acb2d29..96ba128 100644
> --- a/modules/demux/mkv/util.cpp
> +++ b/modules/demux/mkv/util.cpp
> @@ -415,3 +415,45 @@ block_t * packetize_wavpack( mkv_track_t * p_tk,
> uint8_t * buffer, size_t  size)
> 
>      return p_block;
>  }
> +
> +void MkvTree_va( demux_t& demuxer, int i_level, const char* fmt, 
> va_list args)
> +{
> +    static char const * indent = "|    ";
> +    static char const * prefix = "+ ";
> +    static int  const   indent_len = strlen( indent );
> +    static int  const   prefix_len = strlen( prefix );
> +
> +    char   fixed_buffer[256] = {};
> +    size_t const  static_len = sizeof( fixed_buffer );
> +    char *            buffer = fixed_buffer;
> +    size_t         total_len = indent_len * i_level + prefix_len +
> strlen( fmt );
> +
> +    if( total_len >= static_len ) {
> +        buffer = new (std::nothrow) char[total_len] ();
> +
> +        if (buffer == NULL) {
> +            msg_Err (&demuxer, "Unable to allocate memory for format 
> string");
> +            return;
> +        }
> +    }
> +
> +    char * dst = buffer;
> +
> +    for (int i = 0; i < i_level; ++i, dst += indent_len)
> +        memcpy( dst, indent, indent_len );
> +
> +    strcat( dst, prefix );
> +    strcat( dst, fmt );
> +
> +    msg_GenericVa( &demuxer, VLC_MSG_DBG, buffer, args );
> +
> +    if (buffer != fixed_buffer)
> +        delete [] buffer;
> +}
> +
> +void MkvTree( demux_t & demuxer, int i_level, const char *psz_format, 
> ... )
> +{
> +    va_list args; va_start( args, psz_format );
> +    MkvTree_va( demuxer, i_level, psz_format, args );
> +    va_end( args );
> +}
> diff --git a/modules/demux/mkv/util.hpp b/modules/demux/mkv/util.hpp
> index 8c6cc6a..5867d1d 100644
> --- a/modules/demux/mkv/util.hpp
> +++ b/modules/demux/mkv/util.hpp
> @@ -90,3 +90,7 @@ public:
>  };
> 
>  block_t * packetize_wavpack( mkv_track_t *, uint8_t *, size_t);
> +
> +/* helper functions to print the mkv parse tree */
> +void MkvTree_va( demux_t& demuxer, int i_level, const char* fmt, 
> va_list args);
> +void MkvTree( demux_t & demuxer, int i_level, const char *psz_format, 
> ... );

Regards,
-- 
Denis Charmet - TypX
Le mauvais esprit est un art de vivre