[FFmpeg-devel] [PATCH 1/4] lavc: correctly set AVSubtitle format for text based subtitles.

[Clément Bœsch]

On Mon, Dec 31, 2012 at 02:48:11PM +0100, Wolfram Gloger wrote:
> Clément Bœsch <ubitux at gmail.com> writes:
> 
> >> Firstly, the AVSubtitle 'format' field appears to be totally unused in
> >> ffmpeg currently, right?
> >> 
> >
> > In ffplay.c:
> > 1979:        if (got_subtitle && sp->sub.format == 0) {
> 
> Ok, missed that.  Thanks.
> The intent there appears to be "check that all rects are bitmap".
> 
> >> If the intent is to define "format == 1" for text subtitles
> >> rather than 'graphics', please add a comment to avcodec.h to
> >> that effect.
> >> 
> >
> > In lavc/avcodec.h:
> > 3463:    uint16_t format; /* 0 = graphics */
> 
> That comment is not explicit if you use format=1 as well.  I would
> propose to make that:
> 
> uint16_t format; /* 0 = all rects are bitmap type, 1 = all rects are text type */
> 
> and just not define the admittedly far-fetched "some bitmap, some text"
> case yet, or (worse IMHO)
> 
> uint16_t format; /* 0 = all rects are bitmap type, 1 = not all rects are bitmap type */
> 
> But, best would be to leave AvSubtitle as it was and change ffplay.c in
> the for (i = 0; i < sp->sub.num_rects; i++) loop, to check for
> rects[i]->type == SUBTITLE_BITMAP.  That appears to be the most correct
> option.
> 
> Will send proper patch if you agree to any of those alternatives.
> 

I believe this whole thing is pretty fragile anyway.

Alternative: transform this format thing into flags:
    0: undefined/loop yourself/fix the decoder
 1<<0: i haz text
 1<<1: i haz bitmap

Whatever you prefer, but that might break later anyway. Feel free to send
any patch.

> > It's not meant to be a definitive solution, it's just "better". Indeed, we
> > could update all the text subtitles decoders to set the format
> > appropriately, but since there are still a lot of things to change for
> > subtitles, I didn't try to think of something more advanced (that code
> > will likely change). Current solution is AFAICT working for all our
> > decoders.
> 
> Yes, but not for my teletext decoder in ffplay :-)
> 

You're working on teletext support? Can you tell us more?

-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20121231/8f7b2235/attachment.asc>