| Message ID | 20191111185717.1939-1-jamrial@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
James Almer: > This BSF takes Temporal Units split across different AVPackets and merges them > by looking for Temporal Delimiter OBUs. > > Signed-off-by: James Almer <jamrial@gmail.com> > --- > Now copying packet props. > > configure | 1 + > libavcodec/Makefile | 1 + > libavcodec/av1_frame_merge_bsf.c | 172 +++++++++++++++++++++++++++++++ > libavcodec/bitstream_filters.c | 1 + > 4 files changed, 175 insertions(+) > create mode 100644 libavcodec/av1_frame_merge_bsf.c > > diff --git a/configure b/configure > index 1de90e93fd..70f60997c1 100755 > --- a/configure > +++ b/configure > @@ -3115,6 +3115,7 @@ vc1_parser_select="vc1dsp" > > # bitstream_filters > aac_adtstoasc_bsf_select="adts_header" > +av1_frame_merge_bsf_select="cbs_av1" > av1_frame_split_bsf_select="cbs_av1" > av1_metadata_bsf_select="cbs_av1" > eac3_core_bsf_select="ac3_parser" > diff --git a/libavcodec/Makefile b/libavcodec/Makefile > index b990c6ba87..006a472a6d 100644 > --- a/libavcodec/Makefile > +++ b/libavcodec/Makefile > @@ -1075,6 +1075,7 @@ OBJS-$(CONFIG_XMA_PARSER) += xma_parser.o > # bitstream filters > OBJS-$(CONFIG_AAC_ADTSTOASC_BSF) += aac_adtstoasc_bsf.o mpeg4audio.o > OBJS-$(CONFIG_AV1_METADATA_BSF) += av1_metadata_bsf.o > +OBJS-$(CONFIG_AV1_FRAME_MERGE_BSF) += av1_frame_merge_bsf.o > OBJS-$(CONFIG_AV1_FRAME_SPLIT_BSF) += av1_frame_split_bsf.o > OBJS-$(CONFIG_CHOMP_BSF) += chomp_bsf.o > OBJS-$(CONFIG_DUMP_EXTRADATA_BSF) += dump_extradata_bsf.o > diff --git a/libavcodec/av1_frame_merge_bsf.c b/libavcodec/av1_frame_merge_bsf.c > new file mode 100644 > index 0000000000..72b065b664 > --- /dev/null > +++ b/libavcodec/av1_frame_merge_bsf.c > @@ -0,0 +1,172 @@ > +/* > + * Copyright (c) 2019 James Almer <jamrial@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 > + */ > + > +#include "avcodec.h" > +#include "bsf.h" > +#include "cbs.h" > +#include "cbs_av1.h" > + > +typedef struct AV1FMergeContext { > + CodedBitstreamContext *cbc; > + CodedBitstreamFragment temporal_unit; > + CodedBitstreamFragment frag; > + AVPacket *buffer_pkt; > +} AV1FMergeContext; > + > +static int av1_frame_merge_filter(AVBSFContext *bsf, AVPacket *out) > +{ > + AV1FMergeContext *ctx = bsf->priv_data; > + CodedBitstreamFragment *frag = &ctx->frag, *tu = &ctx->temporal_unit; > + AVPacket *in = NULL; > + int err, i; > + > + err = ff_bsf_get_packet(bsf, &in); The allocation that is implicit in ff_bsf_get_packet() can be avoided by putting an AVPacket on the stack here and using this for reading; or even better: Directly read into buffer_pkt if tu->nb_units vanishes and read into the other packet in the other case. Or you can use two buffer_pkt and switch between them; this should eliminate all av_packet_move_ref(). > + if (err < 0) { > + if (err == AVERROR_EOF && tu->nb_units > 0) > + goto eof; > + return err; > + } > + > + err = ff_cbs_read_packet(ctx->cbc, frag, in); > + if (err < 0) { > + av_log(bsf, AV_LOG_ERROR, "Failed to read packet.\n"); > + goto fail; > + } > + > + if (frag->nb_units == 0) { > + av_log(bsf, AV_LOG_ERROR, "No OBU in packet.\n"); > + err = AVERROR_INVALIDDATA; > + goto fail; > + } > + > + if (tu->nb_units == 0 && frag->units[0].type != AV1_OBU_TEMPORAL_DELIMITER) { > + av_log(bsf, AV_LOG_ERROR, "Missing Temporal Delimiter.\n"); > + err = AVERROR_INVALIDDATA; > + goto fail; > + } > + > + if (tu->nb_units > 0 && frag->units[0].type == AV1_OBU_TEMPORAL_DELIMITER) { > +eof: > + err = ff_cbs_write_packet(ctx->cbc, out, tu); There are some complications here with respect to OBU units for which decomposition is unimplemented (i.e. the reserved types): In contrast to cbs_h2645, cbs_jpeg and cbs_mpeg2 cbs_av1 sets the content of all the units it reads, even for those that it can't decompose. cbs_read_fragment_content() then emits its "Decomposition unimplemented" warning, but it doesn't forward the error. So you end up with a unit with content where the content doesn't contain the information to rebuild the unit. Writing them will fail with AVERROR(ENOSYS). I see two solutions for this: a) The obvious solution "don't allocate content like the other cbs_*" would have the drawback that one could no longer enforce the presence of the size field; unless one would somehow store obu header and extension header externally (shoehorn it into unit_type?) and consider the unit's data to be everything after the header and leave writing the obu header to assemble_fragment. This would have the advantage of allowing to stop decomposing certain unit types (if I am not mistaken, filter_units currently only works when fed with low-overhead bitstream format). b) One could add a new type of obu to the union in AV1RawOBU similar to the padding OBU (it could even share code with the padding OBU) so that the content contains all the data necessary to rebuild the unit. This would have the advantage that your current method works (if the content does not exist, ff_cbs_insert_unit_content is not the right function). Of course, we need to consult Mark about this. > + if (err < 0) { > + av_log(bsf, AV_LOG_ERROR, "Failed to write packet.\n"); > + goto fail; > + } > + err = av_packet_copy_props(out, ctx->buffer_pkt); > + if (err < 0) { > + av_log(bsf, AV_LOG_ERROR, "Failed to copy packet props.\n"); > + goto fail; > + } > + av_packet_unref(ctx->buffer_pkt); > + > + ff_cbs_fragment_reset(ctx->cbc, tu); > + for (i = 0; i < frag->nb_units; i++) { > + if (i && frag->units[i].type == AV1_OBU_TEMPORAL_DELIMITER) { > + av_log(bsf, AV_LOG_ERROR, "Temporal Delimiter in the middle of a packet.\n"); > + err = AVERROR_INVALIDDATA; > + goto fail; > + } > + err = ff_cbs_insert_unit_content(ctx->cbc, tu, -1, frag->units[i].type, > + frag->units[i].content, frag->units[i].content_ref); You could remove this and the reset a few lines below if you adaptively switched the roles of the fragments. > + if (err < 0) > + goto fail; > + } > + ff_cbs_fragment_reset(ctx->cbc, frag); > + > + if (in) > + av_packet_move_ref(ctx->buffer_pkt, in); > + av_packet_free(&in); > + > + return 0; > + } > + > + if (tu->nb_units == 0) > + av_packet_move_ref(ctx->buffer_pkt, in); > + > + for (i = 0; i < frag->nb_units; i++) { > + if (i && frag->units[i].type == AV1_OBU_TEMPORAL_DELIMITER) { > + av_log(bsf, AV_LOG_ERROR, "Temporal Delimiter in the middle of a packet.\n"); > + err = AVERROR_INVALIDDATA; > + goto fail; > + } > + err = ff_cbs_insert_unit_content(ctx->cbc, tu, -1, frag->units[i].type, > + frag->units[i].content, frag->units[i].content_ref); You make new references to content and then immediately thereafter unref the old ones. How about adding a function that moves a unit (or several units at once) from one fragment to another? > + if (err < 0) > + goto fail; > + } > + ff_cbs_fragment_reset(ctx->cbc, frag); > + > + av_packet_free(&in); > + > + return err < 0 ? err : AVERROR(EAGAIN); Isn't it impossible to reach this point and have err < 0? > + > +fail: > + ff_cbs_fragment_reset(ctx->cbc, tu); > + ff_cbs_fragment_reset(ctx->cbc, frag); > + av_packet_unref(ctx->buffer_pkt); > + av_packet_unref(out); > + av_packet_free(&in); > + > + return err; > +} > + > +static int av1_frame_merge_init(AVBSFContext *bsf) > +{ > + AV1FMergeContext *ctx = bsf->priv_data; > + > + ctx->buffer_pkt = av_packet_alloc(); > + if (!ctx->buffer_pkt) > + return AVERROR(ENOMEM); Why a separate allocation for buffer_pkt? This is libavcodec, so there is no issue with sizeof(AVPacket). > + > + return ff_cbs_init(&ctx->cbc, AV_CODEC_ID_AV1, bsf);; > +} > + > +static void av1_frame_merge_flush(AVBSFContext *bsf) > +{ > + AV1FMergeContext *ctx = bsf->priv_data; > + > + ff_cbs_fragment_reset(ctx->cbc, &ctx->temporal_unit); > + ff_cbs_fragment_reset(ctx->cbc, &ctx->frag); > + av_packet_unref(ctx->buffer_pkt); > +} > + > +static void av1_frame_merge_close(AVBSFContext *bsf) > +{ > + AV1FMergeContext *ctx = bsf->priv_data; > + > + ff_cbs_fragment_free(ctx->cbc, &ctx->temporal_unit); > + ff_cbs_fragment_free(ctx->cbc, &ctx->frag); > + av_packet_free(&ctx->buffer_pkt); > + ff_cbs_close(&ctx->cbc); > +} > + > +static const enum AVCodecID av1_frame_merge_codec_ids[] = { > + AV_CODEC_ID_AV1, AV_CODEC_ID_NONE, > +}; > + > +const AVBitStreamFilter ff_av1_frame_merge_bsf = { > + .name = "av1_frame_merge", > + .priv_data_size = sizeof(AV1FMergeContext), > + .init = av1_frame_merge_init, > + .flush = av1_frame_merge_flush, > + .close = av1_frame_merge_close, > + .filter = av1_frame_merge_filter, > + .codec_ids = av1_frame_merge_codec_ids, > +}; > diff --git a/libavcodec/bitstream_filters.c b/libavcodec/bitstream_filters.c > index 463003966a..6b5ffe4d70 100644 > --- a/libavcodec/bitstream_filters.c > +++ b/libavcodec/bitstream_filters.c > @@ -25,6 +25,7 @@ > #include "bsf.h" > > extern const AVBitStreamFilter ff_aac_adtstoasc_bsf; > +extern const AVBitStreamFilter ff_av1_frame_merge_bsf; > extern const AVBitStreamFilter ff_av1_frame_split_bsf; > extern const AVBitStreamFilter ff_av1_metadata_bsf; > extern const AVBitStreamFilter ff_chomp_bsf; >
On 11/11/2019 7:45 PM, Andreas Rheinhardt wrote: > James Almer: >> This BSF takes Temporal Units split across different AVPackets and merges them >> by looking for Temporal Delimiter OBUs. >> >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> Now copying packet props. >> >> configure | 1 + >> libavcodec/Makefile | 1 + >> libavcodec/av1_frame_merge_bsf.c | 172 +++++++++++++++++++++++++++++++ >> libavcodec/bitstream_filters.c | 1 + >> 4 files changed, 175 insertions(+) >> create mode 100644 libavcodec/av1_frame_merge_bsf.c >> >> diff --git a/configure b/configure >> index 1de90e93fd..70f60997c1 100755 >> --- a/configure >> +++ b/configure >> @@ -3115,6 +3115,7 @@ vc1_parser_select="vc1dsp" >> >> # bitstream_filters >> aac_adtstoasc_bsf_select="adts_header" >> +av1_frame_merge_bsf_select="cbs_av1" >> av1_frame_split_bsf_select="cbs_av1" >> av1_metadata_bsf_select="cbs_av1" >> eac3_core_bsf_select="ac3_parser" >> diff --git a/libavcodec/Makefile b/libavcodec/Makefile >> index b990c6ba87..006a472a6d 100644 >> --- a/libavcodec/Makefile >> +++ b/libavcodec/Makefile >> @@ -1075,6 +1075,7 @@ OBJS-$(CONFIG_XMA_PARSER) += xma_parser.o >> # bitstream filters >> OBJS-$(CONFIG_AAC_ADTSTOASC_BSF) += aac_adtstoasc_bsf.o mpeg4audio.o >> OBJS-$(CONFIG_AV1_METADATA_BSF) += av1_metadata_bsf.o >> +OBJS-$(CONFIG_AV1_FRAME_MERGE_BSF) += av1_frame_merge_bsf.o >> OBJS-$(CONFIG_AV1_FRAME_SPLIT_BSF) += av1_frame_split_bsf.o >> OBJS-$(CONFIG_CHOMP_BSF) += chomp_bsf.o >> OBJS-$(CONFIG_DUMP_EXTRADATA_BSF) += dump_extradata_bsf.o >> diff --git a/libavcodec/av1_frame_merge_bsf.c b/libavcodec/av1_frame_merge_bsf.c >> new file mode 100644 >> index 0000000000..72b065b664 >> --- /dev/null >> +++ b/libavcodec/av1_frame_merge_bsf.c >> @@ -0,0 +1,172 @@ >> +/* >> + * Copyright (c) 2019 James Almer <jamrial@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 >> + */ >> + >> +#include "avcodec.h" >> +#include "bsf.h" >> +#include "cbs.h" >> +#include "cbs_av1.h" >> + >> +typedef struct AV1FMergeContext { >> + CodedBitstreamContext *cbc; >> + CodedBitstreamFragment temporal_unit; >> + CodedBitstreamFragment frag; >> + AVPacket *buffer_pkt; >> +} AV1FMergeContext; >> + >> +static int av1_frame_merge_filter(AVBSFContext *bsf, AVPacket *out) >> +{ >> + AV1FMergeContext *ctx = bsf->priv_data; >> + CodedBitstreamFragment *frag = &ctx->frag, *tu = &ctx->temporal_unit; >> + AVPacket *in = NULL; >> + int err, i; >> + >> + err = ff_bsf_get_packet(bsf, &in); > > The allocation that is implicit in ff_bsf_get_packet() can be avoided > by putting an AVPacket on the stack here and using this for reading; > or even better: Directly read into buffer_pkt if tu->nb_units vanishes > and read into the other packet in the other case. Or you can use two > buffer_pkt and switch between them; this should eliminate all > av_packet_move_ref(). I'm purposely avoiding storing AVPackets on stack, or using sizeof(AVPacket) altogether. Afaik we want to make them behave like AVFrames in the long term, and that starts by not adding more code that will have to be adapted eventually. I do agree an alternative using an extra AVPacket in AV1FMergeContext may be worth looking into to instead use ff_bsf_get_packet_ref() here. > >> + if (err < 0) { >> + if (err == AVERROR_EOF && tu->nb_units > 0) >> + goto eof; >> + return err; >> + } >> + >> + err = ff_cbs_read_packet(ctx->cbc, frag, in); >> + if (err < 0) { >> + av_log(bsf, AV_LOG_ERROR, "Failed to read packet.\n"); >> + goto fail; >> + } >> + >> + if (frag->nb_units == 0) { >> + av_log(bsf, AV_LOG_ERROR, "No OBU in packet.\n"); >> + err = AVERROR_INVALIDDATA; >> + goto fail; >> + } >> + >> + if (tu->nb_units == 0 && frag->units[0].type != AV1_OBU_TEMPORAL_DELIMITER) { >> + av_log(bsf, AV_LOG_ERROR, "Missing Temporal Delimiter.\n"); >> + err = AVERROR_INVALIDDATA; >> + goto fail; >> + } >> + >> + if (tu->nb_units > 0 && frag->units[0].type == AV1_OBU_TEMPORAL_DELIMITER) { >> +eof: >> + err = ff_cbs_write_packet(ctx->cbc, out, tu); > > There are some complications here with respect to OBU units for which > decomposition is unimplemented (i.e. the reserved types): In contrast > to cbs_h2645, cbs_jpeg and cbs_mpeg2 cbs_av1 sets the content of all > the units it reads, even for those that it can't decompose. > cbs_read_fragment_content() then emits its "Decomposition > unimplemented" warning, but it doesn't forward the error. So you end> up with a unit with content where the content doesn't contain the > information to rebuild the unit. Writing them will fail with > AVERROR(ENOSYS). > > I see two solutions for this: > a) The obvious solution "don't allocate content like the other cbs_*" > would have the drawback that one could no longer enforce the presence > of the size field; unless one would somehow store obu header and > extension header externally (shoehorn it into unit_type?) and consider > the unit's data to be everything after the header and leave writing > the obu header to assemble_fragment. This would have the advantage of > allowing to stop decomposing certain unit types (if I am not mistaken, > filter_units currently only works when fed with low-overhead bitstream > format). > b) One could add a new type of obu to the union in AV1RawOBU similar > to the padding OBU (it could even share code with the padding OBU) so > that the content contains all the data necessary to rebuild the unit. > This would have the advantage that your current method works (if the > content does not exist, ff_cbs_insert_unit_content is not the right > function). > > Of course, we need to consult Mark about this. Personally, i don't think it's worth worrying about reserved OBU types, or even the currently unimplemented Metadata types. AV1 is unlikely to see new revisions with that kind of addition, and Annex B is a very fringe container used almost exclusively for reference samples. ff_cbs_write_packet() returning a failure if it were to attempt to write them is fine for the time being. But yes, if Mark agrees with you then of course I'll be ok with the above. Preferably solution b. > >> + if (err < 0) { >> + av_log(bsf, AV_LOG_ERROR, "Failed to write packet.\n"); >> + goto fail; >> + } >> + err = av_packet_copy_props(out, ctx->buffer_pkt); >> + if (err < 0) { >> + av_log(bsf, AV_LOG_ERROR, "Failed to copy packet props.\n"); >> + goto fail; >> + } >> + av_packet_unref(ctx->buffer_pkt); >> + >> + ff_cbs_fragment_reset(ctx->cbc, tu); >> + for (i = 0; i < frag->nb_units; i++) { >> + if (i && frag->units[i].type == AV1_OBU_TEMPORAL_DELIMITER) { >> + av_log(bsf, AV_LOG_ERROR, "Temporal Delimiter in the middle of a packet.\n"); >> + err = AVERROR_INVALIDDATA; >> + goto fail; >> + } >> + err = ff_cbs_insert_unit_content(ctx->cbc, tu, -1, frag->units[i].type, >> + frag->units[i].content, frag->units[i].content_ref); > > You could remove this and the reset a few lines below if you > adaptively switched the roles of the fragments. I'd rather look into your suggestion below for a move function. > >> + if (err < 0) >> + goto fail; >> + } >> + ff_cbs_fragment_reset(ctx->cbc, frag); >> + >> + if (in) >> + av_packet_move_ref(ctx->buffer_pkt, in); >> + av_packet_free(&in); >> + >> + return 0; >> + } >> + >> + if (tu->nb_units == 0) >> + av_packet_move_ref(ctx->buffer_pkt, in); >> + >> + for (i = 0; i < frag->nb_units; i++) { >> + if (i && frag->units[i].type == AV1_OBU_TEMPORAL_DELIMITER) { >> + av_log(bsf, AV_LOG_ERROR, "Temporal Delimiter in the middle of a packet.\n"); >> + err = AVERROR_INVALIDDATA; >> + goto fail; >> + } >> + err = ff_cbs_insert_unit_content(ctx->cbc, tu, -1, frag->units[i].type, >> + frag->units[i].content, frag->units[i].content_ref); > > You make new references to content and then immediately thereafter > unref the old ones. How about adding a function that moves a unit (or > several units at once) from one fragment to another? A new function that moves units sounds good. Shouldn't be hard to implement either, i think. > >> + if (err < 0) >> + goto fail; >> + } >> + ff_cbs_fragment_reset(ctx->cbc, frag); >> + >> + av_packet_free(&in); >> + >> + return err < 0 ? err : AVERROR(EAGAIN); > > Isn't it impossible to reach this point and have err < 0? Yeah, it's a remnant of a mid development version before i made all the failures jump to the label below. Will remove. > >> + >> +fail: >> + ff_cbs_fragment_reset(ctx->cbc, tu); >> + ff_cbs_fragment_reset(ctx->cbc, frag); >> + av_packet_unref(ctx->buffer_pkt); >> + av_packet_unref(out); >> + av_packet_free(&in); >> + >> + return err; >> +} >> + >> +static int av1_frame_merge_init(AVBSFContext *bsf) >> +{ >> + AV1FMergeContext *ctx = bsf->priv_data; >> + >> + ctx->buffer_pkt = av_packet_alloc(); >> + if (!ctx->buffer_pkt) >> + return AVERROR(ENOMEM); > > Why a separate allocation for buffer_pkt? This is libavcodec, so there > is no issue with sizeof(AVPacket). See above. And this is init(), so unlike the filter() case with recurrent ff_bsf_get_packet() calls, this allocation is negligible. > >> + >> + return ff_cbs_init(&ctx->cbc, AV_CODEC_ID_AV1, bsf);; >> +} >> + >> +static void av1_frame_merge_flush(AVBSFContext *bsf) >> +{ >> + AV1FMergeContext *ctx = bsf->priv_data; >> + >> + ff_cbs_fragment_reset(ctx->cbc, &ctx->temporal_unit); >> + ff_cbs_fragment_reset(ctx->cbc, &ctx->frag); >> + av_packet_unref(ctx->buffer_pkt); >> +} >> + >> +static void av1_frame_merge_close(AVBSFContext *bsf) >> +{ >> + AV1FMergeContext *ctx = bsf->priv_data; >> + >> + ff_cbs_fragment_free(ctx->cbc, &ctx->temporal_unit); >> + ff_cbs_fragment_free(ctx->cbc, &ctx->frag); >> + av_packet_free(&ctx->buffer_pkt); >> + ff_cbs_close(&ctx->cbc); >> +} >> + >> +static const enum AVCodecID av1_frame_merge_codec_ids[] = { >> + AV_CODEC_ID_AV1, AV_CODEC_ID_NONE, >> +}; >> + >> +const AVBitStreamFilter ff_av1_frame_merge_bsf = { >> + .name = "av1_frame_merge", >> + .priv_data_size = sizeof(AV1FMergeContext), >> + .init = av1_frame_merge_init, >> + .flush = av1_frame_merge_flush, >> + .close = av1_frame_merge_close, >> + .filter = av1_frame_merge_filter, >> + .codec_ids = av1_frame_merge_codec_ids, >> +}; >> diff --git a/libavcodec/bitstream_filters.c b/libavcodec/bitstream_filters.c >> index 463003966a..6b5ffe4d70 100644 >> --- a/libavcodec/bitstream_filters.c >> +++ b/libavcodec/bitstream_filters.c >> @@ -25,6 +25,7 @@ >> #include "bsf.h" >> >> extern const AVBitStreamFilter ff_aac_adtstoasc_bsf; >> +extern const AVBitStreamFilter ff_av1_frame_merge_bsf; >> extern const AVBitStreamFilter ff_av1_frame_split_bsf; >> extern const AVBitStreamFilter ff_av1_metadata_bsf; >> extern const AVBitStreamFilter ff_chomp_bsf; >> > > _______________________________________________ > 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". >
James Almer: > On 11/11/2019 8:36 PM, James Almer wrote: >>>> + for (i = 0; i < frag->nb_units; i++) { >>>> + if (i && frag->units[i].type == AV1_OBU_TEMPORAL_DELIMITER) { >>>> + av_log(bsf, AV_LOG_ERROR, "Temporal Delimiter in the middle of a packet.\n"); >>>> + err = AVERROR_INVALIDDATA; >>>> + goto fail; >>>> + } >>>> + err = ff_cbs_insert_unit_content(ctx->cbc, tu, -1, frag->units[i].type, >>>> + frag->units[i].content, frag->units[i].content_ref); >>> You make new references to content and then immediately thereafter >>> unref the old ones. How about adding a function that moves a unit (or >>> several units at once) from one fragment to another? >> A new function that moves units sounds good. Shouldn't be hard to >> implement either, i think. > > So a unit move function would require a memcpy for the unit from one > fragment to another, a memset to 0 on the source unit, then a call to > ff_cbs_delete_unit() which memmoves the remaining units to fill the gap > created by the removed one, if any. I don't know if it's that much > better than this current implementation creating a new AVBufferRef per > unit, then wiping the whole source fragment in one go. On second thought I think that it's not worth the effort here. After all, said function needs to be cbs-internal any way (it has to make sure that there is enough space in the destination array and for this it needs to examine and possibly modify the number of allocated units) and therefore it should not be a simple "move a whole fragment to the end of another fragment" (that's what I had in mind and this would be pretty simple to implement and also pretty fast as no remaining units needed to be moved). But for the other place where you move units, nothing beats simply swapping the roles of the two fragments. - Andreas
diff --git a/configure b/configure index 1de90e93fd..70f60997c1 100755 --- a/configure +++ b/configure @@ -3115,6 +3115,7 @@ vc1_parser_select="vc1dsp" # bitstream_filters aac_adtstoasc_bsf_select="adts_header" +av1_frame_merge_bsf_select="cbs_av1" av1_frame_split_bsf_select="cbs_av1" av1_metadata_bsf_select="cbs_av1" eac3_core_bsf_select="ac3_parser" diff --git a/libavcodec/Makefile b/libavcodec/Makefile index b990c6ba87..006a472a6d 100644 --- a/libavcodec/Makefile +++ b/libavcodec/Makefile @@ -1075,6 +1075,7 @@ OBJS-$(CONFIG_XMA_PARSER) += xma_parser.o # bitstream filters OBJS-$(CONFIG_AAC_ADTSTOASC_BSF) += aac_adtstoasc_bsf.o mpeg4audio.o OBJS-$(CONFIG_AV1_METADATA_BSF) += av1_metadata_bsf.o +OBJS-$(CONFIG_AV1_FRAME_MERGE_BSF) += av1_frame_merge_bsf.o OBJS-$(CONFIG_AV1_FRAME_SPLIT_BSF) += av1_frame_split_bsf.o OBJS-$(CONFIG_CHOMP_BSF) += chomp_bsf.o OBJS-$(CONFIG_DUMP_EXTRADATA_BSF) += dump_extradata_bsf.o diff --git a/libavcodec/av1_frame_merge_bsf.c b/libavcodec/av1_frame_merge_bsf.c new file mode 100644 index 0000000000..72b065b664 --- /dev/null +++ b/libavcodec/av1_frame_merge_bsf.c @@ -0,0 +1,172 @@ +/* + * Copyright (c) 2019 James Almer <jamrial@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 + */ + +#include "avcodec.h" +#include "bsf.h" +#include "cbs.h" +#include "cbs_av1.h" + +typedef struct AV1FMergeContext { + CodedBitstreamContext *cbc; + CodedBitstreamFragment temporal_unit; + CodedBitstreamFragment frag; + AVPacket *buffer_pkt; +} AV1FMergeContext; + +static int av1_frame_merge_filter(AVBSFContext *bsf, AVPacket *out) +{ + AV1FMergeContext *ctx = bsf->priv_data; + CodedBitstreamFragment *frag = &ctx->frag, *tu = &ctx->temporal_unit; + AVPacket *in = NULL; + int err, i; + + err = ff_bsf_get_packet(bsf, &in); + if (err < 0) { + if (err == AVERROR_EOF && tu->nb_units > 0) + goto eof; + return err; + } + + err = ff_cbs_read_packet(ctx->cbc, frag, in); + if (err < 0) { + av_log(bsf, AV_LOG_ERROR, "Failed to read packet.\n"); + goto fail; + } + + if (frag->nb_units == 0) { + av_log(bsf, AV_LOG_ERROR, "No OBU in packet.\n"); + err = AVERROR_INVALIDDATA; + goto fail; + } + + if (tu->nb_units == 0 && frag->units[0].type != AV1_OBU_TEMPORAL_DELIMITER) { + av_log(bsf, AV_LOG_ERROR, "Missing Temporal Delimiter.\n"); + err = AVERROR_INVALIDDATA; + goto fail; + } + + if (tu->nb_units > 0 && frag->units[0].type == AV1_OBU_TEMPORAL_DELIMITER) { +eof: + err = ff_cbs_write_packet(ctx->cbc, out, tu); + if (err < 0) { + av_log(bsf, AV_LOG_ERROR, "Failed to write packet.\n"); + goto fail; + } + err = av_packet_copy_props(out, ctx->buffer_pkt); + if (err < 0) { + av_log(bsf, AV_LOG_ERROR, "Failed to copy packet props.\n"); + goto fail; + } + av_packet_unref(ctx->buffer_pkt); + + ff_cbs_fragment_reset(ctx->cbc, tu); + for (i = 0; i < frag->nb_units; i++) { + if (i && frag->units[i].type == AV1_OBU_TEMPORAL_DELIMITER) { + av_log(bsf, AV_LOG_ERROR, "Temporal Delimiter in the middle of a packet.\n"); + err = AVERROR_INVALIDDATA; + goto fail; + } + err = ff_cbs_insert_unit_content(ctx->cbc, tu, -1, frag->units[i].type, + frag->units[i].content, frag->units[i].content_ref); + if (err < 0) + goto fail; + } + ff_cbs_fragment_reset(ctx->cbc, frag); + + if (in) + av_packet_move_ref(ctx->buffer_pkt, in); + av_packet_free(&in); + + return 0; + } + + if (tu->nb_units == 0) + av_packet_move_ref(ctx->buffer_pkt, in); + + for (i = 0; i < frag->nb_units; i++) { + if (i && frag->units[i].type == AV1_OBU_TEMPORAL_DELIMITER) { + av_log(bsf, AV_LOG_ERROR, "Temporal Delimiter in the middle of a packet.\n"); + err = AVERROR_INVALIDDATA; + goto fail; + } + err = ff_cbs_insert_unit_content(ctx->cbc, tu, -1, frag->units[i].type, + frag->units[i].content, frag->units[i].content_ref); + if (err < 0) + goto fail; + } + ff_cbs_fragment_reset(ctx->cbc, frag); + + av_packet_free(&in); + + return err < 0 ? err : AVERROR(EAGAIN); + +fail: + ff_cbs_fragment_reset(ctx->cbc, tu); + ff_cbs_fragment_reset(ctx->cbc, frag); + av_packet_unref(ctx->buffer_pkt); + av_packet_unref(out); + av_packet_free(&in); + + return err; +} + +static int av1_frame_merge_init(AVBSFContext *bsf) +{ + AV1FMergeContext *ctx = bsf->priv_data; + + ctx->buffer_pkt = av_packet_alloc(); + if (!ctx->buffer_pkt) + return AVERROR(ENOMEM); + + return ff_cbs_init(&ctx->cbc, AV_CODEC_ID_AV1, bsf);; +} + +static void av1_frame_merge_flush(AVBSFContext *bsf) +{ + AV1FMergeContext *ctx = bsf->priv_data; + + ff_cbs_fragment_reset(ctx->cbc, &ctx->temporal_unit); + ff_cbs_fragment_reset(ctx->cbc, &ctx->frag); + av_packet_unref(ctx->buffer_pkt); +} + +static void av1_frame_merge_close(AVBSFContext *bsf) +{ + AV1FMergeContext *ctx = bsf->priv_data; + + ff_cbs_fragment_free(ctx->cbc, &ctx->temporal_unit); + ff_cbs_fragment_free(ctx->cbc, &ctx->frag); + av_packet_free(&ctx->buffer_pkt); + ff_cbs_close(&ctx->cbc); +} + +static const enum AVCodecID av1_frame_merge_codec_ids[] = { + AV_CODEC_ID_AV1, AV_CODEC_ID_NONE, +}; + +const AVBitStreamFilter ff_av1_frame_merge_bsf = { + .name = "av1_frame_merge", + .priv_data_size = sizeof(AV1FMergeContext), + .init = av1_frame_merge_init, + .flush = av1_frame_merge_flush, + .close = av1_frame_merge_close, + .filter = av1_frame_merge_filter, + .codec_ids = av1_frame_merge_codec_ids, +}; diff --git a/libavcodec/bitstream_filters.c b/libavcodec/bitstream_filters.c index 463003966a..6b5ffe4d70 100644 --- a/libavcodec/bitstream_filters.c +++ b/libavcodec/bitstream_filters.c @@ -25,6 +25,7 @@ #include "bsf.h" extern const AVBitStreamFilter ff_aac_adtstoasc_bsf; +extern const AVBitStreamFilter ff_av1_frame_merge_bsf; extern const AVBitStreamFilter ff_av1_frame_split_bsf; extern const AVBitStreamFilter ff_av1_metadata_bsf; extern const AVBitStreamFilter ff_chomp_bsf;
This BSF takes Temporal Units split across different AVPackets and merges them by looking for Temporal Delimiter OBUs. Signed-off-by: James Almer <jamrial@gmail.com> --- Now copying packet props. configure | 1 + libavcodec/Makefile | 1 + libavcodec/av1_frame_merge_bsf.c | 172 +++++++++++++++++++++++++++++++ libavcodec/bitstream_filters.c | 1 + 4 files changed, 175 insertions(+) create mode 100644 libavcodec/av1_frame_merge_bsf.c