[FFmpeg-devel] [PATCH] lavc: add text subtitles decoder.

[Nicolas George]

Beware, twice today you replied to me personally instead of to the list.

Le quartidi 24 vendémiaire, an CCXXI, Clément Bœsch a écrit :
> I was going to change it but I'm actually not really fond of that
> suggestion; it sounds to me like it might be OK to have some ass markup
> and "keep it in its current state", while the escape ass things being
> default is like "hey we're fixing your crap, but you can ignore it".
> 
> Basically, I prefer the user to explicitly disable the stuff we do to
> clean their mess, than making them think they are fixing our supposedly
> wrong behaviour with an option.

We agree that the default behaviour should be the correct one. But
additionally, I consider that "0" is a more default value than "1", or, in
other words, that users should be requested to say "do this wrong thing"
instead of "do not do this right thing".

Also, it should be a global option, not a per-codec implementation.
Hopefully, it will be when the design will be cleaned up.

Maybe the option can be hidden for now, and only exposed either when things
are done cleanly or someone shows they need it. Just suggesting at random.

> > > +        else if (*p != '\r')
> > > +            av_bprint_chars(buf, *p, 1);
> > ???

Forget it, I read it backwards.

> That's what we do everywhere. If we don't, ff_ass_add_rect() will append
> an unterminated ASS line event. It could be factorized in that function at
> some point, but that's relative to another patch.

Well, that is ugly, but we have to live with it.

> Oh and by the way, we must be careful if we do that: at the moment, it
> zero-ends the buffer properly: AFAIK, using av_bprint_chars() doesn't make
> sure the text buffer is null terminated, so if we remove that av_bprintf
> from the decoders, we need to replace it with something like
> av_bprint_chars(buf, '\0', 1), or maybe av_bprintf(buf, "") in case we are
> doing some av_bprint_chars() calls in the loop. Note that this issue might
> exists in some other places…

I believe you read the code wrong: bprint always 0-terminates its buffer
(unless there is no buffer at all). This is done in av_bprint_grow().

> So much work to do on the subtitles… Any help would be welcome :)

I intend to.


> From 230de618f3444f991569756ffa4f760b42d22d40 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Cl=C3=A9ment=20B=C5=93sch?= <ubitux at gmail.com>
> Date: Sun, 14 Oct 2012 03:08:30 +0200
> Subject: [PATCH] lavc: add text subtitles decoder.

No other remarks on the code.

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20121017/519c3dba/attachment.asc>