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
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
