diff mbox series

[FFmpeg-devel,v2,07/13] cbs: Implement common parts of cbs-based bitstream filters separately

Message ID 20210118224455.750030-8-sw@jkqxz.net
State Accepted
Headers show
Series Metadata handling in CBS, first half, v2
Related show

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Mark Thompson Jan. 18, 2021, 10:44 p.m. UTC
This allows removal of a lot of duplicated code between BSFs.
---
 libavcodec/Makefile  |   2 +-
 libavcodec/cbs_bsf.c | 159 +++++++++++++++++++++++++++++++++++++++++++
 libavcodec/cbs_bsf.h | 131 +++++++++++++++++++++++++++++++++++
 3 files changed, 291 insertions(+), 1 deletion(-)
 create mode 100644 libavcodec/cbs_bsf.c
 create mode 100644 libavcodec/cbs_bsf.h

Comments

James Almer Jan. 19, 2021, 3:07 a.m. UTC | #1
On 1/18/2021 7:44 PM, Mark Thompson wrote:
> +#define BSF_ELEMENT_OPTIONS_PIR(name, help, field, unit_name) \

You could reuse name instead of also passing unit_name. The only bsf 
where it differs is h264_metadata with display_orientation vs disp_or, 
and changing the latter into the former should be fine (Is it even user 
facing?).

> +    { name, help, OFFSET(field), AV_OPT_TYPE_INT, \
> +        { .i64 = BSF_ELEMENT_PASS }, \
> +        BSF_ELEMENT_PASS, BSF_ELEMENT_REMOVE, FLAGS, unit_name }, \

This depends on FLAGS being defined before this macro is invoked. It's 
probably safer and/or more robust to pass the flags as an argument.

> +    { "pass",   NULL, 0, AV_OPT_TYPE_CONST, \
> +        { .i64 = BSF_ELEMENT_PASS   }, .flags = FLAGS, .unit = unit_name }, \
> +    { "insert", NULL, 0, AV_OPT_TYPE_CONST, \
> +        { .i64 = BSF_ELEMENT_INSERT }, .flags = FLAGS, .unit = unit_name }, \
> +    { "remove", NULL, 0, AV_OPT_TYPE_CONST, \
> +        { .i64 = BSF_ELEMENT_REMOVE }, .flags = FLAGS, .unit = unit_name }
> +
> +#define BSF_ELEMENT_OPTIONS_PIRE(name, help, field, unit_name) \
> +    { name, help, OFFSET(field), AV_OPT_TYPE_INT, \
> +        { .i64 = BSF_ELEMENT_PASS }, \
> +        BSF_ELEMENT_PASS, BSF_ELEMENT_EXTRACT, FLAGS, unit_name }, \
> +    { "pass",   NULL, 0, AV_OPT_TYPE_CONST, \
> +        { .i64 = BSF_ELEMENT_PASS   }, .flags = FLAGS, .unit = unit_name }, \
> +    { "insert", NULL, 0, AV_OPT_TYPE_CONST, \
> +        { .i64 = BSF_ELEMENT_INSERT }, .flags = FLAGS, .unit = unit_name }, \
> +    { "remove", NULL, 0, AV_OPT_TYPE_CONST, \
> +        { .i64 = BSF_ELEMENT_REMOVE }, .flags = FLAGS, .unit = unit_name }, \
> +    { "extract", NULL, 0, AV_OPT_TYPE_CONST, \
> +        { .i64 = BSF_ELEMENT_EXTRACT }, .flags = FLAGS, .unit = unit_name } \
Mark Thompson Jan. 19, 2021, 9:39 p.m. UTC | #2
On 19/01/2021 03:07, James Almer wrote:
> On 1/18/2021 7:44 PM, Mark Thompson wrote:
>> +#define BSF_ELEMENT_OPTIONS_PIR(name, help, field, unit_name) \
> 
> You could reuse name instead of also passing unit_name. The only bsf where it differs is h264_metadata with display_orientation vs disp_or, and changing the latter into the former should be fine (Is it even user facing?).

I don't think it can have any user effect, though technically they can inspect it.  So, sure, that's simpler.

>> +    { name, help, OFFSET(field), AV_OPT_TYPE_INT, \
>> +        { .i64 = BSF_ELEMENT_PASS }, \
>> +        BSF_ELEMENT_PASS, BSF_ELEMENT_REMOVE, FLAGS, unit_name }, \
> 
> This depends on FLAGS being defined before this macro is invoked. It's probably safer and/or more robust to pass the flags as an argument.

All BSFs with options use this idiom, so it felt safe ("grep -r AV_OPT_FLAG_BSF_PARAM libavcodec/").

