[FFmpeg-devel] [RFC] AST subtitles

[Michael Niedermayer]

On Sun, Nov 25, 2012 at 07:49:00PM +0100, Clément Bœsch wrote:
> On Sun, Nov 25, 2012 at 07:28:35PM +0100, Michael Niedermayer wrote:
> > On Sat, Nov 24, 2012 at 10:08:33PM +0100, Clément Bœsch wrote:
> > > Hi there,
> > > 
> > > I wrote a new prototype for storing text subtitles instead of a custom ASS
> > > line like we currently do. It's trying to be flexible enough to be able to
> > > deal with any kind of text subtitles markups, while being as simple as
> > > possible for our users, but also for decoders and encoders.
> > > 
> > > Of course, we will have to deal with retro compat. The simpler I found
> > > here was to introduce a new AVSubtitleType (SUBTITLE_AST), and we would
> > > use a new field AVSubtitleRect->ast instead of AVSubtitleRect->ass.
> > > 
> > > I used "AST" initially, but it's actually not an AST, so far it's just
> > > kind of a list, feel free to propose a better random name; I took this
> > > name because it expresses the fact that it's an arbitrary structure
> > > layout, and not a data buffer like currently.
> > > 
> > > Anyway, here are the basic structures:
> > > 
> > >     typedef struct AVSubtitleASTChunk {
> > 
> > >         int type;           ///< one of the AVSUBTITLE_AST_SETTING_TYPE_*
> > 
> > enum
> > 
> 
> I didn't know how to name the enum :(

in absence of other suggestions you could use
AVSubtitleChunkType


    
> 
> > 
> > >         int reset;          /**< this chunk restores the setting to the default
> > >                                  value (or disable the previous one in nested
> > >                                  mode) */
> > 
> > what does "previous" refer to here ?
> > it could be previous in the list, previous as in from the last packet
> > or possibly something else
> 
> It's the previous chunk, see the SubRip decoding example I gave below;
> it's all about nested/flat.

yes but it should be documented better in the doxy i think


> 
> > 
> > 
> > >         union {
> > >             char *s;        ///< must be a av_malloc'ed string if string type
> > 
> > >             double d;
> > >             int i;
> > >             int64_t i64;
> > >             uint32_t u32;
> > 
> > isnt double or double+int64 enough for these ?
> > 
> 
> It should yes, but for a semantic usage it's pretty handy to have
> different versions. It doesn't take more memory, so is there any problem
> doing this?

the problem is if the wrong one is used by mistake
theres no type checking done by the compiler, so it could be annoying
to debug.


> 
> > also i was wondering if having a pointer to the string that was
> > parsed could be usefull or allow some simplifications.
> > What i had in mind here would be along the lines of parsing a
> > marked up string and point back into the string where text is needed
> > from it.
> > If for nothing else such a additional field could help debuging
> 
> Why not, but it will require decoders to provide the information in the chunk,
> it's likely it will get a bit laborious for them; here is an example on how a
> decoder queue chunks:
> 
>     } else if (!buffer[1] && strchr("bisu", av_tolower(buffer[0]))) {
>         AVSubtitleASTChunk tag = {.reset = tag_close, .i = !tag_close};
> 
>         switch (av_tolower(buffer[0])) {
>         case 'b': tag.type = AVSUBTITLE_AST_CHUNK_BOLD;         break;
>         case 'i': tag.type = AVSUBTITLE_AST_CHUNK_ITALIC;       break;
>         case 's': tag.type = AVSUBTITLE_AST_CHUNK_STRIKEOUT;    break;
>         case 'u': tag.type = AVSUBTITLE_AST_CHUNK_UNDERLINE;    break;
>         }
>         av_sub_ast_add_chunk(sub, tag);
>      }
> 
> What you would propose instead is a bunch of chunks associated with position
> range in the original event instead of a stack of successive chunks?

yes or it could be just "meta" data for the user app like AVPacket.pos
is almost. That is for that it could be just a int index

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

In fact, the RIAA has been known to suggest that students drop out
of college or go to community college in order to be able to afford
settlements. -- The RIAA
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20121125/6f65708a/attachment.asc>