[FFmpeg-devel] [PATCH] movtextdec: MPEG4 Part 17 Timed Text Decoder.

Nicolas George nicolas.george at normalesup.org
Thu Jul 5 21:27:53 CEST 2012


L'octidi 18 messidor, an CCXX, Philip Langdale a écrit :
> Well, there are definitely samples with \n or \r\n in the middle of a
> line. In practice, I could just drop the \r and not reset line_start,
> but semantically, that's what a \r is so I put it in.

The semantic of \r is unclear at best, thanks to some people unable to see
the difference between a text file and control codes for a line printer.

> I implemented this behaviour based on what the other decoders in ffmpeg
> do (srt, realtext, etc). This is not per the subtitle format, but seems
> to be what we expect a decoder to do. If not desired, I have no problem
> taking it out.

> Again, I'm just following the pattern. ASS represents line-endings
> within
> the subtitles with "\N" as above, but all our other decoders
> terminate the
> sample going to ASS with \r\n.

Well, there certainly is need for cleanup here, it looks like a lot of
duplicated code. But I will not take it as a reason to block your patch. But
I would also say it is ok to just drop it and put the text as is without any
sanitizing.

> 2048 is the recommended maximum line length in the spec, and is
> consistent with
> the arbitrary limits enforced in the other decoders.

IMHO, a decoder should not enforce a recommended arbitrary limit, especially
when there is a structural limit not so much bigger (and bprint will handle
it efficiently): people can encounter files that overflow the limit, it
would be nice if they can decode them.

It would be different for an encoder of course; it would probably depend on
the value of AVCodecContext.strict_std_compliance.

Therefore, I suggest you replace 2048 by -1 (it means virtually unlimited
for bprint).

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/20120705/a76b4087/attachment.asc>


More information about the ffmpeg-devel mailing list