[vlc-devel] [PATCH 1/2] demux/mkv: make it possible to check result of ParseCluster

[Steve Lhomme]

LGTM

On Mon, Oct 31, 2016 at 12:57 PM, Filip Roséen <filip at atch.se> wrote:
> Parsing a cluster can fail for a number of different reasons, though
> previously it was impossible for the callee to know whether parsing
> was successful or not.
>
> These changes changes the signature of
> matroska_segment_c::ParseCluster so that it returns a boolean (true on
> success, false on failure).
> ---
>  modules/demux/mkv/matroska_segment.cpp       | 13 ++++++++-----
>  modules/demux/mkv/matroska_segment.hpp       |  2 +-
>  modules/demux/mkv/matroska_segment_parse.cpp |  8 +++++---
>  3 files changed, 14 insertions(+), 9 deletions(-)
>
> diff --git a/modules/demux/mkv/matroska_segment.cpp b/modules/demux/mkv/matroska_segment.cpp
> index acb8a54..266477f 100644
> --- a/modules/demux/mkv/matroska_segment.cpp
> +++ b/modules/demux/mkv/matroska_segment.cpp
> @@ -492,8 +492,8 @@ bool matroska_segment_c::PreloadClusters(uint64 i_cluster_pos)
>
>          E_CASE( KaxCluster, kcluster )
>          {
> -            vars.obj->ParseCluster( &kcluster, false );
> -            vars.obj->IndexAppendCluster( &kcluster );
> +            if( vars.obj->ParseCluster( &kcluster, false ) )
> +                vars.obj->IndexAppendCluster( &kcluster );
>          }
>
>          E_CASE_DEFAULT( el )
> @@ -633,9 +633,10 @@ bool matroska_segment_c::Preload( )
>              }
>              msg_Dbg( &sys.demuxer, "|   + Cluster" );
>
> -            cluster = kc_ptr;
> +            if( !ParseCluster( kc_ptr ) )
> +                break;
>
> -            ParseCluster( cluster );
> +            cluster = kc_ptr;
>              IndexAppendCluster( cluster );
>
>              // add first cluster as trusted seekpoint for all tracks
> @@ -1025,7 +1026,9 @@ void matroska_segment_c::EnsureDuration()
>          KaxCluster *p_last_cluster = static_cast<KaxCluster*>( eparser.Get() );
>          if( p_last_cluster == NULL )
>              return;
> -        ParseCluster( p_last_cluster, false, SCOPE_PARTIAL_DATA );
> +
> +        if( !ParseCluster( p_last_cluster, false, SCOPE_PARTIAL_DATA ) )
> +            return;
>
>          if( p_last_cluster->IsFiniteSize() == false )
>          {
> diff --git a/modules/demux/mkv/matroska_segment.hpp b/modules/demux/mkv/matroska_segment.hpp
> index 0aa01be..f8df9f6 100644
> --- a/modules/demux/mkv/matroska_segment.hpp
> +++ b/modules/demux/mkv/matroska_segment.hpp
> @@ -164,7 +164,7 @@ private:
>      void ParseTracks( KaxTracks *tracks );
>      void ParseChapterAtom( int i_level, KaxChapterAtom *ca, chapter_item_c & chapters );
>      void ParseTrackEntry( KaxTrackEntry *m );
> -    void ParseCluster( KaxCluster *cluster, bool b_update_start_time = true, ScopeMode read_fully = SCOPE_ALL_DATA );
> +    bool ParseCluster( KaxCluster *cluster, bool b_update_start_time = true, ScopeMode read_fully = SCOPE_ALL_DATA );
>      bool ParseSimpleTags( SimpleTag* out, KaxTagSimple *tag, int level = 50 );
>      void IndexAppendCluster( KaxCluster *cluster );
>      int32_t TrackInit( mkv_track_t * p_tk );
> diff --git a/modules/demux/mkv/matroska_segment_parse.cpp b/modules/demux/mkv/matroska_segment_parse.cpp
> index 4eb0e7a..5429b1f 100644
> --- a/modules/demux/mkv/matroska_segment_parse.cpp
> +++ b/modules/demux/mkv/matroska_segment_parse.cpp
> @@ -1207,12 +1207,12 @@ void matroska_segment_c::ParseChapters( KaxChapters *chapters )
>      KaxChapterHandler::Dispatcher().iterate( chapters->begin(), chapters->end(), KaxChapterHandler::Payload( *this ) );
>  }
>
> -void matroska_segment_c::ParseCluster( KaxCluster *cluster, bool b_update_start_time, ScopeMode read_fully )
> +bool matroska_segment_c::ParseCluster( KaxCluster *cluster, bool b_update_start_time, ScopeMode read_fully )
>  {
>      if( unlikely( cluster->IsFiniteSize() && cluster->GetSize() >= SIZE_MAX ) )
>      {
>          msg_Err( &sys.demuxer, "Cluster too big, aborting" );
> -        return;
> +        return false;
>      }
>
>      try
> @@ -1225,7 +1225,7 @@ void matroska_segment_c::ParseCluster( KaxCluster *cluster, bool b_update_start_
>      catch(...)
>      {
>          msg_Err( &sys.demuxer, "Error while reading cluster" );
> -        return;
> +        return false;
>      }
>
>      for( unsigned int i = 0; i < cluster->ListSize(); ++i )
> @@ -1240,6 +1240,8 @@ void matroska_segment_c::ParseCluster( KaxCluster *cluster, bool b_update_start_
>
>      if( b_update_start_time )
>          i_mk_start_time = cluster->GlobalTimecode() / INT64_C( 1000 );
> +
> +    return true;
>  }
>
>
> --
> 2.10.1
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel