[FFmpeg-devel] [PATCH] Add PJS subtitles demuxer and decoder.

Clément Bœsch ubitux at gmail.com
Mon Dec 31 00:00:01 CET 2012


On Sun, Dec 30, 2012 at 02:37:50PM +0100, Nicolas George wrote:
[...]
> > diff --git a/libavcodec/codec_desc.c b/libavcodec/codec_desc.c
> > index 2ea5e99..aecee73 100644
> > --- a/libavcodec/codec_desc.c
> > +++ b/libavcodec/codec_desc.c
> > @@ -2423,6 +2423,12 @@ static const AVCodecDescriptor codec_descriptors[] = {
> >          .long_name = NULL_IF_CONFIG_SMALL("JACOsub subtitle"),
> >      },
> >      {
> > +        .id        = AV_CODEC_ID_PJS,
> > +        .type      = AVMEDIA_TYPE_SUBTITLE,
> > +        .name      = "pjs",
> 
> > +        .long_name = NULL_IF_CONFIG_SMALL("PJS subtitle"),
> 
> For consistency with other subtitles, "Phoenix Japanimation Society" should
> be present here, IMHO.
> 

Changed.

[...]
> > diff --git a/libavformat/pjsdec.c b/libavformat/pjsdec.c
> > new file mode 100644
> > index 0000000..b13c39d
> > --- /dev/null
> > +++ b/libavformat/pjsdec.c
> > @@ -0,0 +1,135 @@
> > +/*
> > + * Copyright (c) 2012 Clément Bœsch
> > + *
> > + * 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
> > + * PJS (Phoenix Japanimation Society) subtitles format demuxer
> > + */
> 
> Any reference to the format? Or at least an excerpt, to help understanding
> the logic of the parser?
> 

Added a link, might not be accurate but I've not much to propose. The
FATE sample can help too.

[...]
> > +static int pjs_read_header(AVFormatContext *s)
> > +{
> > +    PJSContext *pjs = s->priv_data;
> > +    AVStream *st = avformat_new_stream(s, NULL);
> > +    int res = 0;
> > +
> > +    if (!st)
> > +        return AVERROR(ENOMEM);
> > +    avpriv_set_pts_info(st, 64, 1, 10);
> > +    st->codec->codec_type = AVMEDIA_TYPE_SUBTITLE;
> > +    st->codec->codec_id   = AV_CODEC_ID_PJS;
> > +
> > +    while (!url_feof(s->pb)) {
> > +        char line[4096];
> > +        char *p = line;
> > +        const int64_t pos = avio_tell(s->pb);
> > +        int len = ff_get_line(s->pb, line, sizeof(line));
> > +        int64_t pts_start;
> > +        int duration;
> > +
> > +        if (!len)
> > +            break;
> > +
> 
> > +        line[strcspn(line, "\r\n")] = 0;
> 
> Same remark as my last mail.
> 

I still love it.

> > +
> > +        pts_start = read_ts(&p, &duration);
> > +        if (pts_start != AV_NOPTS_VALUE) {
> > +            AVPacket *sub;
> > +
> > +            p[strcspn(p, "\"")] = 0;
> > +            sub = ff_subtitles_queue_insert(&pjs->q, p, strlen(p), 0);
> > +            if (!sub)
> > +                return AVERROR(ENOMEM);
> > +            sub->pos = pos;
> > +            sub->pts = pts_start;
> > +            sub->duration = duration;
> > +        }
> 
> I believe this part is missing error report when garbage is found.
> 

Yeah but I haven't seen enough samples to tell if that garbage is not
supposed to be random settings.

[...]

Pushed.

-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20121231/6cbb51e1/attachment.asc>


More information about the ffmpeg-devel mailing list