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

[Wolfram Gloger]

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.

> 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 :-)

Regards,
Wolfram.