diff mbox series

[FFmpeg-devel,1/3] lavc/pgs_frame_merge_bsf: add bsf to merge PGS segments

Message ID 20200416225736.166440-1-jstebbins@jetheaddev.com
State Superseded
Headers show
Series [FFmpeg-devel,1/3] lavc/pgs_frame_merge_bsf: add bsf to merge PGS segments | expand

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

John Stebbins April 16, 2020, 10:57 p.m. UTC
Required to remux m2ts to mkv
---
 libavcodec/Makefile              |   1 +
 libavcodec/bitstream_filters.c   |   1 +
 libavcodec/pgs_frame_merge_bsf.c | 164 +++++++++++++++++++++++++++++++
 3 files changed, 166 insertions(+)
 create mode 100644 libavcodec/pgs_frame_merge_bsf.c

Comments

John Stebbins April 16, 2020, 10:59 p.m. UTC | #1
On Thu, 2020-04-16 at 16:57 -0600, John Stebbins wrote:
> Required to remux m2ts to mkv
> ---
>  libavcodec/Makefile              |   1 +
>  libavcodec/bitstream_filters.c   |   1 +
>  libavcodec/pgs_frame_merge_bsf.c | 164
> +++++++++++++++++++++++++++++++
>  3 files changed, 166 insertions(+)
>  create mode 100644 libavcodec/pgs_frame_merge_bsf.c
> 

Ugh!  I forgot the version bump.  Will get on that...

> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> index c1c9a44f2b..c7d8fbebaa 100644
> --- a/libavcodec/Makefile
> +++ b/libavcodec/Makefile
> @@ -1110,6 +1110,7 @@ OBJS-$(CONFIG_MP3_HEADER_DECOMPRESS_BSF)  +=
> mp3_header_decompress_bsf.o \
>  OBJS-$(CONFIG_MPEG2_METADATA_BSF)         += mpeg2_metadata_bsf.o
>  OBJS-$(CONFIG_NOISE_BSF)                  += noise_bsf.o
>  OBJS-$(CONFIG_NULL_BSF)                   += null_bsf.o
> +OBJS-$(CONFIG_PGS_FRAME_MERGE_BSF)        += pgs_frame_merge_bsf.o
>  OBJS-$(CONFIG_PRORES_METADATA_BSF)        += prores_metadata_bsf.o
>  OBJS-$(CONFIG_REMOVE_EXTRADATA_BSF)       += remove_extradata_bsf.o
>  OBJS-$(CONFIG_TEXT2MOVSUB_BSF)            += movsub_bsf.o
> diff --git a/libavcodec/bitstream_filters.c
> b/libavcodec/bitstream_filters.c
> index 6b5ffe4d70..92619225f0 100644
> --- a/libavcodec/bitstream_filters.c
> +++ b/libavcodec/bitstream_filters.c
> @@ -49,6 +49,7 @@ extern const AVBitStreamFilter
> ff_mpeg4_unpack_bframes_bsf;
>  extern const AVBitStreamFilter ff_mov2textsub_bsf;
>  extern const AVBitStreamFilter ff_noise_bsf;
>  extern const AVBitStreamFilter ff_null_bsf;
> +extern const AVBitStreamFilter ff_pgs_frame_merge_bsf;
>  extern const AVBitStreamFilter ff_prores_metadata_bsf;
>  extern const AVBitStreamFilter ff_remove_extradata_bsf;
>  extern const AVBitStreamFilter ff_text2movsub_bsf;
> diff --git a/libavcodec/pgs_frame_merge_bsf.c
> b/libavcodec/pgs_frame_merge_bsf.c
> new file mode 100644
> index 0000000000..d1bb3a60c2
> --- /dev/null
> +++ b/libavcodec/pgs_frame_merge_bsf.c
> @@ -0,0 +1,164 @@
> +/*
> + * Copyright (c) 2020 John Stebbins <jstebbins.hb@gmail.com>
> + *
> + * 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
> + * This bitstream filter merges PGS subtitle packets containing
> incomplete
> + * set of segments into a single packet
> + *
> + * Packets already containing a complete set of segments will be
> passed through
> + * unchanged.
> + */
> +
> +#include "avcodec.h"
> +#include "bsf.h"
> +#include "libavutil/intreadwrite.h"
> +
> +enum PGSSegmentType {
> +    PALETTE_SEGMENT      = 0x14,
> +    OBJECT_SEGMENT       = 0x15,
> +    PRESENTATION_SEGMENT = 0x16,
> +    WINDOW_SEGMENT       = 0x17,
> +    DISPLAY_SEGMENT      = 0x80,
> +};
> +
> +typedef struct PGSMergeContext {
> +    AVPacket *buffer_pkt, *in;
> +    int presentation_found;
> +} PGSMergeContext;
> +
> +static void frame_merge_flush(AVBSFContext *bsf)
> +{
> +    PGSMergeContext *ctx = bsf->priv_data;
> +
> +    av_packet_unref(ctx->in);
> +    av_packet_unref(ctx->buffer_pkt);
> +}
> +
> +static int frame_merge_filter(AVBSFContext *bsf, AVPacket *out)
> +{
> +    PGSMergeContext *ctx = bsf->priv_data;
> +    AVPacket *in = ctx->in, *pkt = ctx->buffer_pkt;
> +    int ret, i, size, pos, display = 0;
> +
> +    if (!in->data) {
> +        ret = ff_bsf_get_packet_ref(bsf, in);
> +        if (ret < 0)
> +            return ret;
> +    }
> +    if (!in->size) {
> +        av_packet_unref(in);
> +        return AVERROR(EAGAIN);
> +    }
> +
> +    // Validate packet data and find display_end segment
> +    size = in->size;
> +    i = 0;
> +    while (i + 3 <= in->size) {
> +        uint8_t segment_type;
> +        int segment_len;
> +
> +        segment_type = in->data[i];
> +        segment_len  = AV_RB16(in->data + i + 1) + 3;
> +        if (i + segment_len > in->size) // Invalid data
> +            break;
> +        if (segment_type == PRESENTATION_SEGMENT && !ctx-
> >presentation_found) {
> +            int object_count;
> +            ctx->presentation_found = 1;
> +            ret = av_packet_copy_props(pkt, in);
> +            if (ret < 0)
> +                goto fail;
> +            object_count  = in->data[i + 13];
> +            pkt->flags = !!object_count * AV_PKT_FLAG_KEY;
> +        }
> +        i += segment_len;
> +        if (segment_type == DISPLAY_SEGMENT) {
> +            size = display = i;
> +            break;
> +        }
> +    }
> +    if (display && pkt->size == 0 && size == in->size) { //
> passthrough
> +        av_packet_move_ref(out, in);
> +        return 0;
> +    }
> +    if ((!display && i != in->size) || size > in->size) {
> +        av_log(bsf, AV_LOG_WARNING, "Failed to parse PGS
> segments.\n");
> +        // force output what we have
> +        display = size = in->size;
> +    }
> +
> +    pos = pkt->size;
> +    ret = av_grow_packet(pkt, size);
> +    if (ret < 0)
> +        goto fail;
> +    memcpy(pkt->data + pos, in->data, size);
> +
> +    if (size == in->size)
> +        av_packet_unref(in);
> +    else {
> +        in->data += size;
> +        in->size -= size;
> +    }
> +
> +    if (display) {
> +        ctx->presentation_found = 0;
> +        av_packet_move_ref(out, pkt);
> +        return 0;
> +    }
> +    return AVERROR(EAGAIN);
> +
> +fail:
> +    frame_merge_flush(bsf);
> +    return ret;
> +}
> +
> +static int frame_merge_init(AVBSFContext *bsf)
> +{
> +    PGSMergeContext *ctx = bsf->priv_data;
> +
> +    ctx->in  = av_packet_alloc();
> +    ctx->buffer_pkt = av_packet_alloc();
> +    if (!ctx->in || !ctx->buffer_pkt)
> +        return AVERROR(ENOMEM);
> +
> +    return 0;
> +}
> +
> +static void frame_merge_close(AVBSFContext *bsf)
> +{
> +    PGSMergeContext *ctx = bsf->priv_data;
> +
> +    av_packet_free(&ctx->in);
> +    av_packet_free(&ctx->buffer_pkt);
> +}
> +
> +static const enum AVCodecID frame_merge_codec_ids[] = {
> +    AV_CODEC_ID_HDMV_PGS_SUBTITLE, AV_CODEC_ID_NONE,
> +};
> +
> +const AVBitStreamFilter ff_pgs_frame_merge_bsf = {
> +    .name           = "pgs_frame_merge",
> +    .priv_data_size = sizeof(PGSMergeContext),
> +    .init           = frame_merge_init,
> +    .flush          = frame_merge_flush,
> +    .close          = frame_merge_close,
> +    .filter         = frame_merge_filter,
> +    .codec_ids      = frame_merge_codec_ids,
> +};
John Stebbins April 16, 2020, 11:06 p.m. UTC | #2
On Thu, 2020-04-16 at 15:59 -0700, John Stebbins wrote:
> On Thu, 2020-04-16 at 16:57 -0600, John Stebbins wrote:
> > Required to remux m2ts to mkv
> > ---
> >  libavcodec/Makefile              |   1 +
> >  libavcodec/bitstream_filters.c   |   1 +
> >  libavcodec/pgs_frame_merge_bsf.c | 164
> > +++++++++++++++++++++++++++++++
> >  3 files changed, 166 insertions(+)
> >  create mode 100644 libavcodec/pgs_frame_merge_bsf.c
> > 
> 
> Ugh!  I forgot the version bump.  Will get on that...

A question about this.  I have 2 bsf submitted for review.  Should I
just do a separate patch that updates the minor version once.  Or do
you require it to be bumped with each change separately?

> 
> > diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> > index c1c9a44f2b..c7d8fbebaa 100644
> > --- a/libavcodec/Makefile
> > +++ b/libavcodec/Makefile
> > @@ -1110,6 +1110,7 @@ OBJS-$(CONFIG_MP3_HEADER_DECOMPRESS_BSF)  +=
> > mp3_header_decompress_bsf.o \
> >  OBJS-$(CONFIG_MPEG2_METADATA_BSF)         += mpeg2_metadata_bsf.o
> >  OBJS-$(CONFIG_NOISE_BSF)                  += noise_bsf.o
> >  OBJS-$(CONFIG_NULL_BSF)                   += null_bsf.o
> > +OBJS-$(CONFIG_PGS_FRAME_MERGE_BSF)        += pgs_frame_merge_bsf.o
> >  OBJS-$(CONFIG_PRORES_METADATA_BSF)        += prores_metadata_bsf.o
> >  OBJS-$(CONFIG_REMOVE_EXTRADATA_BSF)       +=
> > remove_extradata_bsf.o
> >  OBJS-$(CONFIG_TEXT2MOVSUB_BSF)            += movsub_bsf.o
> > diff --git a/libavcodec/bitstream_filters.c
> > b/libavcodec/bitstream_filters.c
> > index 6b5ffe4d70..92619225f0 100644
> > --- a/libavcodec/bitstream_filters.c
> > +++ b/libavcodec/bitstream_filters.c
> > @@ -49,6 +49,7 @@ extern const AVBitStreamFilter
> > ff_mpeg4_unpack_bframes_bsf;
> >  extern const AVBitStreamFilter ff_mov2textsub_bsf;
> >  extern const AVBitStreamFilter ff_noise_bsf;
> >  extern const AVBitStreamFilter ff_null_bsf;
> > +extern const AVBitStreamFilter ff_pgs_frame_merge_bsf;
> >  extern const AVBitStreamFilter ff_prores_metadata_bsf;
> >  extern const AVBitStreamFilter ff_remove_extradata_bsf;
> >  extern const AVBitStreamFilter ff_text2movsub_bsf;
> > diff --git a/libavcodec/pgs_frame_merge_bsf.c
> > b/libavcodec/pgs_frame_merge_bsf.c
> > new file mode 100644
> > index 0000000000..d1bb3a60c2
> > --- /dev/null
> > +++ b/libavcodec/pgs_frame_merge_bsf.c
> > @@ -0,0 +1,164 @@
> > +/*
> > + * Copyright (c) 2020 John Stebbins <jstebbins.hb@gmail.com>
> > + *
> > + * 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
> > + * This bitstream filter merges PGS subtitle packets containing
> > incomplete
> > + * set of segments into a single packet
> > + *
> > + * Packets already containing a complete set of segments will be
> > passed through
> > + * unchanged.
> > + */
> > +
> > +#include "avcodec.h"
> > +#include "bsf.h"
> > +#include "libavutil/intreadwrite.h"
> > +
> > +enum PGSSegmentType {
> > +    PALETTE_SEGMENT      = 0x14,
> > +    OBJECT_SEGMENT       = 0x15,
> > +    PRESENTATION_SEGMENT = 0x16,
> > +    WINDOW_SEGMENT       = 0x17,
> > +    DISPLAY_SEGMENT      = 0x80,
> > +};
> > +
> > +typedef struct PGSMergeContext {
> > +    AVPacket *buffer_pkt, *in;
> > +    int presentation_found;
> > +} PGSMergeContext;
> > +
> > +static void frame_merge_flush(AVBSFContext *bsf)
> > +{
> > +    PGSMergeContext *ctx = bsf->priv_data;
> > +
> > +    av_packet_unref(ctx->in);
> > +    av_packet_unref(ctx->buffer_pkt);
> > +}
> > +
> > +static int frame_merge_filter(AVBSFContext *bsf, AVPacket *out)
> > +{
> > +    PGSMergeContext *ctx = bsf->priv_data;
> > +    AVPacket *in = ctx->in, *pkt = ctx->buffer_pkt;
> > +    int ret, i, size, pos, display = 0;
> > +
> > +    if (!in->data) {
> > +        ret = ff_bsf_get_packet_ref(bsf, in);
> > +        if (ret < 0)
> > +            return ret;
> > +    }
> > +    if (!in->size) {
> > +        av_packet_unref(in);
> > +        return AVERROR(EAGAIN);
> > +    }
> > +
> > +    // Validate packet data and find display_end segment
> > +    size = in->size;
> > +    i = 0;
> > +    while (i + 3 <= in->size) {
> > +        uint8_t segment_type;
> > +        int segment_len;
> > +
> > +        segment_type = in->data[i];
> > +        segment_len  = AV_RB16(in->data + i + 1) + 3;
> > +        if (i + segment_len > in->size) // Invalid data
> > +            break;
> > +        if (segment_type == PRESENTATION_SEGMENT && !ctx-
> > > presentation_found) {
> > +            int object_count;
> > +            ctx->presentation_found = 1;
> > +            ret = av_packet_copy_props(pkt, in);
> > +            if (ret < 0)
> > +                goto fail;
> > +            object_count  = in->data[i + 13];
> > +            pkt->flags = !!object_count * AV_PKT_FLAG_KEY;
> > +        }
> > +        i += segment_len;
> > +        if (segment_type == DISPLAY_SEGMENT) {
> > +            size = display = i;
> > +            break;
> > +        }
> > +    }
> > +    if (display && pkt->size == 0 && size == in->size) { //
> > passthrough
> > +        av_packet_move_ref(out, in);
> > +        return 0;
> > +    }
> > +    if ((!display && i != in->size) || size > in->size) {
> > +        av_log(bsf, AV_LOG_WARNING, "Failed to parse PGS
> > segments.\n");
> > +        // force output what we have
> > +        display = size = in->size;
> > +    }
> > +
> > +    pos = pkt->size;
> > +    ret = av_grow_packet(pkt, size);
> > +    if (ret < 0)
> > +        goto fail;
> > +    memcpy(pkt->data + pos, in->data, size);
> > +
> > +    if (size == in->size)
> > +        av_packet_unref(in);
> > +    else {
> > +        in->data += size;
> > +        in->size -= size;
> > +    }
> > +
> > +    if (display) {
> > +        ctx->presentation_found = 0;
> > +        av_packet_move_ref(out, pkt);
> > +        return 0;
> > +    }
> > +    return AVERROR(EAGAIN);
> > +
> > +fail:
> > +    frame_merge_flush(bsf);
> > +    return ret;
> > +}
> > +
> > +static int frame_merge_init(AVBSFContext *bsf)
> > +{
> > +    PGSMergeContext *ctx = bsf->priv_data;
> > +
> > +    ctx->in  = av_packet_alloc();
> > +    ctx->buffer_pkt = av_packet_alloc();
> > +    if (!ctx->in || !ctx->buffer_pkt)
> > +        return AVERROR(ENOMEM);
> > +
> > +    return 0;
> > +}
> > +
> > +static void frame_merge_close(AVBSFContext *bsf)
> > +{
> > +    PGSMergeContext *ctx = bsf->priv_data;
> > +
> > +    av_packet_free(&ctx->in);
> > +    av_packet_free(&ctx->buffer_pkt);
> > +}
> > +
> > +static const enum AVCodecID frame_merge_codec_ids[] = {
> > +    AV_CODEC_ID_HDMV_PGS_SUBTITLE, AV_CODEC_ID_NONE,
> > +};
> > +
> > +const AVBitStreamFilter ff_pgs_frame_merge_bsf = {
> > +    .name           = "pgs_frame_merge",
> > +    .priv_data_size = sizeof(PGSMergeContext),
> > +    .init           = frame_merge_init,
> > +    .flush          = frame_merge_flush,
> > +    .close          = frame_merge_close,
> > +    .filter         = frame_merge_filter,
> > +    .codec_ids      = frame_merge_codec_ids,
> > +};
James Almer April 17, 2020, 1:08 a.m. UTC | #3
On 4/16/2020 8:06 PM, John Stebbins wrote:
> On Thu, 2020-04-16 at 15:59 -0700, John Stebbins wrote:
>> On Thu, 2020-04-16 at 16:57 -0600, John Stebbins wrote:
>>> Required to remux m2ts to mkv
>>> ---
>>>  libavcodec/Makefile              |   1 +
>>>  libavcodec/bitstream_filters.c   |   1 +
>>>  libavcodec/pgs_frame_merge_bsf.c | 164
>>> +++++++++++++++++++++++++++++++
>>>  3 files changed, 166 insertions(+)
>>>  create mode 100644 libavcodec/pgs_frame_merge_bsf.c
>>>
>>
>> Ugh!  I forgot the version bump.  Will get on that...
> 
> A question about this.  I have 2 bsf submitted for review.  Should I
> just do a separate patch that updates the minor version once.  Or do
> you require it to be bumped with each change separately?

If they will both go together, then a single bump is enough.
Petri Hintukainen April 17, 2020, 10:21 a.m. UTC | #4
to, 2020-04-16 kello 16:57 -0600, John Stebbins kirjoitti:
> Required to remux m2ts to mkv
> ---
>  libavcodec/Makefile              |   1 +
>  libavcodec/bitstream_filters.c   |   1 +
>  libavcodec/pgs_frame_merge_bsf.c | 164
> +++++++++++++++++++++++++++++++
>  3 files changed, 166 insertions(+)
>  create mode 100644 libavcodec/pgs_frame_merge_bsf.c
> 
> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> index c1c9a44f2b..c7d8fbebaa 100644
> --- a/libavcodec/Makefile
> +++ b/libavcodec/Makefile
> @@ -1110,6 +1110,7 @@ OBJS-$(CONFIG_MP3_HEADER_DECOMPRESS_BSF)  +=
> mp3_header_decompress_bsf.o \
>  OBJS-$(CONFIG_MPEG2_METADATA_BSF)         += mpeg2_metadata_bsf.o
>  OBJS-$(CONFIG_NOISE_BSF)                  += noise_bsf.o
>  OBJS-$(CONFIG_NULL_BSF)                   += null_bsf.o
> +OBJS-$(CONFIG_PGS_FRAME_MERGE_BSF)        += pgs_frame_merge_bsf.o
>  OBJS-$(CONFIG_PRORES_METADATA_BSF)        += prores_metadata_bsf.o
>  OBJS-$(CONFIG_REMOVE_EXTRADATA_BSF)       += remove_extradata_bsf.o
>  OBJS-$(CONFIG_TEXT2MOVSUB_BSF)            += movsub_bsf.o
> diff --git a/libavcodec/bitstream_filters.c
> b/libavcodec/bitstream_filters.c
> index 6b5ffe4d70..92619225f0 100644
> --- a/libavcodec/bitstream_filters.c
> +++ b/libavcodec/bitstream_filters.c
> @@ -49,6 +49,7 @@ extern const AVBitStreamFilter
> ff_mpeg4_unpack_bframes_bsf;
>  extern const AVBitStreamFilter ff_mov2textsub_bsf;
>  extern const AVBitStreamFilter ff_noise_bsf;
>  extern const AVBitStreamFilter ff_null_bsf;
> +extern const AVBitStreamFilter ff_pgs_frame_merge_bsf;
>  extern const AVBitStreamFilter ff_prores_metadata_bsf;
>  extern const AVBitStreamFilter ff_remove_extradata_bsf;
>  extern const AVBitStreamFilter ff_text2movsub_bsf;
> diff --git a/libavcodec/pgs_frame_merge_bsf.c
> b/libavcodec/pgs_frame_merge_bsf.c
> new file mode 100644
> index 0000000000..d1bb3a60c2
> --- /dev/null
> +++ b/libavcodec/pgs_frame_merge_bsf.c
> @@ -0,0 +1,164 @@
> +/*
> + * Copyright (c) 2020 John Stebbins <jstebbins.hb@gmail.com>
> + *
> + * 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
> + * This bitstream filter merges PGS subtitle packets containing
> incomplete
> + * set of segments into a single packet
> + *
> + * Packets already containing a complete set of segments will be
> passed through
> + * unchanged.
> + */
> +
> +#include "avcodec.h"
> +#include "bsf.h"
> +#include "libavutil/intreadwrite.h"
> +
> +enum PGSSegmentType {
> +    PALETTE_SEGMENT      = 0x14,
> +    OBJECT_SEGMENT       = 0x15,
> +    PRESENTATION_SEGMENT = 0x16,
> +    WINDOW_SEGMENT       = 0x17,
> +    DISPLAY_SEGMENT      = 0x80,
> +};

This is duplicated in pgssubdec.c and pgs_frame_split_bsf.c. It's not
much of code for a separate header, but maybe you'll find more while
writing the encoder ?

> +typedef struct PGSMergeContext {
> +    AVPacket *buffer_pkt, *in;
> +    int presentation_found;
> +} PGSMergeContext;
> +
> +static void frame_merge_flush(AVBSFContext *bsf)
> +{
> +    PGSMergeContext *ctx = bsf->priv_data;
> +
> +    av_packet_unref(ctx->in);
> +    av_packet_unref(ctx->buffer_pkt);
> +}
> +
> +static int frame_merge_filter(AVBSFContext *bsf, AVPacket *out)
> +{
> +    PGSMergeContext *ctx = bsf->priv_data;
> +    AVPacket *in = ctx->in, *pkt = ctx->buffer_pkt;
> +    int ret, i, size, pos, display = 0;
> +
> +    if (!in->data) {
> +        ret = ff_bsf_get_packet_ref(bsf, in);
> +        if (ret < 0)
> +            return ret;
> +    }
> +    if (!in->size) {
> +        av_packet_unref(in);
> +        return AVERROR(EAGAIN);
> +    }
> +
> +    // Validate packet data and find display_end segment
> +    size = in->size;
> +    i = 0;
> +    while (i + 3 <= in->size) {
> +        uint8_t segment_type;
> +        int segment_len;
> +
> +        segment_type = in->data[i];
> +        segment_len  = AV_RB16(in->data + i + 1) + 3;
> +        if (i + segment_len > in->size) // Invalid data
> +            break;
> +        if (segment_type == PRESENTATION_SEGMENT && !ctx-
> >presentation_found) {
> +            int object_count;
> +            ctx->presentation_found = 1;
> +            ret = av_packet_copy_props(pkt, in);
> +            if (ret < 0)
> +                goto fail;
> +            object_count  = in->data[i + 13];

Possible OOB read with invalid input ?

> +            pkt->flags = !!object_count * AV_PKT_FLAG_KEY;

Does keyframe here mean something that can be decoded and displayed
without previous display sets ?

If yes, maybe you could check composition descriptor state instead ? I
haven't looked into PGS internals in long time, but I think objects and
palettes can be shared between display sets. Ex. animations could be
implemented with only location / cropping / palette changes.

> +        }
> +        i += segment_len;
> +        if (segment_type == DISPLAY_SEGMENT) {
> +            size = display = i;
> +            break;
> +        }
> +    }
> +    if (display && pkt->size == 0 && size == in->size) { //
> passthrough
> +        av_packet_move_ref(out, in);
> +        return 0;
> +    }
> +    if ((!display && i != in->size) || size > in->size) {
> +        av_log(bsf, AV_LOG_WARNING, "Failed to parse PGS
> segments.\n");
> +        // force output what we have
> +        display = size = in->size;
> +    }
> +
> +    pos = pkt->size;
> +    ret = av_grow_packet(pkt, size);
> +    if (ret < 0)
> +        goto fail;
> +    memcpy(pkt->data + pos, in->data, size);
> +
> +    if (size == in->size)
> +        av_packet_unref(in);
> +    else {
> +        in->data += size;
> +        in->size -= size;
> +    }
> +
> +    if (display) {
> +        ctx->presentation_found = 0;
> +        av_packet_move_ref(out, pkt);
> +        return 0;
> +    }
> +    return AVERROR(EAGAIN);
> +
> +fail:
> +    frame_merge_flush(bsf);
> +    return ret;
> +}
> +
> +static int frame_merge_init(AVBSFContext *bsf)
> +{
> +    PGSMergeContext *ctx = bsf->priv_data;
> +
> +    ctx->in  = av_packet_alloc();
> +    ctx->buffer_pkt = av_packet_alloc();
> +    if (!ctx->in || !ctx->buffer_pkt)
> +        return AVERROR(ENOMEM);
> +
> +    return 0;
> +}
> +
> +static void frame_merge_close(AVBSFContext *bsf)
> +{
> +    PGSMergeContext *ctx = bsf->priv_data;
> +
> +    av_packet_free(&ctx->in);
> +    av_packet_free(&ctx->buffer_pkt);
> +}
> +
> +static const enum AVCodecID frame_merge_codec_ids[] = {
> +    AV_CODEC_ID_HDMV_PGS_SUBTITLE, AV_CODEC_ID_NONE,
> +};
> +
> +const AVBitStreamFilter ff_pgs_frame_merge_bsf = {
> +    .name           = "pgs_frame_merge",
> +    .priv_data_size = sizeof(PGSMergeContext),
> +    .init           = frame_merge_init,
> +    .flush          = frame_merge_flush,
> +    .close          = frame_merge_close,
> +    .filter         = frame_merge_filter,
> +    .codec_ids      = frame_merge_codec_ids,
> +};
John Stebbins April 17, 2020, 3:52 p.m. UTC | #5
On Fri, 2020-04-17 at 13:21 +0300, Petri Hintukainen wrote:
> to, 2020-04-16 kello 16:57 -0600, John Stebbins kirjoitti:
> > Required to remux m2ts to mkv
> > ---
> >  libavcodec/Makefile              |   1 +
> >  libavcodec/bitstream_filters.c   |   1 +
> >  libavcodec/pgs_frame_merge_bsf.c | 164
> > +++++++++++++++++++++++++++++++
> >  3 files changed, 166 insertions(+)
> >  create mode 100644 libavcodec/pgs_frame_merge_bsf.c
> > 
> > diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> > index c1c9a44f2b..c7d8fbebaa 100644
> > --- a/libavcodec/Makefile
> > +++ b/libavcodec/Makefile
> > @@ -1110,6 +1110,7 @@ OBJS-$(CONFIG_MP3_HEADER_DECOMPRESS_BSF)  +=
> > mp3_header_decompress_bsf.o \
> >  OBJS-$(CONFIG_MPEG2_METADATA_BSF)         += mpeg2_metadata_bsf.o
> >  OBJS-$(CONFIG_NOISE_BSF)                  += noise_bsf.o
> >  OBJS-$(CONFIG_NULL_BSF)                   += null_bsf.o
> > +OBJS-$(CONFIG_PGS_FRAME_MERGE_BSF)        += pgs_frame_merge_bsf.o
> >  OBJS-$(CONFIG_PRORES_METADATA_BSF)        += prores_metadata_bsf.o
> >  OBJS-$(CONFIG_REMOVE_EXTRADATA_BSF)       +=
> > remove_extradata_bsf.o
> >  OBJS-$(CONFIG_TEXT2MOVSUB_BSF)            += movsub_bsf.o
> > diff --git a/libavcodec/bitstream_filters.c
> > b/libavcodec/bitstream_filters.c
> > index 6b5ffe4d70..92619225f0 100644
> > --- a/libavcodec/bitstream_filters.c
> > +++ b/libavcodec/bitstream_filters.c
> > @@ -49,6 +49,7 @@ extern const AVBitStreamFilter
> > ff_mpeg4_unpack_bframes_bsf;
> >  extern const AVBitStreamFilter ff_mov2textsub_bsf;
> >  extern const AVBitStreamFilter ff_noise_bsf;
> >  extern const AVBitStreamFilter ff_null_bsf;
> > +extern const AVBitStreamFilter ff_pgs_frame_merge_bsf;
> >  extern const AVBitStreamFilter ff_prores_metadata_bsf;
> >  extern const AVBitStreamFilter ff_remove_extradata_bsf;
> >  extern const AVBitStreamFilter ff_text2movsub_bsf;
> > diff --git a/libavcodec/pgs_frame_merge_bsf.c
> > b/libavcodec/pgs_frame_merge_bsf.c
> > new file mode 100644
> > index 0000000000..d1bb3a60c2
> > --- /dev/null
> > +++ b/libavcodec/pgs_frame_merge_bsf.c
> > @@ -0,0 +1,164 @@
> > +/*
> > + * Copyright (c) 2020 John Stebbins <jstebbins.hb@gmail.com>
> > + *
> > + * 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
> > + * This bitstream filter merges PGS subtitle packets containing
> > incomplete
> > + * set of segments into a single packet
> > + *
> > + * Packets already containing a complete set of segments will be
> > passed through
> > + * unchanged.
> > + */
> > +
> > +#include "avcodec.h"
> > +#include "bsf.h"
> > +#include "libavutil/intreadwrite.h"
> > +
> > +enum PGSSegmentType {
> > +    PALETTE_SEGMENT      = 0x14,
> > +    OBJECT_SEGMENT       = 0x15,
> > +    PRESENTATION_SEGMENT = 0x16,
> > +    WINDOW_SEGMENT       = 0x17,
> > +    DISPLAY_SEGMENT      = 0x80,
> > +};
> 
> This is duplicated in pgssubdec.c and pgs_frame_split_bsf.c. It's not
> much of code for a separate header, but maybe you'll find more while
> writing the encoder ?
> 

I've already written the encoder.  This enum is the only thing shared.
So hardly seems worth adding a header. 

Testing the encoder is what lead me to writing the bsfs. It "works" in
the sense that it's generating correct bitstreams.  But I realized the
bsfs were needed to handle muxing properly.  And there is still the
case to handle when the input AVSubtitle has end_display_time set.  In
this case I need to generate 2 segment sequences.  One for the subtitle
and one to indicate the end of the subtitle.  The current subtitle
encoder API doesn't handle multipe output packets for on input subtitle
gracefully.  So that's next on my list to fix.

> > +typedef struct PGSMergeContext {
> > +    AVPacket *buffer_pkt, *in;
> > +    int presentation_found;
> > +} PGSMergeContext;
> > +
> > +static void frame_merge_flush(AVBSFContext *bsf)
> > +{
> > +    PGSMergeContext *ctx = bsf->priv_data;
> > +
> > +    av_packet_unref(ctx->in);
> > +    av_packet_unref(ctx->buffer_pkt);
> > +}
> > +
> > +static int frame_merge_filter(AVBSFContext *bsf, AVPacket *out)
> > +{
> > +    PGSMergeContext *ctx = bsf->priv_data;
> > +    AVPacket *in = ctx->in, *pkt = ctx->buffer_pkt;
> > +    int ret, i, size, pos, display = 0;
> > +
> > +    if (!in->data) {
> > +        ret = ff_bsf_get_packet_ref(bsf, in);
> > +        if (ret < 0)
> > +            return ret;
> > +    }
> > +    if (!in->size) {
> > +        av_packet_unref(in);
> > +        return AVERROR(EAGAIN);
> > +    }
> > +
> > +    // Validate packet data and find display_end segment
> > +    size = in->size;
> > +    i = 0;
> > +    while (i + 3 <= in->size) {
> > +        uint8_t segment_type;
> > +        int segment_len;
> > +
> > +        segment_type = in->data[i];
> > +        segment_len  = AV_RB16(in->data + i + 1) + 3;
> > +        if (i + segment_len > in->size) // Invalid data
> > +            break;
> > +        if (segment_type == PRESENTATION_SEGMENT && !ctx-
> > > presentation_found) {
> > +            int object_count;
> > +            ctx->presentation_found = 1;
> > +            ret = av_packet_copy_props(pkt, in);
> > +            if (ret < 0)
> > +                goto fail;
> > +            object_count  = in->data[i + 13];
> 
> Possible OOB read with invalid input ?

Yeah, when I woke up this morning, I realized I had done this.
Will fix

> 
> > +            pkt->flags = !!object_count * AV_PKT_FLAG_KEY;
> 
> Does keyframe here mean something that can be decoded and displayed
> without previous display sets ?
> 
> If yes, maybe you could check composition descriptor state instead ?
> I
> haven't looked into PGS internals in long time, but I think objects
> and
> palettes can be shared between display sets. Ex. animations could be
> implemented with only location / cropping / palette changes.

Hmm, good point.  *All* information to decode a subtitle is only
guaranteed to be present in the current sequence when the
composition_state == 1.  For other values, it may be referencing
objects from previous sequences.  This is probably a better way to
signal key frames.

Thanks!

> 
> > +        }
> > +        i += segment_len;
> > +        if (segment_type == DISPLAY_SEGMENT) {
> > +            size = display = i;
> > +            break;
> > +        }
> > +    }
> > +    if (display && pkt->size == 0 && size == in->size) { //
> > passthrough
> > +        av_packet_move_ref(out, in);
> > +        return 0;
> > +    }
> > +    if ((!display && i != in->size) || size > in->size) {
> > +        av_log(bsf, AV_LOG_WARNING, "Failed to parse PGS
> > segments.\n");
> > +        // force output what we have
> > +        display = size = in->size;
> > +    }
> > +
> > +    pos = pkt->size;
> > +    ret = av_grow_packet(pkt, size);
> > +    if (ret < 0)
> > +        goto fail;
> > +    memcpy(pkt->data + pos, in->data, size);
> > +
> > +    if (size == in->size)
> > +        av_packet_unref(in);
> > +    else {
> > +        in->data += size;
> > +        in->size -= size;
> > +    }
> > +
> > +    if (display) {
> > +        ctx->presentation_found = 0;
> > +        av_packet_move_ref(out, pkt);
> > +        return 0;
> > +    }
> > +    return AVERROR(EAGAIN);
> > +
> > +fail:
> > +    frame_merge_flush(bsf);
> > +    return ret;
> > +}
> > +
> > +static int frame_merge_init(AVBSFContext *bsf)
> > +{
> > +    PGSMergeContext *ctx = bsf->priv_data;
> > +
> > +    ctx->in  = av_packet_alloc();
> > +    ctx->buffer_pkt = av_packet_alloc();
> > +    if (!ctx->in || !ctx->buffer_pkt)
> > +        return AVERROR(ENOMEM);
> > +
> > +    return 0;
> > +}
> > +
> > +static void frame_merge_close(AVBSFContext *bsf)
> > +{
> > +    PGSMergeContext *ctx = bsf->priv_data;
> > +
> > +    av_packet_free(&ctx->in);
> > +    av_packet_free(&ctx->buffer_pkt);
> > +}
> > +
> > +static const enum AVCodecID frame_merge_codec_ids[] = {
> > +    AV_CODEC_ID_HDMV_PGS_SUBTITLE, AV_CODEC_ID_NONE,
> > +};
> > +
> > +const AVBitStreamFilter ff_pgs_frame_merge_bsf = {
> > +    .name           = "pgs_frame_merge",
> > +    .priv_data_size = sizeof(PGSMergeContext),
> > +    .init           = frame_merge_init,
> > +    .flush          = frame_merge_flush,
> > +    .close          = frame_merge_close,
> > +    .filter         = frame_merge_filter,
> > +    .codec_ids      = frame_merge_codec_ids,
> > +};
> 
>
John Stebbins April 17, 2020, 4:11 p.m. UTC | #6
On Fri, 2020-04-17 at 08:52 -0700, John Stebbins wrote:
> On Fri, 2020-04-17 at 13:21 +0300, Petri Hintukainen wrote:
> > to, 2020-04-16 kello 16:57 -0600, John Stebbins kirjoitti:
> > > Required to remux m2ts to mkv
> > > ---
> > >  libavcodec/Makefile              |   1 +
> > >  libavcodec/bitstream_filters.c   |   1 +
> > >  libavcodec/pgs_frame_merge_bsf.c | 164
> > > +++++++++++++++++++++++++++++++
> > >  3 files changed, 166 insertions(+)
> > >  create mode 100644 libavcodec/pgs_frame_merge_bsf.c
> > > 
> > > diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> > > index c1c9a44f2b..c7d8fbebaa 100644
> > > --- a/libavcodec/Makefile
> > > +++ b/libavcodec/Makefile
> > > @@ -1110,6 +1110,7 @@ OBJS-
> > > $(CONFIG_MP3_HEADER_DECOMPRESS_BSF)  +=
> > > mp3_header_decompress_bsf.o \
> > >  OBJS-$(CONFIG_MPEG2_METADATA_BSF)         +=
> > > mpeg2_metadata_bsf.o
> > >  OBJS-$(CONFIG_NOISE_BSF)                  += noise_bsf.o
> > >  OBJS-$(CONFIG_NULL_BSF)                   += null_bsf.o
> > > +OBJS-$(CONFIG_PGS_FRAME_MERGE_BSF)        +=
> > > pgs_frame_merge_bsf.o
> > >  OBJS-$(CONFIG_PRORES_METADATA_BSF)        +=
> > > prores_metadata_bsf.o
> > >  OBJS-$(CONFIG_REMOVE_EXTRADATA_BSF)       +=
> > > remove_extradata_bsf.o
> > >  OBJS-$(CONFIG_TEXT2MOVSUB_BSF)            += movsub_bsf.o
> > > diff --git a/libavcodec/bitstream_filters.c
> > > b/libavcodec/bitstream_filters.c
> > > index 6b5ffe4d70..92619225f0 100644
> > > --- a/libavcodec/bitstream_filters.c
> > > +++ b/libavcodec/bitstream_filters.c
> > > @@ -49,6 +49,7 @@ extern const AVBitStreamFilter
> > > ff_mpeg4_unpack_bframes_bsf;
> > >  extern const AVBitStreamFilter ff_mov2textsub_bsf;
> > >  extern const AVBitStreamFilter ff_noise_bsf;
> > >  extern const AVBitStreamFilter ff_null_bsf;
> > > +extern const AVBitStreamFilter ff_pgs_frame_merge_bsf;
> > >  extern const AVBitStreamFilter ff_prores_metadata_bsf;
> > >  extern const AVBitStreamFilter ff_remove_extradata_bsf;
> > >  extern const AVBitStreamFilter ff_text2movsub_bsf;
> > > diff --git a/libavcodec/pgs_frame_merge_bsf.c
> > > b/libavcodec/pgs_frame_merge_bsf.c
> > > new file mode 100644
> > > index 0000000000..d1bb3a60c2
> > > --- /dev/null
> > > +++ b/libavcodec/pgs_frame_merge_bsf.c
> > > @@ -0,0 +1,164 @@
> > > +/*
> > > + * Copyright (c) 2020 John Stebbins <jstebbins.hb@gmail.com>
> > > + *
> > > + * 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
> > > + * This bitstream filter merges PGS subtitle packets containing
> > > incomplete
> > > + * set of segments into a single packet
> > > + *
> > > + * Packets already containing a complete set of segments will be
> > > passed through
> > > + * unchanged.
> > > + */
> > > +
> > > +#include "avcodec.h"
> > > +#include "bsf.h"
> > > +#include "libavutil/intreadwrite.h"
> > > +
> > > +enum PGSSegmentType {
> > > +    PALETTE_SEGMENT      = 0x14,
> > > +    OBJECT_SEGMENT       = 0x15,
> > > +    PRESENTATION_SEGMENT = 0x16,
> > > +    WINDOW_SEGMENT       = 0x17,
> > > +    DISPLAY_SEGMENT      = 0x80,
> > > +};
> > 
> > This is duplicated in pgssubdec.c and pgs_frame_split_bsf.c. It's
> > not
> > much of code for a separate header, but maybe you'll find more
> > while
> > writing the encoder ?
> > 
> 
> I've already written the encoder.  This enum is the only thing
> shared.
> So hardly seems worth adding a header. 
> 
> Testing the encoder is what lead me to writing the bsfs. It "works"
> in
> the sense that it's generating correct bitstreams.  But I realized
> the
> bsfs were needed to handle muxing properly.  And there is still the
> case to handle when the input AVSubtitle has end_display_time
> set.  In
> this case I need to generate 2 segment sequences.  One for the
> subtitle
> and one to indicate the end of the subtitle.  The current subtitle
> encoder API doesn't handle multipe output packets for on input
> subtitle
> gracefully.  So that's next on my list to fix.
> 
> > > +typedef struct PGSMergeContext {
> > > +    AVPacket *buffer_pkt, *in;
> > > +    int presentation_found;
> > > +} PGSMergeContext;
> > > +
> > > +static void frame_merge_flush(AVBSFContext *bsf)
> > > +{
> > > +    PGSMergeContext *ctx = bsf->priv_data;
> > > +
> > > +    av_packet_unref(ctx->in);
> > > +    av_packet_unref(ctx->buffer_pkt);
> > > +}
> > > +
> > > +static int frame_merge_filter(AVBSFContext *bsf, AVPacket *out)
> > > +{
> > > +    PGSMergeContext *ctx = bsf->priv_data;
> > > +    AVPacket *in = ctx->in, *pkt = ctx->buffer_pkt;
> > > +    int ret, i, size, pos, display = 0;
> > > +
> > > +    if (!in->data) {
> > > +        ret = ff_bsf_get_packet_ref(bsf, in);
> > > +        if (ret < 0)
> > > +            return ret;
> > > +    }
> > > +    if (!in->size) {
> > > +        av_packet_unref(in);
> > > +        return AVERROR(EAGAIN);
> > > +    }
> > > +
> > > +    // Validate packet data and find display_end segment
> > > +    size = in->size;
> > > +    i = 0;
> > > +    while (i + 3 <= in->size) {
> > > +        uint8_t segment_type;
> > > +        int segment_len;
> > > +
> > > +        segment_type = in->data[i];
> > > +        segment_len  = AV_RB16(in->data + i + 1) + 3;
> > > +        if (i + segment_len > in->size) // Invalid data
> > > +            break;
> > > +        if (segment_type == PRESENTATION_SEGMENT && !ctx-
> > > > presentation_found) {
> > > +            int object_count;
> > > +            ctx->presentation_found = 1;
> > > +            ret = av_packet_copy_props(pkt, in);
> > > +            if (ret < 0)
> > > +                goto fail;
> > > +            object_count  = in->data[i + 13];
> > 
> > Possible OOB read with invalid input ?
> 
> Yeah, when I woke up this morning, I realized I had done this.
> Will fix
> 
> > > +            pkt->flags = !!object_count * AV_PKT_FLAG_KEY;
> > 
> > Does keyframe here mean something that can be decoded and displayed
> > without previous display sets ?
> > 
> > If yes, maybe you could check composition descriptor state instead
> > ?
> > I
> > haven't looked into PGS internals in long time, but I think objects
> > and
> > palettes can be shared between display sets. Ex. animations could
> > be
> > implemented with only location / cropping / palette changes.
> 
> Hmm, good point.  *All* information to decode a subtitle is only
> guaranteed to be present in the current sequence when the
> composition_state == 1.  For other values, it may be referencing
> objects from previous sequences.  This is probably a better way to
> signal key frames.
> 
> Thanks!
> 

Well, that's just not going to work (:  It appears the "aquisition
point" state is pretty much never used.  At least I never see it in a
few samples I've tested.  They appear to like to simply set start of
epoch state and normal state.  And both states almost always have
complete information in the current sequence to render the subtitle.

So I'm afraid all we have are kind of bad choices.  Signal a keyframe
whenever there are objects provided and risk that the decoder may
encounter references it can't find.  Signal keyframes only on epoch
start and objects are provided, this will miss all "normal state"
sequences which are the majority. Signal a keyframe on acquisition
state which appears to *never* happen.

I lean toward keyframes for any sequence that has objects and let the
decoder deal with missing references.

> > > +        }
> > > +        i += segment_len;
> > > +        if (segment_type == DISPLAY_SEGMENT) {
> > > +            size = display = i;
> > > +            break;
> > > +        }
> > > +    }
> > > +    if (display && pkt->size == 0 && size == in->size) { //
> > > passthrough
> > > +        av_packet_move_ref(out, in);
> > > +        return 0;
> > > +    }
> > > +    if ((!display && i != in->size) || size > in->size) {
> > > +        av_log(bsf, AV_LOG_WARNING, "Failed to parse PGS
> > > segments.\n");
> > > +        // force output what we have
> > > +        display = size = in->size;
> > > +    }
> > > +
> > > +    pos = pkt->size;
> > > +    ret = av_grow_packet(pkt, size);
> > > +    if (ret < 0)
> > > +        goto fail;
> > > +    memcpy(pkt->data + pos, in->data, size);
> > > +
> > > +    if (size == in->size)
> > > +        av_packet_unref(in);
> > > +    else {
> > > +        in->data += size;
> > > +        in->size -= size;
> > > +    }
> > > +
> > > +    if (display) {
> > > +        ctx->presentation_found = 0;
> > > +        av_packet_move_ref(out, pkt);
> > > +        return 0;
> > > +    }
> > > +    return AVERROR(EAGAIN);
> > > +
> > > +fail:
> > > +    frame_merge_flush(bsf);
> > > +    return ret;
> > > +}
> > > +
> > > +static int frame_merge_init(AVBSFContext *bsf)
> > > +{
> > > +    PGSMergeContext *ctx = bsf->priv_data;
> > > +
> > > +    ctx->in  = av_packet_alloc();
> > > +    ctx->buffer_pkt = av_packet_alloc();
> > > +    if (!ctx->in || !ctx->buffer_pkt)
> > > +        return AVERROR(ENOMEM);
> > > +
> > > +    return 0;
> > > +}
> > > +
> > > +static void frame_merge_close(AVBSFContext *bsf)
> > > +{
> > > +    PGSMergeContext *ctx = bsf->priv_data;
> > > +
> > > +    av_packet_free(&ctx->in);
> > > +    av_packet_free(&ctx->buffer_pkt);
> > > +}
> > > +
> > > +static const enum AVCodecID frame_merge_codec_ids[] = {
> > > +    AV_CODEC_ID_HDMV_PGS_SUBTITLE, AV_CODEC_ID_NONE,
> > > +};
> > > +
> > > +const AVBitStreamFilter ff_pgs_frame_merge_bsf = {
> > > +    .name           = "pgs_frame_merge",
> > > +    .priv_data_size = sizeof(PGSMergeContext),
> > > +    .init           = frame_merge_init,
> > > +    .flush          = frame_merge_flush,
> > > +    .close          = frame_merge_close,
> > > +    .filter         = frame_merge_filter,
> > > +    .codec_ids      = frame_merge_codec_ids,
> > > +};
diff mbox series

Patch

diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index c1c9a44f2b..c7d8fbebaa 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -1110,6 +1110,7 @@  OBJS-$(CONFIG_MP3_HEADER_DECOMPRESS_BSF)  += mp3_header_decompress_bsf.o \
 OBJS-$(CONFIG_MPEG2_METADATA_BSF)         += mpeg2_metadata_bsf.o
 OBJS-$(CONFIG_NOISE_BSF)                  += noise_bsf.o
 OBJS-$(CONFIG_NULL_BSF)                   += null_bsf.o
+OBJS-$(CONFIG_PGS_FRAME_MERGE_BSF)        += pgs_frame_merge_bsf.o
 OBJS-$(CONFIG_PRORES_METADATA_BSF)        += prores_metadata_bsf.o
 OBJS-$(CONFIG_REMOVE_EXTRADATA_BSF)       += remove_extradata_bsf.o
 OBJS-$(CONFIG_TEXT2MOVSUB_BSF)            += movsub_bsf.o
diff --git a/libavcodec/bitstream_filters.c b/libavcodec/bitstream_filters.c
index 6b5ffe4d70..92619225f0 100644
--- a/libavcodec/bitstream_filters.c
+++ b/libavcodec/bitstream_filters.c
@@ -49,6 +49,7 @@  extern const AVBitStreamFilter ff_mpeg4_unpack_bframes_bsf;
 extern const AVBitStreamFilter ff_mov2textsub_bsf;
 extern const AVBitStreamFilter ff_noise_bsf;
 extern const AVBitStreamFilter ff_null_bsf;
+extern const AVBitStreamFilter ff_pgs_frame_merge_bsf;
 extern const AVBitStreamFilter ff_prores_metadata_bsf;
 extern const AVBitStreamFilter ff_remove_extradata_bsf;
 extern const AVBitStreamFilter ff_text2movsub_bsf;
diff --git a/libavcodec/pgs_frame_merge_bsf.c b/libavcodec/pgs_frame_merge_bsf.c
new file mode 100644
index 0000000000..d1bb3a60c2
--- /dev/null
+++ b/libavcodec/pgs_frame_merge_bsf.c
@@ -0,0 +1,164 @@ 
+/*
+ * Copyright (c) 2020 John Stebbins <jstebbins.hb@gmail.com>
+ *
+ * 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
+ * This bitstream filter merges PGS subtitle packets containing incomplete
+ * set of segments into a single packet
+ *
+ * Packets already containing a complete set of segments will be passed through
+ * unchanged.
+ */
+
+#include "avcodec.h"
+#include "bsf.h"
+#include "libavutil/intreadwrite.h"
+
+enum PGSSegmentType {
+    PALETTE_SEGMENT      = 0x14,
+    OBJECT_SEGMENT       = 0x15,
+    PRESENTATION_SEGMENT = 0x16,
+    WINDOW_SEGMENT       = 0x17,
+    DISPLAY_SEGMENT      = 0x80,
+};
+
+typedef struct PGSMergeContext {
+    AVPacket *buffer_pkt, *in;
+    int presentation_found;
+} PGSMergeContext;
+
+static void frame_merge_flush(AVBSFContext *bsf)
+{
+    PGSMergeContext *ctx = bsf->priv_data;
+
+    av_packet_unref(ctx->in);
+    av_packet_unref(ctx->buffer_pkt);
+}
+
+static int frame_merge_filter(AVBSFContext *bsf, AVPacket *out)
+{
+    PGSMergeContext *ctx = bsf->priv_data;
+    AVPacket *in = ctx->in, *pkt = ctx->buffer_pkt;
+    int ret, i, size, pos, display = 0;
+
+    if (!in->data) {
+        ret = ff_bsf_get_packet_ref(bsf, in);
+        if (ret < 0)
+            return ret;
+    }
+    if (!in->size) {
+        av_packet_unref(in);
+        return AVERROR(EAGAIN);
+    }
+
+    // Validate packet data and find display_end segment
+    size = in->size;
+    i = 0;
+    while (i + 3 <= in->size) {
+        uint8_t segment_type;
+        int segment_len;
+
+        segment_type = in->data[i];
+        segment_len  = AV_RB16(in->data + i + 1) + 3;
+        if (i + segment_len > in->size) // Invalid data
+            break;
+        if (segment_type == PRESENTATION_SEGMENT && !ctx->presentation_found) {
+            int object_count;
+            ctx->presentation_found = 1;
+            ret = av_packet_copy_props(pkt, in);
+            if (ret < 0)
+                goto fail;
+            object_count  = in->data[i + 13];
+            pkt->flags = !!object_count * AV_PKT_FLAG_KEY;
+        }
+        i += segment_len;
+        if (segment_type == DISPLAY_SEGMENT) {
+            size = display = i;
+            break;
+        }
+    }
+    if (display && pkt->size == 0 && size == in->size) { // passthrough
+        av_packet_move_ref(out, in);
+        return 0;
+    }
+    if ((!display && i != in->size) || size > in->size) {
+        av_log(bsf, AV_LOG_WARNING, "Failed to parse PGS segments.\n");
+        // force output what we have
+        display = size = in->size;
+    }
+
+    pos = pkt->size;
+    ret = av_grow_packet(pkt, size);
+    if (ret < 0)
+        goto fail;
+    memcpy(pkt->data + pos, in->data, size);
+
+    if (size == in->size)
+        av_packet_unref(in);
+    else {
+        in->data += size;
+        in->size -= size;
+    }
+
+    if (display) {
+        ctx->presentation_found = 0;
+        av_packet_move_ref(out, pkt);
+        return 0;
+    }
+    return AVERROR(EAGAIN);
+
+fail:
+    frame_merge_flush(bsf);
+    return ret;
+}
+
+static int frame_merge_init(AVBSFContext *bsf)
+{
+    PGSMergeContext *ctx = bsf->priv_data;
+
+    ctx->in  = av_packet_alloc();
+    ctx->buffer_pkt = av_packet_alloc();
+    if (!ctx->in || !ctx->buffer_pkt)
+        return AVERROR(ENOMEM);
+
+    return 0;
+}
+
+static void frame_merge_close(AVBSFContext *bsf)
+{
+    PGSMergeContext *ctx = bsf->priv_data;
+
+    av_packet_free(&ctx->in);
+    av_packet_free(&ctx->buffer_pkt);
+}
+
+static const enum AVCodecID frame_merge_codec_ids[] = {
+    AV_CODEC_ID_HDMV_PGS_SUBTITLE, AV_CODEC_ID_NONE,
+};
+
+const AVBitStreamFilter ff_pgs_frame_merge_bsf = {
+    .name           = "pgs_frame_merge",
+    .priv_data_size = sizeof(PGSMergeContext),
+    .init           = frame_merge_init,
+    .flush          = frame_merge_flush,
+    .close          = frame_merge_close,
+    .filter         = frame_merge_filter,
+    .codec_ids      = frame_merge_codec_ids,
+};