[FFmpeg-devel] [PATCH 5/7] avcodec/ass_split: extend recognized fields in ASS splitter

[Clément Bœsch]

On Tue, Oct 14, 2014 at 08:43:06PM +0200, James Darnley wrote:
> On 2014-10-11 19:59, Clément Bœsch wrote:
> 
> > diff --git a/libavcodec/ass_split.h b/libavcodec/ass_split.h
> > index 06c1ce3..c912252 100644
> > --- a/libavcodec/ass_split.h
> > +++ b/libavcodec/ass_split.h
> > @@ -41,13 +41,28 @@ typedef struct {
> >      char *font_name;      /**< font face (case sensitive) */
> >      int   font_size;      /**< font height */
> >      int   primary_color;  /**< color that a subtitle will normally appear in */
> > +    int   secondary_color;
> > +    int   outline_color;  /**< color for outline in ASS, called tertiary in SSA */
> >      int   back_color;     /**< color of the subtitle outline or shadow */
> >      int   bold;           /**< whether text is bold (1) or not (0) */
> >      int   italic;         /**< whether text is italic (1) or not (0) */
> >      int   underline;      /**< whether text is underlined (1) or not (0) */
> > +    int   strikeout;
> > +    float scalex;
> > +    float scaley;
> > +    float spacing;
> > +    float angle;
> > +    int   border_style;
> > +    float outline;
> > +    float shadow;
> >      int   alignment;      /**< position of the text (left, center, top...),
> >                                 defined after the layout of the numpad
> >                                 (1-3 sub, 4-6 mid, 7-9 top) */
> > +    int   margin_l;
> > +    int   margin_r;
> > +    int   margin_v;
> > +    int   alpha_level;
> > +    int   encoding;
> >  } ASSStyle;
> >  
> >  /**
> > @@ -58,6 +73,11 @@ typedef struct {
> >      int   start;    /**< start time of the dialog in centiseconds */
> >      int   end;      /**< end time of the dialog in centiseconds */
> >      char *style;    /**< name of the ASSStyle to use with this dialog */
> > +    char *name;
> > +    int   margin_l;
> > +    int   margin_r;
> > +    int   margin_v;
> > +    char *effect;
> >      char *text;     /**< actual text which will be displayed as a subtitle,
> >                           can include style override control codes (see
> >                           ff_ass_split_override_codes()) */
> > 
> 
> You haven't put any comments describing the new fields (okay, you have
> done 1).  I don't want to block the patch but it is better to comment
> things when they're added so that one doesn't go work on something else
> and then forget.
> 

Yeah right, but unfortunately, these fields are just a direct translation
of the named column of the format. For a lot of them, I don't actually
know what are the effects on the rendering. Here, I'm just making sure
they are extracted in their appropriate fields.

I could take the description from the "specs", but it's unfortunately a
very old .doc which is said to be wrong and incomplete in a lot of places.

Anyone willing to know their meanings will probably have to look at
VSFilter or libass code to see how they're rendered.

So yeah basically, each field in the struct is just mapped to a column of
the ASS format, there is not much to comment about it I believe, and it's
actually pretty straightforward when you have a light knowledge of the
format itself.

> Other than that, it looks fine.
> 


-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20141014/4dc666e4/attachment.asc>