[vlc-devel] [PATCH 16/20] mkv: removed manual memory management from Tags + SimpleTags
Filip Roséen
filip at videolabs.io
Fri Mar 4 17:04:14 CET 2016
In order to provide better runtime safety manual management of dynamic
memory has been removed from `class Tags` and `class SimpleTags`.
This includes introducing usage of `std::vector` as well as
`std::string`, while also removing data-members that are no longer
required.
- SimpleTag::b_default has been removed since it was written to (at
once place), but never read; the write has been replaced by a comment.
- The ParseSimpleTag function has been changed to return `bool` instead
of a pointer to signal whether parsing was successful. The result
will be written to `pout_simple` (instead of being indirectly
returned through a pointer).
---
modules/demux/mkv/demux.cpp | 14 ++--
modules/demux/mkv/matroska_segment.cpp | 117 +++++++++++++--------------------
modules/demux/mkv/matroska_segment.hpp | 24 ++++---
3 files changed, 62 insertions(+), 93 deletions(-)
diff --git a/modules/demux/mkv/demux.cpp b/modules/demux/mkv/demux.cpp
index 2fbc69a..f2eed13 100644
--- a/modules/demux/mkv/demux.cpp
+++ b/modules/demux/mkv/demux.cpp
@@ -682,21 +682,21 @@ bool demux_sys_t::PreloadLinked()
/* Check in tags if the edition has a name */
/* We use only the tags of the first segment as it contains the edition */
- std::vector<Tag*> &tags = opened_segments[0]->tags;
+ matroska_segment_c::tags_t const& tags = opened_segments[0]->tags;
uint64_t i_ed_uid = 0;
if( p_ved->p_edition )
i_ed_uid = (uint64_t) p_ved->p_edition->i_uid;
for( size_t k = 0; k < tags.size(); k++ )
{
- if( tags[k]->i_tag_type == EDITION_UID && tags[k]->i_uid == i_ed_uid )
- for( size_t l = 0; l < tags[k]->simple_tags.size(); l++ )
+ if( tags[k].i_tag_type == EDITION_UID && tags[k].i_uid == i_ed_uid )
+ for( size_t l = 0; l < tags[k].simple_tags.size(); l++ )
{
- SimpleTag * p_st = tags[k]->simple_tags[l];
- if( !strcmp(p_st->psz_tag_name,"TITLE") )
+ SimpleTag const& st = tags[k].simple_tags[l];
+ if ( st.tag_name == "TITLE" )
{
- msg_Dbg( &demuxer, "Using title \"%s\" from tag for edition %" PRIu64, p_st->p_value, i_ed_uid );
- p_title->psz_name = strdup( p_st->p_value );
+ msg_Dbg( &demuxer, "Using title \"%s\" from tag for edition %" PRIu64, st.value.c_str (), i_ed_uid );
+ p_title->psz_name = strdup( st.value.c_str () );
break;
}
}
diff --git a/modules/demux/mkv/matroska_segment.cpp b/modules/demux/mkv/matroska_segment.cpp
index 693d68e..b3947b3 100644
--- a/modules/demux/mkv/matroska_segment.cpp
+++ b/modules/demux/mkv/matroska_segment.cpp
@@ -42,7 +42,6 @@ matroska_segment_c::matroska_segment_c( demux_sys_t & demuxer, EbmlStream & estr
,i_tracks_position(-1)
,i_info_position(-1)
,i_chapters_position(-1)
- ,i_tags_position(-1)
,i_attachments_position(-1)
,cluster(NULL)
,i_block_pos(0)
@@ -94,7 +93,6 @@ matroska_segment_c::~matroska_segment_c()
vlc_delete_all( stored_editions );
vlc_delete_all( translations );
vlc_delete_all( families );
- vlc_delete_all( tags );
}
@@ -257,20 +255,13 @@ static const struct {
{vlc_meta_Title, NULL, 0},
};
-SimpleTag * matroska_segment_c::ParseSimpleTags( KaxTagSimple *tag, int target_type )
+bool matroska_segment_c::ParseSimpleTags( SimpleTag* pout_simple, KaxTagSimple *tag, int target_type )
{
EbmlParser eparser ( &es, tag, &sys.demuxer, var_InheritBool( &sys.demuxer, "mkv-use-dummy" ) );
EbmlElement *el;
- SimpleTag * p_simple = new (std::nothrow) SimpleTag;
size_t max_size = tag->GetSize();
size_t size = 0;
- if( !p_simple )
- {
- msg_Err( &sys.demuxer, "Couldn't allocate memory for Simple Tag... ignoring it");
- return NULL;
- }
-
if( !sys.meta )
sys.meta = vlc_meta_New();
@@ -283,35 +274,36 @@ SimpleTag * matroska_segment_c::ParseSimpleTags( KaxTagSimple *tag, int target_t
{
msg_Err( &sys.demuxer, "Error %s too big ignoring the tag", typeid(*el).name() );
delete ep;
- delete p_simple;
- return NULL;
+ return false;
}
if( MKV_CHECKED_PTR_DECL ( ktn_ptr, KaxTagName, el ) )
{
ktn_ptr->ReadData( es.I_O(), SCOPE_ALL_DATA );
- p_simple->psz_tag_name = strdup( UTFstring( *ktn_ptr ).GetUTF8().c_str() );
+ pout_simple->tag_name = UTFstring( *ktn_ptr ).GetUTF8().c_str();
}
else if( MKV_CHECKED_PTR_DECL ( kts_ptr, KaxTagString, el ) )
{
kts_ptr->ReadData( es.I_O(), SCOPE_ALL_DATA );
- p_simple->p_value = strdup( UTFstring( *kts_ptr ).GetUTF8().c_str() );
+ pout_simple->value = UTFstring( *kts_ptr ).GetUTF8().c_str();
}
else if( MKV_CHECKED_PTR_DECL ( ktl_ptr, KaxTagLangue, el ) )
{
ktl_ptr->ReadData( es.I_O(), SCOPE_ALL_DATA );
- p_simple->psz_lang = strdup( std::string( *ktl_ptr ).c_str());
+ pout_simple->lang = *ktl_ptr;
}
else if( MKV_CHECKED_PTR_DECL ( ktd_ptr, KaxTagDefault, el ) )
{
- ktd_ptr->ReadData( es.I_O(), SCOPE_ALL_DATA );
- p_simple->b_default = (bool) uint8( *ktd_ptr );
+ VLC_UNUSED(ktd_ptr); // TODO: we do not care about this value, but maybe we should?
}
/*Tags can be nested*/
else if( MKV_CHECKED_PTR_DECL ( kts_ptr, KaxTagSimple, el) )
{
- SimpleTag * p_st = ParseSimpleTags( kts_ptr, target_type );
- if( p_st )
- p_simple->sub_tags.push_back( p_st );
+ SimpleTag st; // ParseSimpleTags will write to this variable
+ // the SimpleTag is valid if ParseSimpleTags returns `true`
+
+ if (ParseSimpleTags( &st, kts_ptr, target_type )) {
+ pout_simple->sub_tags.push_back( st );
+ }
}
/*TODO Handle binary tags*/
size += el->HeadSize() + el->GetSize();
@@ -321,30 +313,28 @@ SimpleTag * matroska_segment_c::ParseSimpleTags( KaxTagSimple *tag, int target_t
{
msg_Err( &sys.demuxer, "Error while reading Tag ");
delete ep;
- delete p_simple;
- return NULL;
+ return false;
}
- if( !p_simple->psz_tag_name || !p_simple->p_value )
+ if( pout_simple->tag_name.empty() )
{
msg_Warn( &sys.demuxer, "Invalid MKV SimpleTag found.");
- delete p_simple;
- return NULL;
+ return false;
}
for( int i = 0; metadata_map[i].key; i++ )
{
- if( !strcmp( p_simple->psz_tag_name, metadata_map[i].key ) &&
+ if( pout_simple->tag_name == metadata_map[i].key &&
(metadata_map[i].target_type == 0 || target_type == metadata_map[i].target_type ) )
{
- vlc_meta_Set( sys.meta, metadata_map[i].type, p_simple->p_value );
- msg_Dbg( &sys.demuxer, "| | + Meta %s: %s", p_simple->psz_tag_name, p_simple->p_value);
+ vlc_meta_Set( sys.meta, metadata_map[i].type, pout_simple->value.c_str () );
+ msg_Dbg( &sys.demuxer, "| | + Meta %s: %s", pout_simple->tag_name.c_str (), pout_simple->value.c_str ());
goto done;
}
}
- msg_Dbg( &sys.demuxer, "| | + Meta %s: %s", p_simple->psz_tag_name, p_simple->p_value);
- vlc_meta_AddExtra( sys.meta, p_simple->psz_tag_name, p_simple->p_value);
+ msg_Dbg( &sys.demuxer, "| | + Meta %s: %s", pout_simple->tag_name.c_str (), pout_simple->value.c_str ());
+ vlc_meta_AddExtra( sys.meta, pout_simple->tag_name.c_str (), pout_simple->value.c_str ());
done:
- return p_simple;
+ return true;
}
#define PARSE_TAG( type ) \
@@ -368,12 +358,8 @@ void matroska_segment_c::LoadTags( KaxTags *tags )
{
if( MKV_IS_ID( el, KaxTag ) )
{
- Tag * p_tag = new (std::nothrow) Tag;
- if(!p_tag)
- {
- msg_Err( &sys.demuxer,"Couldn't allocate memory for tag... ignoring it");
- continue;
- }
+ Tag tag;
+
msg_Dbg( &sys.demuxer, "+ Tag" );
eparser.Down();
int target_type = 50;
@@ -401,32 +387,32 @@ void matroska_segment_c::LoadTags( KaxTags *tags )
}
else if( MKV_CHECKED_PTR_DECL ( kttu_ptr, KaxTagTrackUID, el ) )
{
- p_tag->i_tag_type = TRACK_UID;
+ tag.i_tag_type = TRACK_UID;
kttu_ptr->ReadData( es.I_O() );
- p_tag->i_uid = static_cast<uint64>( *kttu_ptr );
- msg_Dbg( &sys.demuxer, "| | + TrackUID: %" PRIu64, p_tag->i_uid);
+ tag.i_uid = static_cast<uint64>( *kttu_ptr );
+ msg_Dbg( &sys.demuxer, "| | + TrackUID: %" PRIu64, tag.i_uid);
}
else if( MKV_CHECKED_PTR_DECL ( kteu_ptr, KaxTagEditionUID, el ) )
{
- p_tag->i_tag_type = EDITION_UID;
+ tag.i_tag_type = EDITION_UID;
kteu_ptr->ReadData( es.I_O() );
- p_tag->i_uid = static_cast<uint64>( *kteu_ptr );
- msg_Dbg( &sys.demuxer, "| | + EditionUID: %" PRIu64, p_tag->i_uid);
+ tag.i_uid = static_cast<uint64>( *kteu_ptr );
+ msg_Dbg( &sys.demuxer, "| | + EditionUID: %" PRIu64, tag.i_uid);
}
else if( MKV_CHECKED_PTR_DECL ( ktcu_ptr, KaxTagChapterUID, el ) )
{
- p_tag->i_tag_type = CHAPTER_UID;
+ tag.i_tag_type = CHAPTER_UID;
ktcu_ptr->ReadData( es.I_O() );
- p_tag->i_uid = static_cast<uint64>( *ktcu_ptr );
- msg_Dbg( &sys.demuxer, "| | + ChapterUID: %" PRIu64, p_tag->i_uid);
+ tag.i_uid = static_cast<uint64>( *ktcu_ptr );
+ msg_Dbg( &sys.demuxer, "| | + ChapterUID: %" PRIu64, tag.i_uid);
}
else if( MKV_CHECKED_PTR_DECL ( ktau_ptr, KaxTagAttachmentUID, el ) )
{
- p_tag->i_tag_type = ATTACHMENT_UID;
+ tag.i_tag_type = ATTACHMENT_UID;
ktau_ptr->ReadData( es.I_O() );
- p_tag->i_uid = static_cast<uint64>( *ktau_ptr );
- msg_Dbg( &sys.demuxer, "| | + AttachmentUID: %" PRIu64, p_tag->i_uid);
+ tag.i_uid = static_cast<uint64>( *ktau_ptr );
+ msg_Dbg( &sys.demuxer, "| | + AttachmentUID: %" PRIu64, tag.i_uid);
}
else
{
@@ -443,9 +429,11 @@ void matroska_segment_c::LoadTags( KaxTags *tags )
}
else if( MKV_CHECKED_PTR_DECL ( kts_ptr, KaxTagSimple, el ) )
{
- SimpleTag * p_simple = ParseSimpleTags( kts_ptr, target_type );
- if( p_simple )
- p_tag->simple_tags.push_back( p_simple );
+ SimpleTag simple;
+
+ if (ParseSimpleTags(&simple, kts_ptr, target_type )) {
+ tag.simple_tags.push_back( simple );
+ }
}
#if 0 // not valid anymore
else if( MKV_IS_ID( el, KaxTagGeneral ) )
@@ -491,7 +479,7 @@ void matroska_segment_c::LoadTags( KaxTags *tags )
}
}
eparser.Up();
- this->tags.push_back(p_tag);
+ this->tags.push_back(tag);
}
else
{
@@ -542,7 +530,7 @@ void matroska_segment_c::InformationCreate( )
}
#endif
#if 0
- if( i_tags_position >= 0 )
+ if( !tags.empty () )
{
bool b_seekable;
@@ -707,10 +695,9 @@ bool matroska_segment_c::Preload( )
else if( MKV_CHECKED_PTR_DECL ( kt_ptr, KaxTags, el ) )
{
msg_Dbg( &sys.demuxer, "| + Tags" );
- if( i_tags_position < 0)
+ if(tags.empty ())
{
LoadTags( kt_ptr );
- i_tags_position = el->GetElementPosition();
}
}
else if( MKV_IS_ID ( el, EbmlVoid ) )
@@ -812,10 +799,9 @@ bool matroska_segment_c::LoadSeekHeadItem( const EbmlCallbacks & ClassInfos, int
else if( MKV_CHECKED_PTR_DECL ( kt_ptr, KaxTags, el ) )
{
msg_Dbg( &sys.demuxer, "| + Tags" );
- if( i_tags_position < 0 )
+ if(tags.empty ())
{
LoadTags( kt_ptr );
- i_tags_position = i_element_position;
}
}
else
@@ -1557,18 +1543,3 @@ int matroska_segment_c::BlockGet( KaxBlock * & pp_block, KaxSimpleBlock * & pp_s
}
}
}
-
-SimpleTag::~SimpleTag()
-{
- free(psz_tag_name);
- free(psz_lang);
- free(p_value);
- for(size_t i = 0; i < sub_tags.size(); i++)
- delete sub_tags[i];
-}
-
-Tag::~Tag()
-{
- for(size_t i = 0; i < simple_tags.size(); i++)
- delete simple_tags[i];
-}
diff --git a/modules/demux/mkv/matroska_segment.hpp b/modules/demux/mkv/matroska_segment.hpp
index 20ef14b..cab3702 100644
--- a/modules/demux/mkv/matroska_segment.hpp
+++ b/modules/demux/mkv/matroska_segment.hpp
@@ -27,6 +27,7 @@
#include "mkv.hpp"
#include <vector>
+#include <string>
class EbmlParser;
@@ -49,31 +50,29 @@ typedef enum
class SimpleTag
{
public:
- SimpleTag():
- psz_tag_name(NULL), psz_lang(NULL), b_default(true), p_value(NULL){}
- ~SimpleTag();
- char *psz_tag_name;
- char *psz_lang; /* NULL value means "undf" */
- bool b_default;
- char * p_value;
- std::vector<SimpleTag *> sub_tags;
+ typedef std::vector<SimpleTag> sub_tags_t;
+ std::string tag_name;
+ std::string lang;
+ std::string value;
+ sub_tags_t sub_tags;
};
class Tag
{
public:
+ typedef std::vector<SimpleTag> simple_tags_t;
Tag():i_tag_type(WHOLE_SEGMENT),i_target_type(50),i_uid(0){}
- ~Tag();
tag_target_type i_tag_type;
uint64_t i_target_type;
uint64_t i_uid;
- std::vector<SimpleTag*> simple_tags;
+ simple_tags_t simple_tags;
};
class matroska_segment_c
{
public:
typedef std::vector<mkv_index_t> indexes_t;
+ typedef std::vector<Tag> tags_t;
matroska_segment_c( demux_sys_t & demuxer, EbmlStream & estream );
virtual ~matroska_segment_c();
@@ -98,7 +97,6 @@ public:
int64_t i_tracks_position;
int64_t i_info_position;
int64_t i_chapters_position;
- int64_t i_tags_position;
int64_t i_attachments_position;
KaxCluster *cluster;
@@ -127,7 +125,7 @@ public:
std::vector<chapter_translation_c*> translations;
std::vector<KaxSegmentFamily*> families;
- std::vector<Tag *> tags;
+ tags_t tags;
demux_sys_t & sys;
EbmlParser *ep;
@@ -164,7 +162,7 @@ private:
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 );
- SimpleTag * ParseSimpleTags( KaxTagSimple *tag, int level = 50 );
+ bool ParseSimpleTags( SimpleTag* out, KaxTagSimple *tag, int level = 50 );
void IndexAppendCluster( KaxCluster *cluster );
int32_t TrackInit( mkv_track_t * p_tk );
void ComputeTrackPriority();
--
2.7.2
More information about the vlc-devel
mailing list