Could someone apply this? On 10/16/19, Andreas Rheinhardt <[email protected]> wrote: > Andreas Rheinhardt: >> Andreas Rheinhardt: >>> 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 <[email protected]> >>>> --- >>>> 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 >>> >> Another ping. >> >> - Andreas >> > And another ping. > > - Andreas > _______________________________________________ > ffmpeg-devel mailing list > [email protected] > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > [email protected] with subject "unsubscribe". _______________________________________________ ffmpeg-devel mailing list [email protected] https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email [email protected] with subject "unsubscribe".
