[MPlayer-dev-eng] [PATCH] mkv: remove ->sub->sh malloc hack and possible memleak

[Reimar Döffinger]

Hello,
this patch removes a hack that mallocs demuxer->sub->sh and memcpys the
contents over instead of just setting it to some demuxer->s_streams.
This reduces the size of the track struct, probably should fix a memleak
(though valgrind can't find any with the old code, but neither can it
find a double free with the new code... Hm, still some bugs left to
fix) and should allow to get rid of the special-case code for mkv track
switching.
Ok to apply?

Greetings,
Reimar Döffinger
-------------- next part --------------
Index: libmpdemux/demux_mkv.c
===================================================================
--- libmpdemux/demux_mkv.c	(revision 23611)
+++ libmpdemux/demux_mkv.c	(working copy)
@@ -132,7 +132,7 @@
   int num_encodings;
 
   /* For VobSubs and SSA/ASS */
-  sh_sub_t sh_sub;
+  sh_sub_t *sh_sub;
 } mkv_track_t;
 
 typedef struct mkv_index
@@ -445,7 +445,7 @@
     return 0;
   memcpy(buf, t->private_data, t->private_size);
   buf[t->private_size] = 0;
-  t->sh_sub.has_palette = 0;
+  t->sh_sub->has_palette = 0;
 
   pos = buf;
   start = buf;
@@ -459,13 +459,13 @@
           *pos = 0;
 
           if (!strncasecmp(start, "size: ", 6))
-            things_found |= vobsub_parse_size(&t->sh_sub, start);
+            things_found |= vobsub_parse_size(t->sh_sub, start);
           else if (!strncasecmp(start, "palette:", 8))
-            things_found |= vobsub_parse_palette(&t->sh_sub, start);
+            things_found |= vobsub_parse_palette(t->sh_sub, start);
           else if (!strncasecmp(start, "custom colors:", 14))
-            things_found |= vobsub_parse_custom_colors(&t->sh_sub, start);
+            things_found |= vobsub_parse_custom_colors(t->sh_sub, start);
           else if (!strncasecmp(start, "forced subs:", 12))
-            things_found |= vobsub_parse_forced_subs(&t->sh_sub, start);
+            things_found |= vobsub_parse_forced_subs(t->sh_sub, start);
 
           if (last)
             break;
@@ -952,8 +952,8 @@
   if (track->audio_timestamp)
     free (track->audio_timestamp);
 #ifdef USE_ASS
-  if (track->sh_sub.ass_track)
-    ass_free_track (track->sh_sub.ass_track);
+  if (track->sh_sub && track->sh_sub->ass_track)
+    ass_free_track (track->sh_sub->ass_track);
 #endif
   demux_mkv_free_encodings(track->encodings, track->num_encodings);
   free(track);
@@ -2283,7 +2283,7 @@
 
       if (track->subtitle_type == MATROSKA_SUBTYPE_SSA)
         {
-          track->sh_sub.ass_track = ass_new_track(ass_library);
+          track->sh_sub->ass_track = ass_new_track(ass_library);
           size = track->private_size;
           m = demux_mkv_decode (track,track->private_data,&buffer,&size,2);
           if (buffer && m)
@@ -2292,7 +2292,7 @@
               track->private_data = buffer;
               track->private_size = size;
             }
-          ass_process_codec_private(track->sh_sub.ass_track, track->private_data, track->private_size);
+          ass_process_codec_private(track->sh_sub->ass_track, track->private_data, track->private_size);
         }
     }
 }
@@ -2304,13 +2304,12 @@
   if (track->subtitle_type != MATROSKA_SUBTYPE_UNKNOWN)
     {
       sh_sub_t *sh = new_sh_sub_sid(demuxer, track->tnum, sid);
-      track->sh_sub.type = 't';
+      track->sh_sub = sh;
+      sh->type = 't';
       if (track->subtitle_type == MATROSKA_SUBTYPE_VOBSUB)
-        track->sh_sub.type = 'v';
+        sh->type = 'v';
       if (track->subtitle_type == MATROSKA_SUBTYPE_SSA)
-        track->sh_sub.type = 'a';
-              if (sh)
-                memcpy(sh, &track->sh_sub, sizeof(sh_sub_t));
+        sh->type = 'a';
     }
   else
     {
@@ -2568,10 +2567,7 @@
                     MSGTR_MPDEMUX_MKV_WillDisplaySubtitleTrack, track->tnum);
 	    dvdsub_id = demux_mkv_reverse_id(mkv_d, track->tnum, MATROSKA_TRACK_SUBTITLE);
             demuxer->sub->id = track->tnum;
-            if (demuxer->sub->sh == NULL)
-              demuxer->sub->sh = malloc(sizeof(sh_sub_t));
-            if (demuxer->sub->sh != NULL)
-              memcpy(demuxer->sub->sh, &track->sh_sub, sizeof(sh_sub_t));
+            demuxer->sub->sh = demuxer->s_streams[track->tnum];
           }
   else
     demuxer->sub->id = -2;
@@ -2756,7 +2752,7 @@
 
 #ifdef USE_ASS
   if (ass_enabled && track->subtitle_type == MATROSKA_SUBTYPE_SSA) {
-    ass_process_chunk(track->sh_sub.ass_track, block, size, (long long)timecode, (long long)block_duration);
+    ass_process_chunk(track->sh_sub->ass_track, block, size, (long long)timecode, (long long)block_duration);
     return;
   }
 #endif
@@ -3630,10 +3626,7 @@
   if (track == NULL)
     return -1;
 
-  if (demuxer->sub->sh == NULL)
-    demuxer->sub->sh = malloc(sizeof(sh_sub_t));
-  if (demuxer->sub->sh != NULL)
-    memcpy(demuxer->sub->sh, &track->sh_sub, sizeof(sh_sub_t));
+  demuxer->sub->sh = demuxer->s_streams[track->tnum];
 
   return track->tnum;
 }