[FFmpeg-devel] [PATCH] Added STL demuxer and decoder

[Eejya Singh]

Please find the revised patch for the STL demuxer and decoder attached. I
am experiencing some network problems while rsyncing from the fate-server
so I couldn't add the test .stl file.

On Thu, Oct 16, 2014 at 2:46 PM, Clément Bœsch <u at pkh.me> wrote:

> On Thu, Oct 16, 2014 at 02:26:30AM +0530, Eejya Singh wrote:
> > From: Eejya <singh.eejya at 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
>
> > + at 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.
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>


-- 
Eejya Singh
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Added-STL-demuxer-and-decoder.patch
Type: text/x-patch
Size: 10658 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20141017/3d7e9203/attachment.bin>