[FFmpeg-devel] [PATCH]Basic XSUB encoder (take 2)

[Björn Axelsson]

On Tue, 3 Feb 2009, Reimar D?ffinger wrote:

> On Tue, Feb 03, 2009 at 10:37:34AM +0100, Bj?rn Axelsson wrote:
> > On Tue, 3 Feb 2009, Reimar D?ffinger wrote:
> > > > +/* For unknown reasons we pad the subtitles with two pixels on either side */
> > > > +#define MARGIN 2
> > >
> > > Huh? I sure hope you know why you are doing this? Maybe not why you have
> > > to do that, but it has to have some bad effect if you don't do it?
> >
> > Unfortunately I don't know why I'm doing it. The original patch from DivX
> > said only "// 2 pixels required on either side of subtitle", and my naive
> > assumption is that they should have a good reason.
>
> Ah, ok then. It might be because the subtitles are rendered via hardware
> overlay, some players might have trouble rendering stuff at the borders
> of the overlay.

One would think that a player that have trouble with overlay borders would
know to add borders before rendering stuff.

> > Do you suggest that I remove the padding unless I can see any
> > problems with the few players I have?
>
> Nah, but making it work with MARGIN == 0 would be nice IMO.

Ok.

> > > > +static inline int getcolor(const uint8_t *row, int w, int x)
> > > > +{
> > > > +    if (x >= MARGIN && x < w+MARGIN)
> > > > +        return row[x-MARGIN] & 3;
> > > > +    else
> > > > +        return 0;
> > >
> > > Hm. That's not exactly fast. While I don't know a solution, this also
> > > assumes that index 0 is a transparent colour...
> >
> > According to the multimediawiki and the XSUB decoder, index 0 is always
> > transparent (XSUB doesn't support alpha).
>
> Ok, as in my suggestion I would prefer it if you used a define (whether
> MARGIN_COLOR or TRANSPARENT_COLOR I do not really care about).
> I would like to point out that to my knowledge for DVD subs it is not
> always index 0 that is transparent, and the XSUB encoder will horribly
> fail for those then.
> Some checking (and printing a warning or failing) or even reordering of
> the palette may be a nice feature, I won't insist though (and it can be
> added after the first version was applied)...

I hadn't thought about that. I guess it falls a bit into the same category
as palette reduction.

I'll add a transparency check and a warning to the next version.
You wouldn't happen to have a sample of DVB subs where index 0 isn't
transparent?

-- 
Bj?rn Axelsson