[FFmpeg-devel] [PATCH] SubRip decoder
Aurelien Jacobs
aurel
Sat Dec 4 15:58:58 CET 2010
On Sun, Nov 28, 2010 at 05:53:04PM -0500, Alexander Strange wrote:
>
> On Nov 28, 2010, at 5:31 PM, Aurelien Jacobs wrote:
>
> > Hi,
> >
> > $subject
> >
> > This SubRip decoder supports all the SubRip features that I know about.
> > It especially support everything tested by the sample files from
> > http://ale5000.altervista.org/subtitles.htm
>
> What generates files with moving display coordinates?
Emacs/vim/... ?
> None of those players support them, and neither does vsfilter:
>
> http://google.com/codesearch/p?hl=en#_g4u1OIsR_M/trunk/src/subtitles/STS.cpp&q=package:vsfilter%20subrip&l=476
>
> Until the day comes when WebSRT is relevant, we might as well be parser compatible with vsfilter and ignore them.
Well, if someone happens to write them in a file, I guess it's not just
for fun. So why should we ignore them instead of trying to render them ?
Anyway, if you insist, I can comment this part.
> > + snprintf(header, sizeof(header),
> > + "[Script Info]\r\n"
> > + "ScriptType: v4.00+\r\n"
> > + "\r\n"
> > + "[V4+ Styles]\r\n"
>
> Watch out for the weird default PlayRes* values.
The default is 384x288.
Do you think I should choose another default value ?
> > + * Copyright (c) 2010 Aurelien Jacobs <aurel at gnuage.org>
>
> Double space.
Yes. So what ?
$ git grep "Copyright.* "|wc -l
85
It seems it is just a copy/paste from the template in COPYING.LGPLv2.1.
> > + case '{':
> > + an += sscanf(in, "{\\an%*1u}%c", &c) == 1;
> > + if ((an != 1 && sscanf(in, "{\\%*[^}]}%n%c", &len, &c) > 0) ||
> > + sscanf(in, "{%*1[CcFfoPSsYy]:%*[^}]}%n%c", &len, &c) > 0) {
> > + in += len - 1;
> > + } else
> > + *out++ = *in;
> > + break;
>
>
> What does this do? I hate reading handwritten parsers...
Comment added.
> > + int i, j, unkown = 0;
>
>
> 'unknown'
Thanks, fixed.
> > +static const char *read_ts(const char *buf, int *ts_start, int *ts_end,
> > + int *x1, int *y1, int *x2, int *y2)
> > +{
> > + int i, hs, ms, ss, he, me, se;
> > +
> > + for (i=0; i<2; i++) {
>
>
> What is the loop for?
It allows supporting standard srt as well as the variant which don't
have a line containing event number just before the timestamp line.
> Please add some testcases.
Added a fate test.
Aurel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: srtdec.diff
Type: text/x-diff
Size: 16803 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20101204/6bd7a847/attachment.diff>
More information about the ffmpeg-devel
mailing list