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

[Clément Bœsch]

On Mon, Dec 31, 2012 at 01:30:57PM +0100, Wolfram Gloger wrote:
> Hi,
> 
> > On Wed, Nov 28, 2012 at 11:37:15AM +0100, Nicolas George wrote:
> >> Le septidi 7 frimaire, an CCXXI, Clément Bœsch a écrit :
> >> > ---
> >> >  libavcodec/utils.c | 1 +
> >> >  1 file changed, 1 insertion(+)
> >> 
> >> I do not think it can cause any problem, but this AVSubtitle structure is
> >> very ugly. So probably ok.
> 
> Clément Bœsch <ubitux at gmail.com> writes:
> 
> > Finally applied, thanks.
> 
> libavcodec/utils.c
>          ret = avctx->codec->decode(avctx, sub, got_sub_ptr, &tmp);
> +        sub->format = sub->num_rects && sub->rects[0]->ass;
> 
> 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) {

> 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 */

> But I think this _will_ cause problems because
> 
> (a) I see nothing that forbids a decoder generating graphics and _text_
>     rectangles in the same AVSubtitle.  Why should we forbid that?
> 

That's something we should consider if we end up doing so, but at the
moment there is no such thing.

> (b) If anything, the codec->decode function should set sub->format;
>     otherwise, how would a completely new decoder set text format?
>     ASS markup may be good today but I still think it's not the least
>     common denominator and also it may not be possible for all future
>     text decoders.

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.

-- 
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/6a6db522/attachment.asc>