I guess it is slightly surprising to have that extra implicit argument, though, so no objection to making it explicit.

>> +    { "pass",   NULL, 0, AV_OPT_TYPE_CONST, \
>> +        { .i64 = BSF_ELEMENT_PASS   }, .flags = FLAGS, .unit = unit_name }, \
>> +    { "insert", NULL, 0, AV_OPT_TYPE_CONST, \
>> +        { .i64 = BSF_ELEMENT_INSERT }, .flags = FLAGS, .unit = unit_name }, \
>> +    { "remove", NULL, 0, AV_OPT_TYPE_CONST, \
>> +        { .i64 = BSF_ELEMENT_REMOVE }, .flags = FLAGS, .unit = unit_name }
>> +
>> +#define BSF_ELEMENT_OPTIONS_PIRE(name, help, field, unit_name) \
>> +    { name, help, OFFSET(field), AV_OPT_TYPE_INT, \
>> +        { .i64 = BSF_ELEMENT_PASS }, \
>> +        BSF_ELEMENT_PASS, BSF_ELEMENT_EXTRACT, FLAGS, unit_name }, \
>> +    { "pass",   NULL, 0, AV_OPT_TYPE_CONST, \
>> +        { .i64 = BSF_ELEMENT_PASS   }, .flags = FLAGS, .unit = unit_name }, \
>> +    { "insert", NULL, 0, AV_OPT_TYPE_CONST, \
>> +        { .i64 = BSF_ELEMENT_INSERT }, .flags = FLAGS, .unit = unit_name }, \
>> +    { "remove", NULL, 0, AV_OPT_TYPE_CONST, \
>> +        { .i64 = BSF_ELEMENT_REMOVE }, .flags = FLAGS, .unit = unit_name }, \
>> +    { "extract", NULL, 0, AV_OPT_TYPE_CONST, \
>> +        { .i64 = BSF_ELEMENT_EXTRACT }, .flags = FLAGS, .unit = unit_name } \
Thanks,

- Mark
diff mbox series

Patch

diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index 1fcb94fa48..6c1e7a1c7a 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -69,7 +69,7 @@  OBJS-$(CONFIG_AUDIODSP)                += audiodsp.o
 OBJS-$(CONFIG_BLOCKDSP)                += blockdsp.o
 OBJS-$(CONFIG_BSWAPDSP)                += bswapdsp.o
 OBJS-$(CONFIG_CABAC)                   += cabac.o
-OBJS-$(CONFIG_CBS)                     += cbs.o
+OBJS-$(CONFIG_CBS)                     += cbs.o cbs_bsf.o
 OBJS-$(CONFIG_CBS_AV1)                 += cbs_av1.o
 OBJS-$(CONFIG_CBS_H264)                += cbs_h2645.o cbs_sei.o h2645_parse.o
 OBJS-$(CONFIG_CBS_H265)                += cbs_h2645.o cbs_sei.o h2645_parse.o
