[FFmpeg-devel] [PATCH] lavf: JSON captions demuxer.

[Nicolas George]

Le decadi 30 brumaire, an CCXXI, Clément Bœsch a écrit :
> Please add an entry in general.texi.

Done.

> > + at url{javascript:(function()%7bd%3Dwindow.open%28%22%22%2C%22sub%22%2C%22width%3D256%2Cheight%3D512%2Cresizable%3Dyes%2Cscrollbars%3Dyes%22%29.document%3B%20l%3Ddocument.getElementById%28%22languageCode%22%29.getElementsByTagName%28%22option%22%29%3B%20for%28i%3D1%3Bi%3Cl.length%3Bi++%29%7B%20d.body.appendChild%28p%3Dd.createElement%28%22p%22%29%29%3B%20p.appendChild%28a%3Dd.createElement%28%22a%22%29%29%3B%20a.appendChild%28d.createTextNode%28l%5Bi%5D.textContent%29%29%3B%20a.href%3D%22http%3A//www.ted.com/talks/subtitles/id/%22%20+%20talkID+%22/lang/%22+l%5Bi%5D.value%3B%20%7D%20%7d)();void%200, TED Talks captions}.
> Erk…

I agree.

> Can't you point out a library like libquvi (if it supports this) or
> something for such task?

I do not think anything already exists: this is very specific.

>			   Or maybe just add a script in tools/.

It is a bookmarklet: people are supposed to store it in their bookmarks, and
then activate it when they are on the relevant page. I can create
tools/bookmarklets.html with a readable version of the javascript code, but
it is the best I can think of.

> "15000 (15s)"

Changed.

> > +ffmpeg -i http://www.ted.com/talks/subtitles/id/1/lang/en talk1-en.srt
> Since it's an example, maybe use a more uncommon id than 1, such 685?

I do not want to point to an arbitrary talk, it may be taken as a message,
and I could not find any talk about anything remotely related to FFmpeg.

> Note: I wonder if this kind of code couldn't be shared with some other
> subtitles demuxers, such as WebVTT.

Probably. This is quite common for text parsers.

> > +        if ((((unsigned)*cur_byte) | 32) - 'a' <= 'z' - 'a')
> I may be missing something obvious, but is the -'a' necessary?

You always have "a <= b  iif  a-c <= b-c" for mathematical integers, but
that does not stand for computer integers, because of overflows. The
unsigned cast is there exactly for that:

	(unsigned)(a - b) <= c

tests that 0 <= a - b <= c in one single test. I rewrote it using a macro,
hopefully more readable.

> Hopefully there is no TED talks with SubRip markup :)

That would be unlikely.

> nit: trailing \n

Fixed.

> Too bad we have to parse the whole file two times. I'm a bit concerned by
> the time this probing function will take; it's likely it will slow down
> auto detection of other formats. I'd suggest to just browse the probe data
> and check if the stream makes sense. You could do this when second
> parse_file parameter is NULL for instance.

I completely replaced the probe by something much simpler.

> please make this a bit more readable

More or less done.

> The rest looks OK

New patch incoming.

> "tedsubs" or "tedcaptions", with or without '_' would be nice yes. I
> believe we can find several other js based subtitles, so I think it's
> better with such name to avoid renaming it when adding a new one.

tedcaptions it is.

Thanks for the review.

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/20121121/fb78f02f/attachment.asc>