[FFmpeg-devel] [PATCH v2] Add SUP/PGS subtitle muxer

Petri Hintukainen phintuka at gmail.com
Wed Oct 1 11:48:26 CEST 2014


On ti, 2014-09-30 at 15:56 +0200, wm4 wrote:
> On Tue, 30 Sep 2014 11:01:17 +0300
> phintuka at gmail.com wrote:
> 
> > From: Petri Hintukainen <phintuka at users.sourceforge.net>
> > 
> > Fixes ticket #2208
> > ---
> >  Changelog                |  2 +-
> >  doc/general.texi         |  2 +-
> >  libavformat/Makefile     |  1 +
> >  libavformat/allformats.c |  2 +-
> >  libavformat/supenc.c     | 96 ++++++++++++++++++++++++++++++++++++++++++++++++
> >  5 files changed, 100 insertions(+), 3 deletions(-)
> >  create mode 100644 libavformat/supenc.c
> > 
> > diff --git a/Changelog b/Changelog
> > index 039d1ca..f171c17 100644
> > --- a/Changelog
> > +++ b/Changelog
> > @@ -3,7 +3,7 @@ releases are sorted from youngest to oldest.
> >  
> >  version <next>:
> >  - HEVC/H.265 RTP payload format (draft v6) packetizer
> > -- SUP/PGS subtitle demuxer
> > +- SUP/PGS subtitle demuxer and muxer
> >  
> >  version 2.4:
> >  - Icecast protocol
> > diff --git a/doc/general.texi b/doc/general.texi
> > index 2252f7b..b848e7e 100644
> > --- a/doc/general.texi
> > +++ b/doc/general.texi
> > @@ -454,7 +454,7 @@ library:
> >  @item Sony Wave64 (W64)         @tab X @tab X
> >  @item SoX native format         @tab X @tab X
> >  @item SUN AU format             @tab X @tab X
> > - at item SUP raw PGS subtitles     @tab   @tab X
> > + at item SUP raw PGS subtitles     @tab X @tab X
> >  @item Text files                @tab   @tab X
> >  @item THP                       @tab   @tab X
> >      @tab Used on the Nintendo GameCube.
> > diff --git a/libavformat/Makefile b/libavformat/Makefile
> > index 86064ea..7385b67 100644
> > --- a/libavformat/Makefile
> > +++ b/libavformat/Makefile
> > @@ -407,6 +407,7 @@ OBJS-$(CONFIG_SRT_MUXER)                 += srtenc.o
> >  OBJS-$(CONFIG_STR_DEMUXER)               += psxstr.o
> >  OBJS-$(CONFIG_SUBVIEWER1_DEMUXER)        += subviewer1dec.o subtitles.o
> >  OBJS-$(CONFIG_SUBVIEWER_DEMUXER)         += subviewerdec.o subtitles.o
> > +OBJS-$(CONFIG_SUP_MUXER)                 += supenc.o
> >  OBJS-$(CONFIG_SUP_DEMUXER)               += supdec.o
> >  OBJS-$(CONFIG_SWF_DEMUXER)               += swfdec.o swf.o
> >  OBJS-$(CONFIG_SWF_MUXER)                 += swfenc.o swf.o
> > diff --git a/libavformat/allformats.c b/libavformat/allformats.c
> > index d54ed9b..0b64355 100644
> > --- a/libavformat/allformats.c
> > +++ b/libavformat/allformats.c
> > @@ -280,7 +280,7 @@ void av_register_all(void)
> >      REGISTER_DEMUXER (STR,              str);
> >      REGISTER_DEMUXER (SUBVIEWER1,       subviewer1);
> >      REGISTER_DEMUXER (SUBVIEWER,        subviewer);
> > -    REGISTER_DEMUXER (SUP,              sup);
> > +    REGISTER_MUXDEMUX(SUP,              sup);
> >      REGISTER_MUXDEMUX(SWF,              swf);
> >      REGISTER_DEMUXER (TAK,              tak);
> >      REGISTER_MUXER   (TEE,              tee);
> > diff --git a/libavformat/supenc.c b/libavformat/supenc.c
> > new file mode 100644
> > index 0000000..f5f6b58
> > --- /dev/null
> > +++ b/libavformat/supenc.c
> > @@ -0,0 +1,96 @@
> > +/*
> > + * SUP muxer
> > + * Copyright (c) 2014 Petri Hintukainen <phintuka at users.sourceforge.net>
> > + *
> > + * 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
> > + */
> > +
> > +#include "avformat.h"
> > +#include "internal.h"
> > +#include "libavutil/intreadwrite.h"
> > +
> > +#define SUP_PGS_MAGIC 0x5047 /* "PG", big endian */
> > +
> > +static int sup_write_packet(AVFormatContext *s, AVPacket *pkt)
> > +{
> > +    uint8_t *data = pkt->data;
> > +    size_t size = pkt->size;
> > +    uint32_t pts = 0, dts = 0;
> > +
> > +    if (pkt->pts != AV_NOPTS_VALUE) {
> > +        pts = (uint32_t)pkt->pts;
> > +    }
> > +    if (pkt->dts != AV_NOPTS_VALUE) {
> > +        dts = (uint32_t)pkt->dts;
> > +    }
> > +
> > +    /*
> > +      Split frame to segments.
> > +      mkvmerge stores multiple segments in one frame.
> > +    */
> > +    while (size > 2) {
> > +        size_t len = AV_RB16(data + 1) + 3;
> > +
> > +        if (len > size) {
> > +            av_log(s, AV_LOG_ERROR, "Not enough data, skipping %d bytes\n",
> > +                     (int)size);
> > +            return AVERROR_INVALIDDATA;
> > +        }
> > +
> > +        /* header */
> > +        avio_wb16(s->pb, SUP_PGS_MAGIC);
> > +        avio_wb32(s->pb, pts);
> > +        avio_wb32(s->pb, dts);
> > +
> > +        avio_write(s->pb, data, len);
> > +
> > +        data += len;
> > +        size -= len;
> > +    }
> 
> Ah, I see now why you wanted to add a parser. But yeah, a separate
> parser would probably be overkill in this case.

Agreed.
Maybe parser could also generate missing timestamps correctly.

> Are you sure the additional segments should have the same PTS and DTS
> as the initial segment? Well, they probably have to, I'm not sure.

No, they should not, it is PTS of the first segment. But all other
timestamps have been already lost. This is how mkvextract does, except
that it leaves DTS to 0.

Maybe I could set DTS to 0 when PTS == DTS, that should prevent leaking
incorrectly generated DTS outside of ffmpeg. If PTS and DTS are really
the same, missing DTS in stream implies DTS = PTS anyway ?

Something like

        if (pts == dts || type == 0x80 /* END */ || type == 0x14 /* PDS
*/) {
            avio_wb32(s->pb, 0);
        } else {
            avio_wb32(s->pb, dts);
        }

(PDS and END have no DTS)

I could also set unknown PTS to 0, but I don't know how well it would
work with other tools. I could also analyze segment contents and create
timestamps, but that would be quite much code (segments should be partly
decoded). Of course, such code could be shared with possible encoder so
it wouldn't be just waste of time :)

But, such checks and fixes should really not be muxer's job.

> > +
> > +    if (size > 0) {
> > +        av_log(s, AV_LOG_ERROR, "Skipping %d bytes after last segment in frame\n",
> > +                 (int)size);
> > +        return AVERROR_INVALIDDATA;
> > +    }
> > +
> > +    return 0;
> > +}
> > +
> > +static int sup_write_header(AVFormatContext *s)
> > +{
> > +    if (s->nb_streams != 1) {
> > +        av_log(s, AV_LOG_ERROR, "%s files have exactly one stream\n",
> > +               s->oformat->name);
> > +        return AVERROR(EINVAL);
> > +    }
> > +
> > +    avpriv_set_pts_info(s->streams[0], 32, 1, 90000);
> > +
> > +    return 0;
> > +}
> > +
> > +AVOutputFormat ff_sup_muxer = {
> > +    .name           = "sup",
> > +    .long_name      = NULL_IF_CONFIG_SMALL("raw HDMV Presentation Graphic Stream subtitles"),
> > +    .extensions     = "sup",
> > +    .mime_type      = "application/x-pgs",
> > +    .subtitle_codec = AV_CODEC_ID_HDMV_PGS_SUBTITLE,
> > +    .write_header   = sup_write_header,
> > +    .write_packet   = sup_write_packet,
> > +    .flags          = AVFMT_VARIABLE_FPS | AVFMT_TS_NONSTRICT,
> > +};
> 
> In general, looks good to me.
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel




More information about the ffmpeg-devel mailing list