| Message ID | 20210101213537.169546-8-sw@jkqxz.net |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Metadata handling in CBS | expand |
| 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 |
Mark Thompson: > This allows removal of a lot of duplicated code between BSFs. > --- > libavcodec/Makefile | 2 +- > libavcodec/cbs_bsf.c | 161 +++++++++++++++++++++++++++++++++++++++++++ > libavcodec/cbs_bsf.h | 100 +++++++++++++++++++++++++++ > 3 files changed, 262 insertions(+), 1 deletion(-) > create mode 100644 libavcodec/cbs_bsf.c > create mode 100644 libavcodec/cbs_bsf.h > > diff --git a/libavcodec/Makefile b/libavcodec/Makefile > index 6e12a8171d..8f50217ad4 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..429f360014 > --- /dev/null > +++ b/libavcodec/cbs_bsf.c > @@ -0,0 +1,161 @@ > +/* > + * 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 ff_cbs_bsf_update_side_data(AVBSFContext *bsf, AVPacket *pkt) The ff_ prefix is not for static functions. > +{ > + 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) > + goto fail; This is inconsistent: All the other errors paths out of this function do not reset the fragment; and that is fine, because the one and only caller already does so. (Did you intend for this function to be more standalone?) > + > + 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); > + > + err = 0; > +fail: > + ff_cbs_fragment_reset(frag); > + return err; > +} > + > +int ff_cbs_bsf_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 = ff_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_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_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..8cab3f144c > --- /dev/null > +++ b/libavcodec/cbs_bsf.h > @@ -0,0 +1,100 @@ > +/* > + * 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; > + > +typedef struct CBSBSFContext { > + const AVClass *class; > + const CBSBSFType *type; > + > + CodedBitstreamContext *input; > + CodedBitstreamContext *output; > + CodedBitstreamFragment fragment; > +} CBSBSFContext; > + > + > +int ff_cbs_bsf_filter(AVBSFContext *bsf, AVPacket *pkt); > +int ff_cbs_bsf_init(AVBSFContext *bsf, const CBSBSFType *type); > +void ff_cbs_bsf_close(AVBSFContext *bsf); > + > + > +// 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 */ >
On 01/01/2021 22:18, Andreas Rheinhardt wrote: > Mark Thompson: >> This allows removal of a lot of duplicated code between BSFs. >> --- >> libavcodec/Makefile | 2 +- >> libavcodec/cbs_bsf.c | 161 +++++++++++++++++++++++++++++++++++++++++++ >> libavcodec/cbs_bsf.h | 100 +++++++++++++++++++++++++++ >> 3 files changed, 262 insertions(+), 1 deletion(-) >> create mode 100644 libavcodec/cbs_bsf.c >> create mode 100644 libavcodec/cbs_bsf.h >> >> diff --git a/libavcodec/Makefile b/libavcodec/Makefile >> index 6e12a8171d..8f50217ad4 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..429f360014 >> --- /dev/null >> +++ b/libavcodec/cbs_bsf.c >> @@ -0,0 +1,161 @@ >> +/* >> + * 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 ff_cbs_bsf_update_side_data(AVBSFContext *bsf, AVPacket *pkt) > > The ff_ prefix is not for static functions. > >> +{ >> + 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) >> + goto fail; > > This is inconsistent: All the other errors paths out of this function do > not reset the fragment; and that is fine, because the one and only > caller already does so. (Did you intend for this function to be more > standalone?) IIRC it originally was a standalone function, but got changed when it was clear that every BSF was calling it in exactly the same way. (None of this code has changed since the previous version in August.) The reset is needed, because we are going to use the same fragment in read_packet() immediately after the return. So, added the gotos to all other error cases except the first. >> + >> + 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); >> + >> + err = 0; >> +fail: >> + ff_cbs_fragment_reset(frag); >> + return err; >> +} >> + >> ... Thanks, - Mark
Mark Thompson: > On 01/01/2021 22:18, Andreas Rheinhardt wrote: >> Mark Thompson: >>> This allows removal of a lot of duplicated code between BSFs. >>> --- >>> libavcodec/Makefile | 2 +- >>> libavcodec/cbs_bsf.c | 161 +++++++++++++++++++++++++++++++++++++++++++ >>> libavcodec/cbs_bsf.h | 100 +++++++++++++++++++++++++++ >>> 3 files changed, 262 insertions(+), 1 deletion(-) >>> create mode 100644 libavcodec/cbs_bsf.c >>> create mode 100644 libavcodec/cbs_bsf.h >>> >>> diff --git a/libavcodec/Makefile b/libavcodec/Makefile >>> index 6e12a8171d..8f50217ad4 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..429f360014 >>> --- /dev/null >>> +++ b/libavcodec/cbs_bsf.c >>> @@ -0,0 +1,161 @@ >>> +/* >>> + * 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 ff_cbs_bsf_update_side_data(AVBSFContext *bsf, AVPacket >>> *pkt) >> >> The ff_ prefix is not for static functions. >> >>> +{ >>> + 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) >>> + goto fail; >> >> This is inconsistent: All the other errors paths out of this function do >> not reset the fragment; and that is fine, because the one and only >> caller already does so. (Did you intend for this function to be more >> standalone?) > > IIRC it originally was a standalone function, but got changed when it > was clear that every BSF was calling it in exactly the same way. (None > of this code has changed since the previous version in August.) > > The reset is needed, because we are going to use the same fragment in > read_packet() immediately after the return. So, added the gotos to all > other error cases except the first. > The reset here is redundant, because the only caller already resets the fragment if ff_cbs_bsf_update_side_data returns an error. And ff_cbs_read() returns unclean fragments on error, so treating it differently than an error from update_fragment is inconsistent (albeit not dangerous because (as said) the only caller resets the fragment). >>> + >>> + 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); >>> + >>> + err = 0; >>> +fail: >>> + ff_cbs_fragment_reset(frag); >>> + return err; >>> +} >>> + >>> ... > > Thanks, > > - Mark > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
On 06/01/2021 20:01, Andreas Rheinhardt wrote: > Mark Thompson: >> On 01/01/2021 22:18, Andreas Rheinhardt wrote: >>> Mark Thompson: >>>> This allows removal of a lot of duplicated code between BSFs. >>>> --- >>>> libavcodec/Makefile | 2 +- >>>> libavcodec/cbs_bsf.c | 161 +++++++++++++++++++++++++++++++++++++++++++ >>>> libavcodec/cbs_bsf.h | 100 +++++++++++++++++++++++++++ >>>> 3 files changed, 262 insertions(+), 1 deletion(-) >>>> create mode 100644 libavcodec/cbs_bsf.c >>>> create mode 100644 libavcodec/cbs_bsf.h >>>> >>>> diff --git a/libavcodec/Makefile b/libavcodec/Makefile >>>> index 6e12a8171d..8f50217ad4 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..429f360014 >>>> --- /dev/null >>>> +++ b/libavcodec/cbs_bsf.c >>>> @@ -0,0 +1,161 @@ >>>> +/* >>>> + * 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 ff_cbs_bsf_update_side_data(AVBSFContext *bsf, AVPacket >>>> *pkt) >>> >>> The ff_ prefix is not for static functions. >>> >>>> +{ >>>> + 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) >>>> + goto fail; >>> >>> This is inconsistent: All the other errors paths out of this function do >>> not reset the fragment; and that is fine, because the one and only >>> caller already does so. (Did you intend for this function to be more >>> standalone?) >> >> IIRC it originally was a standalone function, but got changed when it >> was clear that every BSF was calling it in exactly the same way. (None >> of this code has changed since the previous version in August.) >> >> The reset is needed, because we are going to use the same fragment in >> read_packet() immediately after the return. So, added the gotos to all >> other error cases except the first. >> > > The reset here is redundant, because the only caller already resets the > fragment if ff_cbs_bsf_update_side_data returns an error. > > And ff_cbs_read() returns unclean fragments on error, so treating it > differently than an error from update_fragment is inconsistent (albeit > not dangerous because (as said) the only caller resets the fragment). Oops, yes, you are correct. I was thinking about read_packet() being called after on the dirty fragment, but obviously that only happens if this didn't return an error so we only need the reset on success. Removed all of the gotos. >>>> + >>>> + 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); >>>> + >>>> + err = 0; >>>> +fail: >>>> + ff_cbs_fragment_reset(frag); >>>> + return err; >>>> +} >>>> + >>>> ... Thanks, - Mark
diff --git a/libavcodec/Makefile b/libavcodec/Makefile index 6e12a8171d..8f50217ad4 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..429f360014 --- /dev/null +++ b/libavcodec/cbs_bsf.c @@ -0,0 +1,161 @@ +/* + * 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 ff_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) + goto fail; + + 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); + + err = 0; +fail: + ff_cbs_fragment_reset(frag); + return err; +} + +int ff_cbs_bsf_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 = ff_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_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_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..8cab3f144c --- /dev/null +++ b/libavcodec/cbs_bsf.h @@ -0,0 +1,100 @@ +/* + * 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; + +typedef struct CBSBSFContext { + const AVClass *class; + const CBSBSFType *type; + + CodedBitstreamContext *input; + CodedBitstreamContext *output; + CodedBitstreamFragment fragment; +} CBSBSFContext; + + +int ff_cbs_bsf_filter(AVBSFContext *bsf, AVPacket *pkt); +int ff_cbs_bsf_init(AVBSFContext *bsf, const CBSBSFType *type); +void ff_cbs_bsf_close(AVBSFContext *bsf); + + +// 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 */