Andreas Rheinhardt:
> 1. Since bd90a2ec, mpeg4_unpack_bframes caches whole packets instead of
> just the pointer to the buffer and the buffer's size in order to be able
> to make use of refcounting to avoid copying of data; this unfortunately
> introduced copies of packet structures and side data (if existing),
> although the only fields that are needed are the buffer-related ones
> (data, size and buf). This can be changed without compromising the
> advantages of refcounting by storing a reference to the buffer.
>
> 2. This change also makes it easy to use only one packet throughout
> so that an allocation and free of an AVPacket structure per filtered
> packet can be saved by switching to ff_bsf_get_packet_ref.
>
> 3. Furthermore, this commit also fixes a memleak introduced in bd90a2ec:
> If a stored b_frame with side data was used for a later frame, the side
> data would leak when the input frame's properties were copied into the
> output frame.
>
> Signed-off-by: Andreas Rheinhardt <andreas.rheinha...@gmail.com>
> ---
> libavcodec/mpeg4_unpack_bframes_bsf.c | 77 ++++++++++++---------------
> 1 file changed, 35 insertions(+), 42 deletions(-)
>
> diff --git a/libavcodec/mpeg4_unpack_bframes_bsf.c
> b/libavcodec/mpeg4_unpack_bframes_bsf.c
> index 1daf133ce5..382070b423 100644
> --- a/libavcodec/mpeg4_unpack_bframes_bsf.c
> +++ b/libavcodec/mpeg4_unpack_bframes_bsf.c
> @@ -25,7 +25,7 @@
> #include "mpeg4video.h"
>
> typedef struct UnpackBFramesBSFContext {
> - AVPacket *b_frame;
> + AVBufferRef *b_frame_ref;
> } UnpackBFramesBSFContext;
>
> /* determine the position of the packed marker in the userdata,
> @@ -56,32 +56,32 @@ static void scan_buffer(const uint8_t *buf, int buf_size,
> }
> }
>
> -static int mpeg4_unpack_bframes_filter(AVBSFContext *ctx, AVPacket *out)
> +static int mpeg4_unpack_bframes_filter(AVBSFContext *ctx, AVPacket *pkt)
> {
> UnpackBFramesBSFContext *s = ctx->priv_data;
> int pos_p = -1, nb_vop = 0, pos_vop2 = -1, ret = 0;
> - AVPacket *in;
>
> - ret = ff_bsf_get_packet(ctx, &in);
> + ret = ff_bsf_get_packet_ref(ctx, pkt);
> if (ret < 0)
> return ret;
>
> - scan_buffer(in->data, in->size, &pos_p, &nb_vop, &pos_vop2);
> + scan_buffer(pkt->data, pkt->size, &pos_p, &nb_vop, &pos_vop2);
> av_log(ctx, AV_LOG_DEBUG, "Found %d VOP startcode(s) in this packet.\n",
> nb_vop);
>
> if (pos_vop2 >= 0) {
> - if (s->b_frame->data) {
> + if (s->b_frame_ref) {
> av_log(ctx, AV_LOG_WARNING,
> "Missing one N-VOP packet, discarding one B-frame.\n");
> - av_packet_unref(s->b_frame);
> + av_buffer_unref(&s->b_frame_ref);
> }
> - /* store the packed B-frame in the BSFContext */
> - ret = av_packet_ref(s->b_frame, in);
> - if (ret < 0) {
> + /* store a reference to the packed B-frame's data in the BSFContext
> */
> + s->b_frame_ref = av_buffer_ref(pkt->buf);
> + if (!s->b_frame_ref) {
> + ret = AVERROR(ENOMEM);
> goto fail;
> }
> - s->b_frame->size -= pos_vop2;
> - s->b_frame->data += pos_vop2;
> + s->b_frame_ref->data = pkt->data + pos_vop2;
> + s->b_frame_ref->size = pkt->size - pos_vop2;
> }
>
> if (nb_vop > 2) {
> @@ -89,56 +89,49 @@ static int mpeg4_unpack_bframes_filter(AVBSFContext *ctx,
> AVPacket *out)
> "Found %d VOP headers in one packet, only unpacking one.\n", nb_vop);
> }
>
> - if (nb_vop == 1 && s->b_frame->data) {
> - /* use frame from BSFContext */
> - av_packet_move_ref(out, s->b_frame);
> + if (nb_vop == 1 && s->b_frame_ref) {
> + AVBufferRef *tmp = pkt->buf;
>
> - /* use properties from current input packet */
> - ret = av_packet_copy_props(out, in);
> - if (ret < 0) {
> - goto fail;
> - }
> + /* make tmp accurately reflect the packet's data */
> + tmp->data = pkt->data;
> + tmp->size = pkt->size;
> +
> + /* replace data in packet with stored data */
> + pkt->buf = s->b_frame_ref;
> + pkt->data = s->b_frame_ref->data;
> + pkt->size = s->b_frame_ref->size;
> +
> + /* store reference to data into BSFContext */
> + s->b_frame_ref = tmp;
>
> - if (in->size <= MAX_NVOP_SIZE) {
> - /* N-VOP */
> + if (s->b_frame_ref->size <= MAX_NVOP_SIZE) {
> + /* N-VOP - discard stored data */
> av_log(ctx, AV_LOG_DEBUG, "Skipping N-VOP.\n");
> - } else {
> - /* copy packet into BSFContext */
> - av_packet_move_ref(s->b_frame, in);
> + av_buffer_unref(&s->b_frame_ref);
> }
> } else if (nb_vop >= 2) {
> /* use first frame of the packet */
> - av_packet_move_ref(out, in);
> - out->size = pos_vop2;
> + pkt->size = pos_vop2;
> } else if (pos_p >= 0) {
> - ret = av_packet_make_writable(in);
> + ret = av_packet_make_writable(pkt);
> if (ret < 0)
> goto fail;
> av_log(ctx, AV_LOG_DEBUG, "Updating DivX userdata (remove trailing
> 'p').\n");
> - av_packet_move_ref(out, in);
> /* remove 'p' (packed) from the end of the (DivX) userdata string */
> - out->data[pos_p] = '\0';
> + pkt->data[pos_p] = '\0';
> } else {
> - /* copy packet */
> - av_packet_move_ref(out, in);
> + /* use packet as is */
> }
>
> fail:
> if (ret < 0)
> - av_packet_unref(out);
> - av_packet_free(&in);
> + av_packet_unref(pkt);
>
> return ret;
> }
>
> static int mpeg4_unpack_bframes_init(AVBSFContext *ctx)
> {
> - UnpackBFramesBSFContext *s = ctx->priv_data;
> -
> - s->b_frame = av_packet_alloc();
> - if (!s->b_frame)
> - return AVERROR(ENOMEM);
> -
> if (ctx->par_in->extradata) {
> int pos_p_ext = -1;
> scan_buffer(ctx->par_in->extradata, ctx->par_in->extradata_size,
> &pos_p_ext, NULL, NULL);
> @@ -155,13 +148,13 @@ static int mpeg4_unpack_bframes_init(AVBSFContext *ctx)
> static void mpeg4_unpack_bframes_flush(AVBSFContext *bsfc)
> {
> UnpackBFramesBSFContext *ctx = bsfc->priv_data;
> - av_packet_unref(ctx->b_frame);
> + av_buffer_unref(&ctx->b_frame_ref);
> }
>
> static void mpeg4_unpack_bframes_close(AVBSFContext *bsfc)
> {
> UnpackBFramesBSFContext *ctx = bsfc->priv_data;
> - av_packet_free(&ctx->b_frame);
> + av_buffer_unref(&ctx->b_frame_ref);
> }
>
> static const enum AVCodecID codec_ids[] = {
>
Ping.
- Andreas
_______________________________________________
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".
