[vlc-devel] [PATCH 03/11] demux/mkv: replaced manual memory-management with `std::vector`
Filip Roséen
filip at videolabs.io
Wed Mar 2 18:04:44 CET 2016
matroska_segment.cpp
--------------------
Manually managing memory comes at a cost of both maintainability (in
terms of safety) and performance, as such I have replaced `p_indexes`
with a `std::vector` with equivalent functionality.
Three helper member-functions have been introduced in order to clean up
the usage of the functionality, as well as removal of two now obsolete
member-variables. A `typedef` has also been introduced to aid future
development.
mkv.cpp
--------
The changes in `mkv.cpp` are due to the fact that it needs access to the
indexes present in `matroska_segment_c`; this should be refactored away
in the future.
---
modules/demux/mkv/matroska_segment.cpp | 126 +++++++++++++++------------------
modules/demux/mkv/matroska_segment.hpp | 11 ++-
modules/demux/mkv/mkv.cpp | 19 ++---
3 files changed, 74 insertions(+), 82 deletions(-)
diff --git a/modules/demux/mkv/matroska_segment.cpp b/modules/demux/mkv/matroska_segment.cpp
index 77b569e..10a05c1 100644
--- a/modules/demux/mkv/matroska_segment.cpp
+++ b/modules/demux/mkv/matroska_segment.cpp
@@ -52,8 +52,6 @@ matroska_segment_c::matroska_segment_c( demux_sys_t & demuxer, EbmlStream & estr
,p_prev_segment_uid(NULL)
,p_next_segment_uid(NULL)
,b_cues(false)
- ,i_index(0)
- ,i_index_max(1024)
,psz_muxing_application(NULL)
,psz_writing_application(NULL)
,psz_segment_filename(NULL)
@@ -65,7 +63,8 @@ matroska_segment_c::matroska_segment_c( demux_sys_t & demuxer, EbmlStream & estr
,b_preloaded(false)
,b_ref_external_segments(false)
{
- p_indexes = (mkv_index_t*)malloc( sizeof( mkv_index_t ) * i_index_max );
+ indexes.reserve (1024);
+ indexes.resize (1);
}
matroska_segment_c::~matroska_segment_c()
@@ -85,7 +84,6 @@ matroska_segment_c::~matroska_segment_c()
free( psz_segment_filename );
free( psz_title );
free( psz_date_utc );
- free( p_indexes );
delete ep;
delete segment;
@@ -126,13 +124,13 @@ void matroska_segment_c::LoadCues( KaxCues *cues )
if( MKV_IS_ID( el, KaxCuePoint ) )
{
b_invalid_cue = false;
-#define idx p_indexes[i_index]
+ mkv_index_t& last_idx = index();
- idx.i_track = -1;
- idx.i_block_number= -1;
- idx.i_position = -1;
- idx.i_mk_time = -1;
- idx.b_key = true;
+ last_idx.i_track = -1;
+ last_idx.i_block_number= -1;
+ last_idx.i_position = -1;
+ last_idx.i_mk_time = -1;
+ last_idx.b_key = true;
ep->Down();
while( ( el = ep->Get() ) != NULL )
@@ -156,7 +154,7 @@ void matroska_segment_c::LoadCues( KaxCues *cues )
b_invalid_cue = true;
break;
}
- idx.i_mk_time = uint64( ctime ) * i_timescale / INT64_C(1000);
+ last_idx.i_mk_time = uint64( ctime ) * i_timescale / INT64_C(1000);
}
else if( MKV_IS_ID( el, KaxCueTrackPositions ) )
{
@@ -177,21 +175,21 @@ void matroska_segment_c::LoadCues( KaxCues *cues )
{
KaxCueTrack &ctrack = *(KaxCueTrack*)el;
ctrack.ReadData( es.I_O() );
- idx.i_track = uint16( ctrack );
+ last_idx.i_track = uint16( ctrack );
}
else if( MKV_IS_ID( el, KaxCueClusterPosition ) )
{
KaxCueClusterPosition &ccpos = *(KaxCueClusterPosition*)el;
ccpos.ReadData( es.I_O() );
- idx.i_position = segment->GetGlobalPosition( uint64( ccpos ) );
+ last_idx.i_position = segment->GetGlobalPosition( uint64( ccpos ) );
}
else if( MKV_IS_ID( el, KaxCueBlockNumber ) )
{
KaxCueBlockNumber &cbnum = *(KaxCueBlockNumber*)el;
cbnum.ReadData( es.I_O() );
- idx.i_block_number = uint32( cbnum );
+ last_idx.i_block_number = uint32( cbnum );
}
#if LIBMATROSKA_VERSION >= 0x010401
else if( MKV_IS_ID( el, KaxCueRelativePosition ) )
@@ -227,20 +225,11 @@ void matroska_segment_c::LoadCues( KaxCues *cues )
#if 0
msg_Dbg( &sys.demuxer, " * added time=%" PRId64 " pos=%" PRId64
- " track=%d bnum=%d", idx.i_time, idx.i_position,
- idx.i_track, idx.i_block_number );
+ " track=%d bnum=%d", last_idx.i_time, last_idx.i_position,
+ last_idx.i_track, last_idx.i_block_number );
#endif
if( likely( !b_invalid_cue ) )
- {
- i_index++;
- if( i_index >= i_index_max )
- {
- i_index_max += 1024;
- p_indexes = (mkv_index_t*)xrealloc( p_indexes,
- sizeof( mkv_index_t ) * i_index_max );
- }
- }
-#undef idx
+ indexes.push_back (mkv_index_t ());
}
else
{
@@ -596,21 +585,15 @@ void matroska_segment_c::InformationCreate( )
void matroska_segment_c::IndexAppendCluster( KaxCluster *cluster )
{
-#define idx p_indexes[i_index]
- idx.i_track = -1;
- idx.i_block_number= -1;
- idx.i_position = cluster->GetElementPosition();
- idx.i_mk_time = cluster->GlobalTimecode() / INT64_C(1000);
- idx.b_key = true;
-
- i_index++;
- if( i_index >= i_index_max )
- {
- i_index_max += 1024;
- p_indexes = (mkv_index_t*)xrealloc( p_indexes,
- sizeof( mkv_index_t ) * i_index_max );
- }
-#undef idx
+ mkv_index_t& last_idx = index();
+
+ last_idx.i_track = -1;
+ last_idx.i_block_number= -1;
+ last_idx.i_position = cluster->GetElementPosition();
+ last_idx.i_mk_time = cluster->GlobalTimecode() / INT64_C(1000);
+ last_idx.b_key = true;
+
+ indexes.push_back (mkv_index_t ());
}
bool matroska_segment_c::PreloadFamily( const matroska_segment_c & of_segment )
@@ -903,10 +886,10 @@ void matroska_segment_c::Seek( mtime_t i_mk_date, mtime_t i_mk_time_offset, int6
EbmlElement *el = NULL;
/* Start from the last known index instead of the beginning eachtime */
- if( i_index == 0)
+ if(index_idx())
es.I_O().setFilePointer( i_start_pos, seek_beginning );
else
- es.I_O().setFilePointer( p_indexes[ i_index - 1 ].i_position,
+ es.I_O().setFilePointer( prev_index().i_position,
seek_beginning );
delete ep;
ep = new EbmlParser( &es, segment, &sys.demuxer,
@@ -919,9 +902,8 @@ void matroska_segment_c::Seek( mtime_t i_mk_date, mtime_t i_mk_time_offset, int6
{
cluster = (KaxCluster *)el;
i_cluster_pos = cluster->GetElementPosition();
- if( i_index == 0 ||
- ( i_index > 0 &&
- p_indexes[i_index - 1].i_position < (int64_t)cluster->GetElementPosition() ) )
+ if( index_idx() ||
+ ( prev_index().i_position < (int64_t)cluster->GetElementPosition() ) )
{
ParseCluster( cluster, false, SCOPE_NO_DATA );
IndexAppendCluster( cluster );
@@ -948,19 +930,20 @@ void matroska_segment_c::Seek( mtime_t i_mk_date, mtime_t i_mk_time_offset, int6
return;
}
- int i_idx = 0;
- if ( i_index > 0 )
- {
+ indexes_t::const_iterator index_it = indexes.begin ();
- for( ; i_idx < i_index; i_idx++ )
- if( p_indexes[i_idx].i_mk_time != -1 && p_indexes[i_idx].i_mk_time + i_mk_time_offset > i_mk_date )
+ if ( index_idx() )
+ {
+ while (index_it != indexes.end ()) {
+ if( index_it->i_mk_time != -1 && index_it->i_mk_time + i_mk_time_offset > i_mk_date )
break;
+ }
- if( i_idx > 0 )
- i_idx--;
+ if( index_it != indexes.begin ())
+ --index_it;
- i_seek_position = p_indexes[i_idx].i_position;
- i_mk_seek_time = p_indexes[i_idx].i_mk_time;
+ i_seek_position = index_it->i_position;
+ i_mk_seek_time = index_it->i_mk_time;
}
msg_Dbg( &sys.demuxer, "seek got %" PRId64 " - %" PRId64, i_mk_seek_time, i_seek_position );
@@ -1080,14 +1063,14 @@ void matroska_segment_c::Seek( mtime_t i_mk_date, mtime_t i_mk_time_offset, int6
delete block;
} while( i_mk_pts < i_mk_date );
- if( b_has_key || !i_idx )
+ if( b_has_key || !index_idx())
break;
/* No key picture was found in the cluster seek to previous seekpoint */
- i_mk_date = i_mk_time_offset + p_indexes[i_idx].i_mk_time;
- i_idx--;
+ i_mk_date = i_mk_time_offset + index_it->i_mk_time;
+ index_it--;
i_mk_pts = 0;
- es.I_O().setFilePointer( p_indexes[i_idx].i_position );
+ es.I_O().setFilePointer( index_it->i_position );
delete ep;
ep = new EbmlParser( &es, segment, &sys.demuxer,
var_InheritBool( &sys.demuxer, "mkv-use-dummy" ) );
@@ -1219,9 +1202,9 @@ void matroska_segment_c::EnsureDuration()
uint64 i_last_cluster_pos = 0;
// find the last Cluster from the Cues
- if ( b_cues && i_index > 0 && p_indexes != NULL)
+ if ( b_cues && index_idx ())
{
- i_last_cluster_pos = p_indexes[i_index-1].i_position;
+ i_last_cluster_pos = prev_index().i_position;
}
// find the last Cluster manually
@@ -1424,16 +1407,18 @@ int matroska_segment_c::BlockGet( KaxBlock * & pp_block, KaxSimpleBlock * & pp_s
}
/* update the index */
-#define idx p_indexes[i_index - 1]
- if( i_index > 0 && idx.i_mk_time == -1 )
+
+ if(index_idx() && prev_index().i_mk_time == -1 )
{
+ mkv_index_t& last_idx = prev_index();
+
if ( pp_simpleblock != NULL )
- idx.i_mk_time = pp_simpleblock->GlobalTimecode() / INT64_C(1000);
+ last_idx.i_mk_time = pp_simpleblock->GlobalTimecode() / INT64_C(1000);
else
- idx.i_mk_time = (*pp_block).GlobalTimecode() / INT64_C(1000);
- idx.b_key = *pb_key_picture;
+ last_idx.i_mk_time = (*pp_block).GlobalTimecode() / INT64_C(1000);
+
+ last_idx.b_key = *pb_key_picture;
}
-#undef idx
return VLC_SUCCESS;
}
@@ -1509,10 +1494,11 @@ int matroska_segment_c::BlockGet( KaxBlock * & pp_block, KaxSimpleBlock * & pp_s
cluster->InitTimecode( uint64( ctc ), i_timescale );
/* add it to the index */
- if( i_index == 0 ||
- ( i_index > 0 &&
- p_indexes[i_index - 1].i_position < (int64_t)cluster->GetElementPosition() ) )
+ if( index_idx() ||
+ ( prev_index().i_position < (int64_t)cluster->GetElementPosition() ) )
+ {
IndexAppendCluster( cluster );
+ }
}
else if( MKV_IS_ID( el, KaxClusterSilentTracks ) )
{
diff --git a/modules/demux/mkv/matroska_segment.hpp b/modules/demux/mkv/matroska_segment.hpp
index 790122b..20ef14b 100644
--- a/modules/demux/mkv/matroska_segment.hpp
+++ b/modules/demux/mkv/matroska_segment.hpp
@@ -26,6 +26,7 @@
#define VLC_MKV_MATROSKA_SEGMENT_HPP_
#include "mkv.hpp"
+#include <vector>
class EbmlParser;
@@ -72,6 +73,8 @@ public:
class matroska_segment_c
{
public:
+ typedef std::vector<mkv_index_t> indexes_t;
+
matroska_segment_c( demux_sys_t & demuxer, EbmlStream & estream );
virtual ~matroska_segment_c();
@@ -107,9 +110,7 @@ public:
KaxNextUID *p_next_segment_uid;
bool b_cues;
- int i_index;
- int i_index_max;
- mkv_index_t *p_indexes;
+ indexes_t indexes;
/* info */
char *psz_muxing_application;
@@ -145,6 +146,10 @@ public:
bool Select( mtime_t i_mk_start_time );
void UnSelect();
+ size_t index_idx () const { return indexes.size () - 1; }
+ mkv_index_t& index () { return *indexes.rbegin (); }
+ mkv_index_t& prev_index () { return *(indexes.end()-2); }
+
static bool CompareSegmentUIDs( const matroska_segment_c * item_a, const matroska_segment_c * item_b );
private:
diff --git a/modules/demux/mkv/mkv.cpp b/modules/demux/mkv/mkv.cpp
index b8927a4..bb55ad7 100644
--- a/modules/demux/mkv/mkv.cpp
+++ b/modules/demux/mkv/mkv.cpp
@@ -441,8 +441,6 @@ static void Seek( demux_t *p_demux, mtime_t i_mk_date, double f_percent, virtual
matroska_segment_c *p_segment = p_vsegment->CurrentSegment();
int64_t i_global_position = -1;
- int i_index;
-
msg_Dbg( p_demux, "seek request to %" PRId64 " (%f%%)", i_mk_date, f_percent );
if( i_mk_date < 0 && f_percent < 0 )
{
@@ -474,16 +472,19 @@ static void Seek( demux_t *p_demux, mtime_t i_mk_date, double f_percent, virtual
int64_t i_pos = int64_t( f_percent * stream_Size( p_demux->s ) );
msg_Dbg( p_demux, "lengthy way of seeking for pos:%" PRId64, i_pos );
- for( i_index = 0; i_index < p_segment->i_index; i_index++ )
- {
- if( p_segment->p_indexes[i_index].i_position >= i_pos &&
- p_segment->p_indexes[i_index].i_mk_time != -1 )
+
+ matroska_segment_c::indexes_t::iterator it = p_segment->indexes.begin ();;
+
+ while (it != p_segment->indexes.end ()) {
+ if( it->i_position >= i_pos && it->i_mk_time != -1 )
break;
+ ++it;
}
- if( i_index == p_segment->i_index )
- i_index--;
- if( p_segment->p_indexes[i_index].i_position < i_pos )
+ if (it == p_segment->indexes.end ())
+ p_segment->indexes.pop_back ();
+
+ if( p_segment->indexes.back().i_position < i_pos )
{
msg_Dbg( p_demux, "no cues, seek request to global pos: %" PRId64, i_pos );
i_global_position = i_pos;
--
2.7.2
More information about the vlc-devel
mailing list