diff --git a/libavcodec/cbs_bsf.c b/libavcodec/cbs_bsf.c
new file mode 100644
index 0000000000..9b521cf111
--- /dev/null
+++ b/libavcodec/cbs_bsf.c
@@ -0,0 +1,159 @@ 
+/*
+ * 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 "bsf_internal.h"
+#include "cbs_bsf.h"
+
+static int cbs_bsf_update_side_data(AVBSFContext *bsf, AVPacket *pkt)
+{
+    CBSBSFContext           *ctx = bsf->priv_data;
+    CodedBitstreamFragment *frag = &ctx->fragment;
+    uint8_t *side_data;
+    int side_data_size;
+    int err;
+
+    side_data = av_packet_get_side_data(pkt, AV_PKT_DATA_NEW_EXTRADATA,
+                                        &side_data_size);
+    if (!side_data_size)
+        return 0;
+
+    err = ff_cbs_read(ctx->input, frag, side_data, side_data_size);
+    if (err < 0) {
+        av_log(bsf, AV_LOG_ERROR,
+               "Failed to read extradata from packet side data.\n");
+        return err;
+    }
+
+    err = ctx->type->update_fragment(bsf, NULL, frag);
+    if (err < 0)
+        return err;
+
+    err = ff_cbs_write_fragment_data(ctx->output, frag);
+    if (err < 0) {
+        av_log(bsf, AV_LOG_ERROR,
+               "Failed to write extradata into packet side data.\n");
+        return err;
+    }
+
+    side_data = av_packet_new_side_data(pkt, AV_PKT_DATA_NEW_EXTRADATA,
+                                        frag->data_size);
+    if (!side_data)
+        return AVERROR(ENOMEM);
+    memcpy(side_data, frag->data, frag->data_size);
+
+    ff_cbs_fragment_reset(frag);
+    return 0;
+}
+
+int ff_cbs_bsf_generic_filter(AVBSFContext *bsf, AVPacket *pkt)
+{
+    CBSBSFContext           *ctx = bsf->priv_data;
+    CodedBitstreamFragment *frag = &ctx->fragment;
+    int err;
+
+    err = ff_bsf_get_packet_ref(bsf, pkt);
+    if (err < 0)
+        return err;
+
+    err = cbs_bsf_update_side_data(bsf, pkt);
+    if (err < 0)
+        goto fail;
+
+    err = ff_cbs_read_packet(ctx->input, frag, pkt);
+    if (err < 0) {
+        av_log(bsf, AV_LOG_ERROR, "Failed to read %s from packet.\n",
+               ctx->type->fragment_name);
+        goto fail;
+    }
+
+    if (frag->nb_units == 0) {
+        av_log(bsf, AV_LOG_ERROR, "No %s found in packet.\n",
+               ctx->type->unit_name);
+        err = AVERROR_INVALIDDATA;
+        goto fail;
+    }
+
+    err = ctx->type->update_fragment(bsf, pkt, frag);
+    if (err < 0)
+        goto fail;
+
+    err = ff_cbs_write_packet(ctx->output, pkt, frag);
+    if (err < 0) {
+        av_log(bsf, AV_LOG_ERROR, "Failed to write %s into packet.\n",
+               ctx->type->fragment_name);
+        goto fail;
+    }
+
+    err = 0;
+fail:
+    ff_cbs_fragment_reset(frag);
+
+    if (err < 0)
+        av_packet_unref(pkt);
+
+    return err;
+}
+
+int ff_cbs_bsf_generic_init(AVBSFContext *bsf, const CBSBSFType *type)
+{
+    CBSBSFContext           *ctx = bsf->priv_data;
+    CodedBitstreamFragment *frag = &ctx->fragment;
+    int err;
+
+    ctx->type = type;
+
+    err = ff_cbs_init(&ctx->input, type->codec_id, bsf);
+    if (err < 0)
+        return err;
+
+    err = ff_cbs_init(&ctx->output, type->codec_id, bsf);
+    if (err < 0)
+        return err;
+
+    if (bsf->par_in->extradata) {
+        err = ff_cbs_read_extradata(ctx->input, frag, bsf->par_in);
+        if (err < 0) {
+            av_log(bsf, AV_LOG_ERROR, "Failed to read extradata.\n");
+            goto fail;
+        }
+
+        err = type->update_fragment(bsf, NULL, frag);
+        if (err < 0)
+            goto fail;
+
+        err = ff_cbs_write_extradata(ctx->output, bsf->par_out, frag);
+        if (err < 0) {
+            av_log(bsf, AV_LOG_ERROR, "Failed to write extradata.\n");
+            goto fail;
+        }
+    }
+
+    err = 0;
+fail:
+    ff_cbs_fragment_reset(frag);
+    return err;
+}
+
+void ff_cbs_bsf_generic_close(AVBSFContext *bsf)
+{
+    CBSBSFContext *ctx = bsf->priv_data;
+
+    ff_cbs_fragment_free(&ctx->fragment);
+    ff_cbs_close(&ctx->input);
+    ff_cbs_close(&ctx->output);
+}
diff --git a/libavcodec/cbs_bsf.h b/libavcodec/cbs_bsf.h
new file mode 100644
index 0000000000..5b1a88d4cb
--- /dev/null
+++ b/libavcodec/cbs_bsf.h
@@ -0,0 +1,131 @@ 
+/*
+ * 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
+ */
+
+#ifndef AVCODEC_CBS_BSF_H
+#define AVCODEC_CBS_BSF_H
+
+#include "cbs.h"
+
+
+typedef struct CBSBSFType {
+    enum AVCodecID codec_id;
+
+    // Name of a frame fragment in this codec (e.g. "access unit",
+    // "temporal unit").
+    const char *fragment_name;
+
+    // Name of a unit for this BSF, for use in error messages (e.g.
+    // "NAL unit", "OBU").
+    const char *unit_name;
+
+    // Update the content of a fragment with whatever metadata changes
+    // are desired.  The associated AVPacket is provided so that any side
+    // data associated with the fragment can be inspected or edited.  If
+    // pkt is NULL, then an extradata header fragment is being updated.
+    int (*update_fragment)(AVBSFContext *bsf, AVPacket *pkt,
+                           CodedBitstreamFragment *frag);
+} CBSBSFType;
+
+// Common structure for all generic CBS BSF users.  An instance of this
+// structure must be the first member of the BSF private context (to be
+// pointed to by AVBSFContext.priv_data).
+typedef struct CBSBSFContext {
+    const AVClass         *class;
+    const CBSBSFType      *type;
+
+    CodedBitstreamContext *input;
+    CodedBitstreamContext *output;
+    CodedBitstreamFragment fragment;
+} CBSBSFContext;
+
+/**
+ * Initialise generic CBS BSF setup.
+ *
+ * Creates the input and output CBS instances, and applies the filter to
+ * the extradata on the input codecpar if any is present.
+ *
+ * Since it calls the update_fragment() function immediately to deal with
+ * extradata, this should be called after any codec-specific setup is done
+ * (probably at the end of the AVBitStreamFilter.init function).
+ */
+int ff_cbs_bsf_generic_init(AVBSFContext *bsf, const CBSBSFType *type);
+
+/**
+ * Close a generic CBS BSF instance.
+ *
+ * If no other deinitialisation is required then this function can be used
+ * directly as AVBitStreamFilter.close.
+ */
+void ff_cbs_bsf_generic_close(AVBSFContext *bsf);
+
+/**
+ * Filter operation for CBS BSF.
+ *
+ * Reads the input packet into a CBS fragment, calls update_fragment() on
+ * it, then writes the result to an output packet.  If the input packet
+ * has AV_PKT_DATA_NEW_EXTRADATA side-data associated with it then it does
+ * the same thing to that new extradata to form the output side-data first.
+ *
+ * If the BSF does not do anything else then this function can be used
+ * directly as AVBitStreamFilter.filter.
+ */
+int ff_cbs_bsf_generic_filter(AVBSFContext *bsf, AVPacket *pkt);
+
+
+// Options for element manipulation.
+enum {
+    // Pass this element through unchanged.
+    BSF_ELEMENT_PASS,
+    // Insert this element, replacing any existing instances of it.
+    // Associated values may be provided explicitly (as addtional options)
+    // or implicitly (either as side data or deduced from other parts of
+    // the stream).
+    BSF_ELEMENT_INSERT,
+    // Remove this element if it appears in the stream.
+    BSF_ELEMENT_REMOVE,
+    // Extract this element to side data, so that further manipulation
+    // can happen elsewhere.
+    BSF_ELEMENT_EXTRACT,
+};
+
+#define BSF_ELEMENT_OPTIONS_PIR(name, help, field, unit_name) \
+    { name, help, OFFSET(field), AV_OPT_TYPE_INT, \
+        { .i64 = BSF_ELEMENT_PASS }, \
+        BSF_ELEMENT_PASS, BSF_ELEMENT_REMOVE, FLAGS, unit_name }, \
+    { "pass",   NULL, 0, AV_OPT_TYPE_CONST, \
+        { .i64 = BSF_ELEMENT_PASS   }, .flags = FLAGS, .unit = unit_name }, \
+    { "insert", NULL, 0, AV_OPT_TYPE_CONST, \
+        { .i64 = BSF_ELEMENT_INSERT }, .flags = FLAGS, .unit = unit_name }, \
+    { "remove", NULL, 0, AV_OPT_TYPE_CONST, \
+        { .i64 = BSF_ELEMENT_REMOVE }, .flags = FLAGS, .unit = unit_name }
+
+#define BSF_ELEMENT_OPTIONS_PIRE(name, help, field, unit_name) \
+    { name, help, OFFSET(field), AV_OPT_TYPE_INT, \
+        { .i64 = BSF_ELEMENT_PASS }, \
+        BSF_ELEMENT_PASS, BSF_ELEMENT_EXTRACT, FLAGS, unit_name }, \
+    { "pass",   NULL, 0, AV_OPT_TYPE_CONST, \
+        { .i64 = BSF_ELEMENT_PASS   }, .flags = FLAGS, .unit = unit_name }, \
+    { "insert", NULL, 0, AV_OPT_TYPE_CONST, \
+        { .i64 = BSF_ELEMENT_INSERT }, .flags = FLAGS, .unit = unit_name }, \
+    { "remove", NULL, 0, AV_OPT_TYPE_CONST, \
+        { .i64 = BSF_ELEMENT_REMOVE }, .flags = FLAGS, .unit = unit_name }, \
+    { "extract", NULL, 0, AV_OPT_TYPE_CONST, \
+        { .i64 = BSF_ELEMENT_EXTRACT }, .flags = FLAGS, .unit = unit_name } \
+
+
+#endif /* AVCODEC_CBS_BSF_H */