[vlc-devel] [V2 04/16] mkv: Use input_attachment_t directly

[Hugo Beauzée-Luyssen]

So we can just hold them instead of performing a deep copy when they are
fetched by the core
---
 modules/demux/mkv/demux.cpp                  |  2 --
 modules/demux/mkv/demux.hpp                  |  5 ++-
 modules/demux/mkv/matroska_segment_parse.cpp | 37 +++++++++-----------
 modules/demux/mkv/mkv.cpp                    |  9 +----
 modules/demux/mkv/mkv.hpp                    | 30 ----------------
 5 files changed, 22 insertions(+), 61 deletions(-)

diff --git a/modules/demux/mkv/demux.cpp b/modules/demux/mkv/demux.cpp
index 99f1cd67d1..1ea99e1d9e 100644
--- a/modules/demux/mkv/demux.cpp
+++ b/modules/demux/mkv/demux.cpp
@@ -39,8 +39,6 @@ demux_sys_t::~demux_sys_t()
         delete opened_segments[i];
     for ( i=0; i<used_vsegments.size(); i++ )
         delete used_vsegments[i];
-    for ( i=0; i<stored_attachments.size(); i++ )
-        delete stored_attachments[i];
     if( meta ) vlc_meta_Delete( meta );
 
     while( titles.size() )
diff --git a/modules/demux/mkv/demux.hpp b/modules/demux/mkv/demux.hpp
index e5d53596e4..b7eb92e55c 100644
--- a/modules/demux/mkv/demux.hpp
+++ b/modules/demux/mkv/demux.hpp
@@ -31,6 +31,8 @@
 #include "dvd_types.hpp"
 #include "events.hpp"
 
+#include <memory>
+
 namespace mkv {
 
 class virtual_segment_c;
@@ -80,7 +82,8 @@ public:
     unsigned                         i_updates;
 
     std::vector<matroska_stream_c*>  streams;
-    std::vector<attachment_c*>       stored_attachments;
+    std::vector<std::unique_ptr<input_attachment_t,
+                    void(*)(input_attachment_t*)>> stored_attachments;
     std::vector<matroska_segment_c*> opened_segments;
     std::vector<virtual_segment_c*>  used_vsegments;
     virtual_segment_c                *p_current_vsegment;
diff --git a/modules/demux/mkv/matroska_segment_parse.cpp b/modules/demux/mkv/matroska_segment_parse.cpp
index 49e1d3f260..1a312a9256 100644
--- a/modules/demux/mkv/matroska_segment_parse.cpp
+++ b/modules/demux/mkv/matroska_segment_parse.cpp
@@ -1456,33 +1456,30 @@ void matroska_segment_c::ParseAttachments( KaxAttachments *attachments )
     {
         KaxFileData  &img_data     = GetChild<KaxFileData>( *attachedFile );
         std::string attached_filename( UTFstring( GetChild<KaxFileName>( *attachedFile ) ).GetUTF8() );
-        attachment_c *new_attachment = new attachment_c( attached_filename,
-                                                         GetChild<KaxMimeType>( *attachedFile ),
-                                                         img_data.GetSize() );
-
-        msg_Dbg( &sys.demuxer, "|   |   - %s (%s)", new_attachment->fileName(), new_attachment->mimeType() );
-
-        if( new_attachment->init() )
-        {
-            memcpy( new_attachment->p_data, img_data.GetBuffer(), img_data.GetSize() );
-            sys.stored_attachments.push_back( new_attachment );
-            if( !strncmp( new_attachment->mimeType(), "image/", 6 ) )
+        auto new_attachment = vlc_input_attachment_New( attached_filename.c_str(),
+                                                        GetChild<KaxMimeType>( *attachedFile ).GetValue().c_str(),
+                                                        nullptr,
+                                                        img_data.GetBuffer(),
+                                                        img_data.GetSize() );
+        if (!new_attachment)
+            continue;
+        msg_Dbg( &sys.demuxer, "|   |   - %s (%s)", new_attachment->psz_name,
+                 new_attachment->psz_mime );
+
+        if( !strncmp( new_attachment->psz_mime, "image/", 6 ) )
+        {
+            char *psz_url;
+            if( asprintf( &psz_url, "attachment://%s",
+                          new_attachment->psz_name ) >= 0 )
             {
-                char *psz_url;
-                if( asprintf( &psz_url, "attachment://%s",
-                              new_attachment->fileName() ) == -1 )
-                    continue;
                 if( !sys.meta )
                     sys.meta = vlc_meta_New();
                 vlc_meta_SetArtURL( sys.meta, psz_url );
                 free( psz_url );
             }
         }
-        else
-        {
-            delete new_attachment;
-        }
-
+        sys.stored_attachments.push_back( vlc::wrap_cptr( new_attachment,
+                                                          &vlc_input_attachment_Release ) );
         attachedFile = &GetNextChild<KaxAttached>( *attachments, *attachedFile );
     }
 }
diff --git a/modules/demux/mkv/mkv.cpp b/modules/demux/mkv/mkv.cpp
index 9157d36d3e..7a6b72a32e 100644
--- a/modules/demux/mkv/mkv.cpp
+++ b/modules/demux/mkv/mkv.cpp
@@ -340,14 +340,7 @@ static int Control( demux_t *p_demux, int i_query, va_list args )
                 return VLC_ENOMEM;
             for( size_t i = 0; i < p_sys->stored_attachments.size(); i++ )
             {
-                attachment_c *a = p_sys->stored_attachments[i];
-                (*ppp_attach)[i] = vlc_input_attachment_New( a->fileName(), a->mimeType(), NULL,
-                                                             a->p_data, a->size() );
-                if( !(*ppp_attach)[i] )
-                {
-                    free(*ppp_attach);
-                    return VLC_ENOMEM;
-                }
+                (*ppp_attach)[i] = vlc_input_attachment_Hold( p_sys->stored_attachments[i].get() );
             }
             return VLC_SUCCESS;
 
diff --git a/modules/demux/mkv/mkv.hpp b/modules/demux/mkv/mkv.hpp
index 924d460b23..5d8c24603b 100644
--- a/modules/demux/mkv/mkv.hpp
+++ b/modules/demux/mkv/mkv.hpp
@@ -124,36 +124,6 @@ void BlockDecode( demux_t *p_demux, KaxBlock *block, KaxSimpleBlock *simpleblock
                   vlc_tick_t i_pts, vlc_tick_t i_duration, bool b_key_picture,
                   bool b_discardable_picture );
 
-class attachment_c
-{
-public:
-    attachment_c( const std::string& _str_file_name, const std::string& _str_mime_type, int _i_size )
-        :i_size(_i_size)
-        ,str_file_name( _str_file_name)
-        ,str_mime_type( _str_mime_type)
-    {
-        p_data = NULL;
-    }
-    ~attachment_c() { free( p_data ); }
-
-    /* Allocs the data space. Returns true if allocation went ok */
-    bool init()
-    {
-        p_data = malloc( i_size );
-        return (p_data != NULL);
-    }
-
-    const char* fileName() const { return str_file_name.c_str(); }
-    const char* mimeType() const { return str_mime_type.c_str(); }
-    int         size() const    { return i_size; }
-
-    void          *p_data;
-private:
-    int            i_size;
-    std::string    str_file_name;
-    std::string    str_mime_type;
-};
-
 class matroska_segment_c;
 struct matroska_stream_c
 {
-- 
2.29.2