On Thu, Oct 16, 2014 at 02:26:30AM +0530, Eejya Singh wrote: > From: Eejya <singh.ee...@gmail.com> > > --- > Changelog | 1 + > doc/general.texi | 1 + > libavcodec/Makefile | 1 + > libavcodec/allcodecs.c | 1 + > libavcodec/avcodec.h | 1 + > libavcodec/codec_desc.c | 7 +++ > libavcodec/textdec.c | 18 ++++++- > libavformat/Makefile | 1 + > libavformat/allformats.c | 1 + > libavformat/stldec.c | 136 > +++++++++++++++++++++++++++++++++++++++++++++++ > 10 files changed, 167 insertions(+), 1 deletion(-) > create mode 100644 libavformat/stldec.c >
Please reply in the same thread when iterating the patch, it's easier for us to track. > diff --git a/Changelog b/Changelog > index b59058b..9626d4a 100644 > --- a/Changelog > +++ b/Changelog > @@ -5,6 +5,7 @@ version <next>: > - HEVC/H.265 RTP payload format (draft v6) packetizer > - SUP/PGS subtitle demuxer > - ffprobe -show_pixel_formats option > +- STL subtitle demuxer and decoder > > version 2.4: > - Icecast protocol > diff --git a/doc/general.texi b/doc/general.texi > index 2252f7b..681c5c9 100644 > --- a/doc/general.texi > +++ b/doc/general.texi > @@ -1032,6 +1032,7 @@ performance on systems without hardware floating point > support). > @item RealText @tab @tab X @tab @tab X > @item SAMI @tab @tab X @tab @tab X > @item SSA/ASS @tab X @tab X @tab X @tab X > +@item STL @tab @tab X @tab @tab X Maybe mention Spruce here [...] > diff --git a/libavformat/stldec.c b/libavformat/stldec.c > new file mode 100644 > index 0000000..9e79720 > --- /dev/null > +++ b/libavformat/stldec.c > @@ -0,0 +1,136 @@ > +/* > + * Copyright (c) 2014 Eejya Singh > + * > + * This file is part of FFmpeg. > + * > + * FFmpeg is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2.1 of the License, or (at your option) any later version. > + * > + * FFmpeg is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with FFmpeg; if not, write to the Free Software > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 > USA > + */ > + > +/** > + * @file > + * STL subtitles format demuxer > + */ Maybe add a "@see" entry with a link to the specifications here (git grep '@see http' for examples) Please also add a line breaks after the "*/" > +#include "avformat.h" > +#include "internal.h" > +#include "subtitles.h" > +#include "libavutil/intreadwrite.h" > +#include "libavutil/avstring.h" > + > +typedef struct { > + FFDemuxSubtitlesQueue q; > +} STLContext; > + > +static int stl_probe(AVProbeData *p) > +{ > + char c; > + const unsigned char *ptr = p->buf; > + > + if (AV_RB24(ptr) == 0xEFBBBF) > + ptr += 3; /* skip UTF-8 BOM */ > + while(*ptr=='\r' || *ptr=='\n' || *ptr=='$' || !strncmp(ptr,"//",2)) > + ptr+=ff_subtitles_next_line(ptr); Here and several times later in the file, the style is wrong. Please check how spaces are supposed to be. See http://ffmpeg.org/developer.html#toc-Coding-Rules-1 > + if (sscanf(ptr, "%*d:%*d:%*d:%*d , %*d:%*d:%*d:%*d , %c", &c) == 1) > + return AVPROBE_SCORE_MAX; > + return 0; > +} One more line break here to separate the functions > +static int64_t get_pts(char **buf, int *duration) > +{ > + int hh1, mm1, ss1, ms1; > + int hh2, mm2, ss2, ms2; > + int len=0; > + if (sscanf(*buf, "%2d:%2d:%2d:%2d , %2d:%2d:%2d:%2d , %n", > + &hh1, &mm1, &ss1, &ms1, > + &hh2, &mm2, &ss2, &ms2, &len) >= 8 && len>0) { > + int64_t start = (hh1*3600LL + mm1*60LL + ss1) * 100LL + ms1; > + int64_t end = (hh2*3600LL + mm2*60LL + ss2) * 100LL + ms2; > + *duration = end - start; > + *buf+=len; > + return start; > + } > + return AV_NOPTS_VALUE; The indent of this function is wrong [...] Rest of the code LGTM. It would be nice if you could add a test for this. Pick a .stl file, try to add weird cases like timestamps with and without spaces, with an UTF-8 BOM, with random line breaks, use of '|', and with style changes (for later). Not need for a huge file, just a sample that covers most of the code. Move your samples in fate-suite/sub/, then add an entry in tests/fate/subtitles.mak and use make fate-sub-stl GEN=1 so it creates the reference file. If we agree with the test, we'll upload the sample. Thanks. -- Clément B.
pgpmLB_Lz_KHn.pgp
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel