[FFmpeg-devel] [PATCH] Added STL demuxer and decoder
Clément Bœsch
u at pkh.me
Thu Oct 16 11:16:13 CEST 2014
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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20141016/52358645/attachment.asc>
More information about the ffmpeg-devel
mailing list