[FFmpeg-devel] [PATCH v5] Add SUP/PGS subtitle demuxer

[Hendrik Leppkes]

On Mon, Sep 1, 2014 at 10:01 AM, Petri Hintukainen <phintuka at gmail.com> wrote:
> On ma, 2014-09-01 at 00:34 +0200, wm4 wrote:
>> ---
>> Use AV_RB16 instead of memcpy.
>> Don't use AVPROBE_SCORE_EXTENSION.
>> ---
>>  libavformat/Makefile     |   1 +
>>  libavformat/allformats.c |   1 +
>>  libavformat/supdec.c     | 107 +++++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 109 insertions(+)
>>  create mode 100644 libavformat/supdec.c
>>
>> diff --git a/libavformat/Makefile b/libavformat/Makefile
>> index 3d124fb..b4965fe 100644
>> --- a/libavformat/Makefile
>> +++ b/libavformat/Makefile
>> @@ -405,6 +405,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_DEMUXER)               += supdec.o
>>  OBJS-$(CONFIG_SWF_DEMUXER)               += swfdec.o swf.o
>>  OBJS-$(CONFIG_SWF_MUXER)                 += swfenc.o swf.o
>>  OBJS-$(CONFIG_TAK_DEMUXER)               += takdec.o apetag.o img2.o rawdec.o
>> diff --git a/libavformat/allformats.c b/libavformat/allformats.c
>> index 8f70c4b..e6c0e5f 100644
>> --- a/libavformat/allformats.c
>> +++ b/libavformat/allformats.c
>> @@ -280,6 +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(SWF,              swf);
>>      REGISTER_DEMUXER (TAK,              tak);
>>      REGISTER_MUXER   (TEE,              tee);
>> diff --git a/libavformat/supdec.c b/libavformat/supdec.c
>> new file mode 100644
>> index 0000000..3726c97
>> --- /dev/null
>> +++ b/libavformat/supdec.c
>> @@ -0,0 +1,107 @@
>> +/*
>> + * 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_read_header(AVFormatContext *s)
>> +{
>> +    AVStream *st = avformat_new_stream(s, NULL);
>> +    if (!st)
>> +        return AVERROR(ENOMEM);
>> +    st->codec->codec_type = AVMEDIA_TYPE_SUBTITLE;
>> +    st->codec->codec_id = AV_CODEC_ID_HDMV_PGS_SUBTITLE;
>> +    avpriv_set_pts_info(st, 32, 1, 90000);
>> +
>> +    return 0;
>> +}
>> +
>> +static int sup_read_packet(struct AVFormatContext *s, AVPacket *pkt)
>> +{
>> +    int64_t pts, pos;
>> +    int ret;
>> +
>> +    pos = avio_tell(s->pb);
>> +
>> +    if (avio_rb16(s->pb) != SUP_PGS_MAGIC)
>> +        return avio_feof(s->pb) ? AVERROR_EOF : AVERROR_INVALIDDATA;
>> +
>> +    pts = avio_rb32(s->pb);
>> +    avio_rb32(s->pb); /* discard DTS (usually 0, and useless) */
>
> Is there some reason to discard DTS ? I would keep DTS when it is
> present, even if ffmpeg decoder does not use it. There are several
> reasons why DTS should be kept:
>

Setting a DTS of 0 for all frames could result in serious problems.
Shouldn't DTS and PTS be the same for subtitles anyway, considering
they are not re-ordered?

- Hendrik