[FFmpeg-devel] Internal handling of subtitles in ffmpeg

[Michael Niedermayer]

On Fri, Jan 02, 2009 at 07:20:06PM +0100, Reimar D?ffinger wrote:
> On Fri, Jan 02, 2009 at 05:47:24PM +0100, Michael Niedermayer wrote:
> > > Reason: require generation of alternative formats when they are not
> > > wanted?
> > 
> > did you want simplicity? if so text/ass surely should always be set for
> > text/ass types, i cant imagine the O(n) loop over n chars where n being
> > < 1000 once a second could matter speedwise ...
> 
> Well, thought that the API might provide a way to get the subtitle
> directly converted to the requested format.
> I just have a general dislike against duplicated information, and it
> might require some extra effort when an external application decodes,
> updates an passes the subtitles back again to FFmpeg.

> Of course that is assuming you want it to be a requirement, and not just
> a promise from FFmpeg side to set them.

the idea was to let the lavc core fill these in from the authorative
variant both for decoding and encoding.
Anyway this promise can be added later without breaking ABI

Suggested patch below, if you are ok with this ill try to change the
existing code to that and commit.
Btw, URLs from mphq for testing are welcome, i think our regression tests
dont cover the subtitles and i suspect neither does fate?

Also strictly below breaks ABI and would need a major bump, does anyone mind
if i ignore this as i belive noone is using AVSubtitle yet and so it would
do more harm than good ...

Also we need to change rects to an array of pointers or we will break ABI
every time AVSubtitleRect is extended.
and id like to change the uint16_t fields to int if there are no objections
and we alraedy break ABI ...


Index: libavcodec/avcodec.h
===================================================================
--- libavcodec/avcodec.h	(revision 16398)
+++ libavcodec/avcodec.h	(working copy)
@@ -30,7 +30,7 @@
 #include "libavutil/avutil.h"
 
 #define LIBAVCODEC_VERSION_MAJOR 52
-#define LIBAVCODEC_VERSION_MINOR  9
+#define LIBAVCODEC_VERSION_MINOR 10
 #define LIBAVCODEC_VERSION_MICRO  0
 
 #define LIBAVCODEC_VERSION_INT  AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
@@ -2375,19 +2375,54 @@
 
 } AVPaletteControl attribute_deprecated;
 
+enum AVSubtitleType {
+    SUBTITLE_NONE,
+
+    SUBTITLE_BITMAP,                ///< A bitmap, pict will be set
+
+    /**
+     * Plain text, the text field must be set by the decoder and is
+     * authoritative. ass and pict fields may contain approximations.
+     */
+    SUBTITLE_TEXT,
+
+    /**
+     * Formatted text, the ass field must be set by the decoder and is
+     * authoritative. pict and text fields may contain approximations.
+     */
+    SUBTITLE_ASS,
+}
+
 typedef struct AVSubtitleRect {
-    uint16_t x;
-    uint16_t y;
-    uint16_t w;
-    uint16_t h;
-    uint16_t nb_colors;
-    int linesize;
-    uint32_t *rgba_palette;
-    uint8_t *bitmap;
+    uint16_t x;         ///< top left corner  of pict, undefined when pict is not set
+    uint16_t y;         ///< top left corner  of pict, undefined when pict is not set
+    uint16_t w;         ///< width            of pict, undefined when pict is not set
+    uint16_t h;         ///< height           of pict, undefined when pict is not set
+    uint16_t nb_colors; ///< number of colors in pict, undefined when pict is not set
+
+    /**
+     * data+linesize for the bitmap of this subtitle.
+     * can be set for text/ass as well once they where rendered
+     */
+    AVPicture pict;
+    enum AVSubtitleType type;
+
+    char *text;                     ///< 0 terminated plain UTF-8 text
+
+    /**
+     * 0 terminated ASS/SSA compatible event line.
+     * The pressentation of this is unaffected by the other values in this
+     * struct.
+     */
+    char *ass;
 } AVSubtitleRect;
 
 typedef struct AVSubtitle {
-    uint16_t format; /* 0 = graphics */
+    /**
+     * if all rects have the same type this should be set to that.
+     * if their type differs this must be set to SUBTITLE_NONE
+     */
+    enum AVSubtitleType common_type;
     uint32_t start_display_time; /* relative to packet pts, in ms */
     uint32_t end_display_time; /* relative to packet pts, in ms */
     uint32_t num_rects;

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Many things microsoft did are stupid, but not doing something just because
microsoft did it is even more stupid. If everything ms did were stupid they
would be bankrupt already.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090102/b24cb861/attachment.pgp>