[MPlayer-dev-eng] [RFC] subtitle cleanup

[Uoti Urpala]

On Thu, 2006-12-21 at 23:26 +0100, Reimar Döffinger wrote:
> Improved version with less code in mplayer.c and a lot of code in
> demux_mkv.c replaced by shared code in sub.c.

I don't particularly like using libvo/sub.c for this. It has nothing to
do with vo. Combining it with find_sub.c/subreader.c would be a better
match IMO.


+  vo_sub = &mkv_d->subs;
+  vo_sub_add_text(ptr1, strlen(ptr1),
+                  (timecode + block_duration) / 1000.0f);
+
   sub_utf8 = 1;
 #ifdef USE_ASS
   if (ass_enabled) {
     mkv_d->subs.start = timecode / 10;
     mkv_d->subs.end = (timecode + block_duration) / 10;
     ass_process_subtitle(track->sh_sub.ass_track, &mkv_d->subs);
-  } else
+  }
 #endif
-  vo_sub = &mkv_d->subs;
-  vo_osd_changed (OSDTYPE_SUBTITLE);

This looks wrong, now if libass rendering for non-SSA subtitles is
enabled they'd be rendered twice. Either the libass rendering case
should be moved from here to the sub_add_text function (probably the
better choice to also support it for other demuxers, though subs without
duration immediately given wouldn't work with current API) or
sub_add_text should not be called when ass_enabled is set.


+    char type = d_dvdsub->sh ? ((sh_sub_t *)d_dvdsub->sh)->type : 'v';

The variable should probably be renamed from d_dvdsub to something else
if it's not specific to dvd subs any more.


+        double nextpts = ds_get_next_pts(d_dvdsub);

As I explained on IRC using current ds_get_next_pts for this is wrong.


Also as is the patch would cause a regression as it converts demux_mkv.c
to use the new code even though it doesn't yet implement multiple
simultaneous subtitles.