From patchwork Wed Jul 10 21:08:26 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 13890 Return-Path: X-Original-To: patchwork@ffaux-bg.ffmpeg.org Delivered-To: patchwork@ffaux-bg.ffmpeg.org Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by ffaux.localdomain (Postfix) with ESMTP id 7EF29448E81 for ; Thu, 11 Jul 2019 00:16:38 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 5B09B68AAB1; Thu, 11 Jul 2019 00:16:38 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wr1-f65.google.com (mail-wr1-f65.google.com [209.85.221.65]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id E94C068A7D9 for ; Thu, 11 Jul 2019 00:16:31 +0300 (EEST) Received: by mail-wr1-f65.google.com with SMTP id 31so3963910wrm.1 for ; Wed, 10 Jul 2019 14:16:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=Bd834+1ECAAtxIBou8itb1NomsSPxaFyHRqwGQZYls4=; b=ZLP+osC3G4La5on3zCHBucupSBISNzlUQxSBt9S7r0bxmafkWHg5AF4UQ6sTGFg5Y9 uFHSVx1qqHNkixVUCfPLMu3vcEQoICen2BqOF2KihBBdhEwU2U/NCLNlbnOw3Q4ZtUsS ejQWB99ymahg9gSlqcyuRenVyctd/IECo+hrQQg44bEOwMhXB4FXhMs9HV0xJoNbKWYs DoONu76v81N0IR1u1KknY/yzQ+VGRmYRPgrGB/oHVKPvBDqjkEcCsAklEIA8JA4cvSld TknZ+jq6RfMb/lcL1U0Hrb6xnPAAsbIYV9D14VIE4FTkUg+ZFSdo+I+o8O9loNYupXh1 d4/g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=Bd834+1ECAAtxIBou8itb1NomsSPxaFyHRqwGQZYls4=; b=bPQNlo8H/nda9+ExbF/Ffrh78Fy0jGDAkqtnR1KJXCWRcb8ym6jOJeIVBhq9xmcJOV JNPxgDci5BNyM+ytOz8x4JzHgcXvwWyJKaRyLinUAsti2VulcrBhYDi+NCkbQnMreU8U qRRBUYD28/e9nHce8feuGjQhJGzM5vKcIHN19dm98fqP3qujbn0ra/no1FZj57eyS9rG QvNJjvhznhxhE5B7n0Q+3J457hQBCp6IoL9zXiYrHxvfRubTgKdlqh8u+sMV+XpXhRvk Ihlm4ZHG7oSn52R6fyaOtg1Wr539PeOKi5nrx37nvJ0UI6DFHHOBlFD1R1M2vZuMJT6A USuw== X-Gm-Message-State: APjAAAVQcu+tf+RkPj6U+TIM1dTrOOSZT5Fm2qVeKOXzS+9BwqX5JvTs pA7aA796HIyaU27crv+EPIdwu9XQ X-Google-Smtp-Source: APXvYqycCWj4sP4qyp5+GisWJptN5EKFDXN2GRlQYBmQR8tgrunLaVJLPUtwFnzUFm/UwNQPHt+wUw== X-Received: by 2002:a5d:468a:: with SMTP id u10mr33503176wrq.177.1562792928811; Wed, 10 Jul 2019 14:08:48 -0700 (PDT) Received: from localhost.localdomain (ipbcc08b8f.dynamic.kabel-deutschland.de. [188.192.139.143]) by smtp.gmail.com with ESMTPSA id z19sm2209058wmi.7.2019.07.10.14.08.47 (version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256); Wed, 10 Jul 2019 14:08:48 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Wed, 10 Jul 2019 23:08:26 +0200 Message-Id: <20190710210827.20409-1-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.21.0 MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH 1/2] mpeg4_unpack_bframes: Avoid allocations and copies of packet structures X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Cc: Andreas Rheinhardt Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" 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 --- 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[] = {