From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id 6DC0144099 for ; Thu, 25 Aug 2022 16:29:24 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id BB20A68B926; Thu, 25 Aug 2022 19:29:20 +0300 (EEST) Received: from EUR02-AM5-obe.outbound.protection.outlook.com (mail-oln040092067086.outbound.protection.outlook.com [40.92.67.86]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id E018968B53E for ; Thu, 25 Aug 2022 19:29:14 +0300 (EEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=VxaAe3BhuRHHtggWxwJLAQAjTKW1Hcu+s3zKwh1elUJO8/yOW4Zs8plwQLwh5Aa8aw1Ru44p/SWVq3kjRqbLFakzYjeJIrJekRNhHD3g9id9iTdsK3xhkrpwh/rMUqUnKNCC58i2fYpl4rxWSmnjgxzBW01kZsPBb5jeaAoOgT76yIkhc4Gh1GS+ZuWbIruwCmg7qZ/77I731RVxEBQCbFuKyoF8icf5vSrmVTxfHGofgOfLX4IW+sK94WiSEFwE6FEUXM8v3y5zSMaVut33zFfVKxd1m5LaoWKJpGB8meY5lvYPgMSGLtJpfsvbA9n/prhHC9WPJSip7YwBSIYV2A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=oiNb94QS4/JELBrEfanM92LMD1YzlpDdeWItKrY0oDY=; b=kD5kdlbMVdZ7OTPn9x9N8jGaVY4Qf7PT1pr6IK+AtbuXUGr4+iFTV9to9Tgh1X+LsPui+0GUFkX/D1fDV76KHYMiKAvKm8iDgWhCLZZMZA2xDlTRpu8EjrUiwl2MLwS3GH8Fv+MJVCWRbS6NSgf0S3UMidwTRFFHdOIvNDtVmkqnvxrGQ4kjpm9ribKgfDsY82MxJtHqnpp4sPAQrDvH0tnzGylEYHFHgH+Nyu4eYqPD70jQhDLstr6NDeBwa/atEYKhcxLxZsiUZygK2zISOBgMkUiibejIpT3pXUIP5SLL1D3uwrwd12aODyx/CMs2mH1iSpAhsdm2uTBWlJZ6Tg== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=none; dmarc=none; dkim=none; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=outlook.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=oiNb94QS4/JELBrEfanM92LMD1YzlpDdeWItKrY0oDY=; b=IxCBmGCjFKnSH43jGYXs9xoAFq10EphWWkErK0cOCUYb88o936gwC6bTEkeALWvVPabrN+CJzBcC7RtFDOFaDIudh8GrgDKjBn78SVqT0YHDoOw7BkxVQnLStj5PyLkz+G27KDoHHHFbInjeg01FTQvy4K+qBJjeHFlfv+LaAid02kKcW5OcJyONqtn7nMlf+Ut3vxtwE+8Rv1opvbHJscjnvv0Jlbas0+lrosDgGc05R3tqRPPPOaEirSTm/LDlEd3Vqb3NLmv/76d+OvjA/VvsF1RlfdB+906oEb62V0hdSrzfugA/Y3J5sE1EMTMnjBI74lR4aGhbLSPbkABm5Q== Received: from DB6PR0101MB2214.eurprd01.prod.exchangelabs.com (2603:10a6:4:42::27) by PAXPR01MB10263.eurprd01.prod.exchangelabs.com (2603:10a6:102:23c::18) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5566.15; Thu, 25 Aug 2022 16:29:13 +0000 Received: from DB6PR0101MB2214.eurprd01.prod.exchangelabs.com ([fe80::210e:b627:bcc9:8c46]) by DB6PR0101MB2214.eurprd01.prod.exchangelabs.com ([fe80::210e:b627:bcc9:8c46%11]) with mapi id 15.20.5546.022; Thu, 25 Aug 2022 16:29:13 +0000 Message-ID: Date: Thu, 25 Aug 2022 18:29:11 +0200 Content-Language: en-US To: ffmpeg-devel@ffmpeg.org References: <20220824225209.4076-1-jamrial@gmail.com> From: Andreas Rheinhardt In-Reply-To: <20220824225209.4076-1-jamrial@gmail.com> X-TMN: [V22kPGH3kMlJS6b131J95SKIvGYXKfqF] X-ClientProxiedBy: ZR0P278CA0039.CHEP278.PROD.OUTLOOK.COM (2603:10a6:910:1d::8) To DB6PR0101MB2214.eurprd01.prod.exchangelabs.com (2603:10a6:4:42::27) X-Microsoft-Original-Message-ID: <4834cc3f-5862-d01b-ac22-cccc9d180dbd@outlook.com> MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: e31be053-fcb1-40f2-4a1a-08da86b6f188 X-MS-TrafficTypeDiagnostic: PAXPR01MB10263:EE_ X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: 8w/JzLx4Ar1/LkpONAoxbw9yZiaexMbM0pXzNZzzx6pjkm7wk9sTudcd63mM4G6LmNNKAQ9Dg53Q8yDcluo8/y7W2Jey+H2tX2wmY4lHX3AUla+e/6yDT8petgh+/KmNrX21X5CKEm67IIy3uXGrZCI2hJYwHJflmxw55ig82czCsh9Mh2C/fFqrypoOrvv2pastSdJKRQu34VeY9nUSsMGqQiloLBa+XRNf0DrVMGGGJmF2RA3dA6NQPPB0pEMA83L6TvK1385av/pwL8MnmA4LWp5ZI5rBJFeN871ABLd7emPEDZVpJ7SQgE1ciBGOkfxPhIC5/ehj3YmtKzjRBBrUEwSHFGHg9ETSybYzL9jY+uzZy2cIDIQsBJt8ocVsIg18xwdEU6H80bChPlaqa0XO4Pfe+peItbQkjQysp4abURK+qF7+1CYkEgpvqarwTkRGN6LxpB02+tg4wGdcqbAQkbWgxDLuYLF51XRI6kzALiAswwj4/DT1gMGLMV+eqPT4LgkpJgJef+b3rq8xU1+wX+pSSHnkAWqq0pLDqnQp5o2k2zzZo6Au+MNpb2EJTo0EgKnRJ29y1CkGUWGWC7KDuXnRx6d/L4x02zuh03JLHH75c7VPrMh9r78xNJeIY4T4me+tLc2R6/LFToz8GxBp9NTZF4hSRsv8eyCpk89gEfjMBhW8OShTztSq00ww X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?M0FkRUlOdElqaGtJY2lhOENwUVZMK2YveXZ0VE56a0xSQkY5ZFhSUDNYUS9F?= =?utf-8?B?enEvREN1Y25hQWRmUjRLUWJ0Ukk5MVd1bVhYM1EyQzd4TnNPamxRTVI5d3RC?= =?utf-8?B?V0twWWk3Ui9qRTdUYnpsZ3Jha3ExUzQyWFVRZkVzRnY0OHJ2WHloclYzeEFZ?= =?utf-8?B?c2hzdnV0STNXTFNBZ1FyRk9LL2UySXRHVW9OTHRjL3BIVnVuRnZLNEJQYitj?= =?utf-8?B?QmpNWWhxQkVweG43QWRVcSsxSXNiUHJKL3R0M3ZoeVB0OHBybUprQW5nTUl4?= =?utf-8?B?RlYwNzhYMklxWXB6RndkUXlWL3JwcGlMbkF4R2R5RHhSREtqS25SazkvN1VH?= =?utf-8?B?SDFyY2ZQSkVxMnlVTzc5TjZXcmdDOFE4ZXFxOXlHVGd1Qlc4Q21ySFE1L0lZ?= =?utf-8?B?ODkwczlLb3RMVW9zRURXdCt5ZUtvYkRtMkFmT08xNCtma3NlaERBS0hUUGRx?= =?utf-8?B?NkVzbWRzcmpWcHBuendoQ0tHeG1SdjM4djdyeUhEYTVHSDc5ajdSMGJkdkd0?= =?utf-8?B?OEU0YXJhdFFoSG54VGZQQTN4WnRZcDNmNVE1US9yM25HeW9RbFhENnFKUXU0?= =?utf-8?B?VlE0d3UyWE5Zc2hVclZta0FRVkZWUkh0eWxIMnNSL0s5UnlrMlBPZzhoWjRK?= =?utf-8?B?amxJRHJkdWljWm9UNVVXZmRRWWlzK212bTZsQ3QvWTNIaDNOQUpjdDJpTlYw?= =?utf-8?B?bmhlUEhpSGtvVXY2dmpnNlpaNVBseExFOUQ2ckJha2xERkhpSTZwZ1hVUW9G?= =?utf-8?B?YjdwY0hVeEVDd2RwcUIxaHJYSGlkR2lBL1ZFRGxTVEtKdFRzYWFkTE9HdFV4?= =?utf-8?B?UDJISER0VWN2d3JLdWxQUldETk0yL2RmbzdhL05Gbm9rS3VOYmFxd1lyZFVn?= =?utf-8?B?Q2JxdGFBNmRwZTVjeGlMRlpOZTMzcUw0cUttMmViU2w1cjNacytabjdBOXBr?= =?utf-8?B?cnVPZWhHU1ZKNm1HaEpoVDhBVlpJZUdyWTR3Yzhlek5uWGRpbFFjU3FxeE4y?= =?utf-8?B?VUE0eWtZV3p3NTJuKzAyZk9Oa1lEWmVxbzYyVEJscXNwUU1JekdUdUxjbjVs?= =?utf-8?B?cVk1Z01lNHJiUmFMMzd2Qk5hUGZyeTNmVlBDbHlWbHkzcDZEUVFPUTZ1eGhl?= =?utf-8?B?N0xSMzZhbHpvdU9SaW9TMFQ0aUVOM1RJN0IwQTAzNzVTR2tydjU3c0FpbzM3?= =?utf-8?B?Tlc5ZklnSFlmcU5ud2ZMU0twSmJ0OEhJRW53dGNGd29HcEcvNVk3Sk42YWpZ?= =?utf-8?B?WDlaV2tQNkxuWmsvNkt3TFJvc3BZNmo0YWpXY0RBdlJOT2RBR080MW5KekxS?= =?utf-8?B?Rjc0TVluanRaUnBrVEdXbjM2QWZIeWR3dE1tMHFPcms2cHR5a2Z2ZDNCV1lC?= =?utf-8?B?S1FXTUlXOHlFVS9XbHd3SG11WDRqeVpXT2U5cFB1UEV4ajlnNG9QbXBLbHd3?= =?utf-8?B?YTQrQmE3VVR3WHRIa2xuYmU0citGZG9LNEZqREdzUTlGb0RZeGZZMWhBcmhM?= =?utf-8?B?MTV2OFhibXpRMUxSY3BhaE1FYWxiZ0kxV043S1lSMWgzbnZxbU1xTDZqVlUr?= =?utf-8?B?MFlOUHh4L2lCWlpoM0ZqVlhyd0ttek5WYUQzYm4vaVJlSjZxalgxaHZwK1VS?= =?utf-8?B?dEc0SUFQalJJTDRQTUtxR2Fjd3ZJdWxjekUzWkl3SVpZNjBLNnJ5RmtYZ2Rz?= =?utf-8?B?VVlvTXBHYmdaWFhDdEVTOWpTS2N3eWhhdXBnRUh0bEU4cE9VK0NrOWxZQksr?= =?utf-8?Q?UTOZpsgUUEGh1iO/g4=3D?= X-OriginatorOrg: outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: e31be053-fcb1-40f2-4a1a-08da86b6f188 X-MS-Exchange-CrossTenant-AuthSource: DB6PR0101MB2214.eurprd01.prod.exchangelabs.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 25 Aug 2022 16:29:12.9975 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa X-MS-Exchange-CrossTenant-RMS-PersistedConsumerOrg: 00000000-0000-0000-0000-000000000000 X-MS-Exchange-Transport-CrossTenantHeadersStamped: PAXPR01MB10263 Subject: Re: [FFmpeg-devel] [PATCH] avcodec/libaomenc: remove one memcpy when queueing packets X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 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 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: James Almer: > Don't use an intermediary buffer. Achieve this by replacing FrameListData with > a PacketList, and by allocating and populating every packet's payload before > inserting them into the list. > > Signed-off-by: James Almer > --- > libavcodec/libaomenc.c | 195 +++++++++++++++-------------------------- > 1 file changed, 70 insertions(+), 125 deletions(-) > > diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c > index 485f554165..f9476b3ddf 100644 > --- a/libavcodec/libaomenc.c > +++ b/libavcodec/libaomenc.c > @@ -38,6 +38,7 @@ > > #include "av1.h" > #include "avcodec.h" > +#include "bytestream.h" > #include "bsf.h" > #include "codec_internal.h" > #include "encode.h" > @@ -46,24 +47,6 @@ > #include "packet_internal.h" > #include "profiles.h" > > -/* > - * Portion of struct aom_codec_cx_pkt from aom_encoder.h. > - * One encoded frame returned from the library. > - */ > -struct FrameListData { > - void *buf; /**< compressed data buffer */ > - size_t sz; /**< length of compressed data */ > - int64_t pts; /**< time stamp to show frame > - (in timebase units) */ > - unsigned long duration; /**< duration to show frame > - (in timebase units) */ > - uint32_t flags; /**< flags for this frame */ > - uint64_t sse[4]; > - int have_sse; /**< true if we have pending sse[] */ > - uint64_t frame_number; > - struct FrameListData *next; > -}; > - > typedef struct AOMEncoderContext { > AVClass *class; > AVBSFContext *bsf; > @@ -71,7 +54,8 @@ typedef struct AOMEncoderContext { > struct aom_image rawimg; > struct aom_fixed_buf twopass_stats; > unsigned twopass_stats_size; > - struct FrameListData *coded_frame_list; > + PacketList coded_frame_list; > + AVPacket *pkt; Renaming this variable to avpkt would improve clarity by simplifying distinguishing it from the aom_codec_cx_pkt packets. > int cpu_used; > int auto_alt_ref; > int arnr_max_frames; > @@ -283,33 +267,6 @@ static av_cold void dump_enc_cfg(AVCodecContext *avctx, > av_log(avctx, level, "\n"); > } > > -static void coded_frame_add(void *list, struct FrameListData *cx_frame) > -{ > - struct FrameListData **p = list; > - > - while (*p) > - p = &(*p)->next; > - *p = cx_frame; > - cx_frame->next = NULL; > -} > - > -static av_cold void free_coded_frame(struct FrameListData *cx_frame) > -{ > - av_freep(&cx_frame->buf); > - av_freep(&cx_frame); > -} > - > -static av_cold void free_frame_list(struct FrameListData *list) > -{ > - struct FrameListData *p = list; > - > - while (p) { > - list = list->next; > - free_coded_frame(p); > - p = list; > - } > -} > - > static av_cold int codecctl_int(AVCodecContext *avctx, > #ifdef UENUM1BYTE > aome_enc_control_id id, > @@ -432,7 +389,8 @@ static av_cold int aom_free(AVCodecContext *avctx) > aom_codec_destroy(&ctx->encoder); > av_freep(&ctx->twopass_stats.buf); > av_freep(&avctx->stats_out); > - free_frame_list(ctx->coded_frame_list); > + avpriv_packet_list_free(&ctx->coded_frame_list); > + av_packet_free(&ctx->pkt); > av_bsf_free(&ctx->bsf); > return 0; > } > @@ -1042,6 +1000,10 @@ static av_cold int aom_init(AVCodecContext *avctx, > return ret; > } > > + ctx->pkt = av_packet_alloc(); > + if (!ctx->pkt) > + return AVERROR(ENOMEM); > + This encoder does not have the INIT_CLEANUP flag set, so everything leaks in case the above allocation fails. In fact, it seems like there are already leaks in several errors paths in this function. > if (enccfg.rc_end_usage == AOM_CBR || > enccfg.g_pass != AOM_RC_ONE_PASS) { > cpb_props->max_bitrate = avctx->rc_max_rate; > @@ -1053,25 +1015,40 @@ static av_cold int aom_init(AVCodecContext *avctx, > return 0; > } > > -static inline void cx_pktcpy(AOMContext *ctx, > - struct FrameListData *dst, > +static inline int cx_pktcpy(AVCodecContext *avctx, We should not override the compiler's inlining behaviour unless we have a good reason to do so, so you could remove it while at it. > + AVPacket *dst, Wrong indentation. > const struct aom_codec_cx_pkt *src) > { > - dst->pts = src->data.frame.pts; > - dst->duration = src->data.frame.duration; > - dst->flags = src->data.frame.flags; > - dst->sz = src->data.frame.sz; > - dst->buf = src->data.frame.buf; > + AOMContext *ctx = avctx->priv_data; > + int av_unused pict_type; > + int ret; > + > + av_packet_unref(dst); Can dst ever be non-blank here (i.e. before the unref)? > + ret = ff_get_encode_buffer(avctx, dst, src->data.frame.sz, 0); > + if (ret < 0) { > + av_log(avctx, AV_LOG_ERROR, > + "Error getting output packet of size %"SIZE_SPECIFIER".\n", src->data.frame.sz); > + return ret; > + } > + memcpy(dst->data, src->data.frame.buf, src->data.frame.sz); > + dst->pts = dst->dts = src->data.frame.pts; > + > + if (src->data.frame.flags & AOM_FRAME_IS_KEY) { > + dst->flags |= AV_PKT_FLAG_KEY; > #ifdef AOM_FRAME_IS_INTRAONLY > - dst->frame_number = ++ctx->frame_number; > - dst->have_sse = ctx->have_sse; > + pict_type = AV_PICTURE_TYPE_I; > + } else if (src->data.frame.flags & AOM_FRAME_IS_INTRAONLY) { > + pict_type = AV_PICTURE_TYPE_I; > + } else { > + pict_type = AV_PICTURE_TYPE_P; > + } > + > if (ctx->have_sse) { > - /* associate last-seen SSE to the frame. */ > - /* Transfers ownership from ctx to dst. */ > - memcpy(dst->sse, ctx->sse, sizeof(dst->sse)); > + ff_side_data_set_encoder_stats(dst, 0, ctx->sse + 1, 3, pict_type); This function can fail. > ctx->have_sse = 0; > - } > #endif > + } > + return 0; > } > > /** > @@ -1081,50 +1058,32 @@ static inline void cx_pktcpy(AOMContext *ctx, > * @return packet data size on success > * @return a negative AVERROR on error > */ > -static int storeframe(AVCodecContext *avctx, struct FrameListData *cx_frame, > - AVPacket *pkt) > +static int storeframe(AVCodecContext *avctx, AVPacket *dst, AVPacket *src) > { > AOMContext *ctx = avctx->priv_data; > - int av_unused pict_type; > - int ret = ff_get_encode_buffer(avctx, pkt, cx_frame->sz, 0); > - if (ret < 0) { > - av_log(avctx, AV_LOG_ERROR, > - "Error getting output packet of size %"SIZE_SPECIFIER".\n", cx_frame->sz); > - return ret; > - } > - memcpy(pkt->data, cx_frame->buf, pkt->size); > - pkt->pts = pkt->dts = cx_frame->pts; > + const uint8_t *sd; > + size_t size; > + int ret; > > - if (!!(cx_frame->flags & AOM_FRAME_IS_KEY)) { > - pkt->flags |= AV_PKT_FLAG_KEY; > -#ifdef AOM_FRAME_IS_INTRAONLY > - pict_type = AV_PICTURE_TYPE_I; > - } else if (cx_frame->flags & AOM_FRAME_IS_INTRAONLY) { > - pict_type = AV_PICTURE_TYPE_I; > - } else { > - pict_type = AV_PICTURE_TYPE_P; > - } > - > - ff_side_data_set_encoder_stats(pkt, 0, cx_frame->sse + 1, > - cx_frame->have_sse ? 3 : 0, pict_type); > + av_packet_move_ref(dst, src); > > - if (cx_frame->have_sse) { > + sd = av_packet_get_side_data(dst, AV_PKT_DATA_QUALITY_STATS, &size); > + if (sd && size >= 4 + 4 + 8 * 3) { > int i; > + sd += 4 + 4; > for (i = 0; i < 3; ++i) { > - avctx->error[i] += cx_frame->sse[i + 1]; > + avctx->error[i] += bytestream_get_le64(&sd); > } > - cx_frame->have_sse = 0; > -#endif > } > > if (avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER) { > - ret = av_bsf_send_packet(ctx->bsf, pkt); > + ret = av_bsf_send_packet(ctx->bsf, dst); > if (ret < 0) { > av_log(avctx, AV_LOG_ERROR, "extract_extradata filter " > "failed to send input packet\n"); > return ret; > } > - ret = av_bsf_receive_packet(ctx->bsf, pkt); > + ret = av_bsf_receive_packet(ctx->bsf, dst); > > if (ret < 0) { > av_log(avctx, AV_LOG_ERROR, "extract_extradata filter " > @@ -1132,7 +1091,7 @@ static int storeframe(AVCodecContext *avctx, struct FrameListData *cx_frame, > return ret; > } > } > - return pkt->size; > + return dst->size; > } > > /** > @@ -1148,16 +1107,14 @@ static int queue_frames(AVCodecContext *avctx, AVPacket *pkt_out) > AOMContext *ctx = avctx->priv_data; > const struct aom_codec_cx_pkt *pkt; > const void *iter = NULL; > - int size = 0; > + int ret, size = 0; > > - if (ctx->coded_frame_list) { > - struct FrameListData *cx_frame = ctx->coded_frame_list; > + if (!avpriv_packet_list_get(&ctx->coded_frame_list, ctx->pkt)) { > /* return the leading frame if we've already begun queueing */ > - size = storeframe(avctx, cx_frame, pkt_out); > - if (size < 0) > - return size; > - ctx->coded_frame_list = cx_frame->next; > - free_coded_frame(cx_frame); > + ret = storeframe(avctx, pkt_out, ctx->pkt); > + if (ret < 0) > + goto fail; > + size = ret; > } > > /* consume all available output from the encoder before returning. buffers > @@ -1165,37 +1122,21 @@ static int queue_frames(AVCodecContext *avctx, AVPacket *pkt_out) > while ((pkt = aom_codec_get_cx_data(&ctx->encoder, &iter))) { > switch (pkt->kind) { > case AOM_CODEC_CX_FRAME_PKT: > + ret = cx_pktcpy(avctx, ctx->pkt, pkt); > + if (ret < 0) > + goto fail; > if (!size) { > - struct FrameListData cx_frame; > - > /* avoid storing the frame when the list is empty and we haven't yet > * provided a frame for output */ > - av_assert0(!ctx->coded_frame_list); > - cx_pktcpy(ctx, &cx_frame, pkt); > - size = storeframe(avctx, &cx_frame, pkt_out); > - if (size < 0) > - return size; > + av_assert0(!ctx->coded_frame_list.head); > + ret = storeframe(avctx, pkt_out, ctx->pkt); > + if (ret < 0) > + goto fail; > + size = ret; > } else { > - struct FrameListData *cx_frame = > - av_malloc(sizeof(struct FrameListData)); > - > - if (!cx_frame) { > - av_log(avctx, AV_LOG_ERROR, > - "Frame queue element alloc failed\n"); > - return AVERROR(ENOMEM); > - } > - cx_pktcpy(ctx, cx_frame, pkt); > - cx_frame->buf = av_malloc(cx_frame->sz); > - > - if (!cx_frame->buf) { > - av_log(avctx, AV_LOG_ERROR, > - "Data buffer alloc (%"SIZE_SPECIFIER" bytes) failed\n", > - cx_frame->sz); > - av_freep(&cx_frame); > - return AVERROR(ENOMEM); > - } > - memcpy(cx_frame->buf, pkt->data.frame.buf, pkt->data.frame.sz); I am shocked to see that there were two memcpies. > - coded_frame_add(&ctx->coded_frame_list, cx_frame); > + ret = avpriv_packet_list_put(&ctx->coded_frame_list, ctx->pkt, NULL, 0); > + if (ret < 0) > + goto fail; wtf: Any error that queue_frames() returns will be translated to "got_packet = 1" by the caller (with return code 0). Error handling in this encoder seems to be a joke. > } > break; > case AOM_CODEC_STATS_PKT: > @@ -1236,6 +1177,10 @@ static int queue_frames(AVCodecContext *avctx, AVPacket *pkt_out) > } > > return size; > +fail: > + av_packet_unref(ctx->pkt); > + av_packet_unref(pkt_out); > + return ret; > } > > static enum AVPixelFormat aomfmt_to_pixfmt(struct aom_image *img) _______________________________________________ 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".