[MPlayer-dev-eng] [PATCH] SSA/ASS subtitles support

Uoti Urpala uoti.urpala at pp1.inet.fi
Fri Apr 21 04:44:45 CEST 2006


You now make the global functions empty macros if HAVE_FREETYPE is not
defined, but still unconditionally build ass.c which does not have the
dummy functions anymore and apparently builds an empty object file. Is
there any need to do that?

You should add a line like "libsub/libsub.a: $(wildcard libsub/*.[ch])"
to the top-level Makefile to trigger recursive make in libsub/ when
files there change.


@@ -2265,6 +2392,8 @@
         default:
           cont = 1;
         case MATROSKA_ID_ATTACHMENTS:
+          cont = demux_mkv_read_attachments (demuxer);
+          break;
         case EBML_ID_VOID:
           ebml_read_skip (s, NULL);
           break;

I think you missed the default case fallthrough above.


+demux_mkv_read_attachments (demuxer_t *demuxer)

The memory allocated in this function is never freed. Embedded fonts are
saved under the user's .mplayer directory, I think there should be at
least an option to turn off such creation of new files. Because the
created files use names from the video file without any security checks,
they can contain things like '../../' and thus new files can be created
anywhere on the filesystem. There seems to be some kind of check against
overwriting files, I don't see the point of that - it's obviously
insufficient as a security check, and if there are different font files
with the same name wouldn't you usually want to use the one from the
video that was last opened?




More information about the MPlayer-dev-eng mailing list