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

[Petri Hintukainen]

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 at 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,
> +};