From patchwork Tue May 19 10:45:59 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 19773 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 8729C44B8F8 for ; Tue, 19 May 2020 14:15:45 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 59D50689257; Tue, 19 May 2020 14:15:45 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wm1-f65.google.com (mail-wm1-f65.google.com [209.85.128.65]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 7C42F680A4B for ; Tue, 19 May 2020 14:15:39 +0300 (EEST) Received: by mail-wm1-f65.google.com with SMTP id t8so498163wmi.0 for ; Tue, 19 May 2020 04:15:39 -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:in-reply-to:references :mime-version:content-transfer-encoding; bh=jIu187CAIZWeKrLC7f7bJBDeDn6BYiLCW3ajyUqfOmw=; b=OHIGSCUf071tM9Xnz6iNJOebRx9RSStLmrOQMRd6kTLrkHdOJR6YirhPvkFUuxno25 CyauK9MTkimUvXjYnnaNyjZFcojlrCBL6SmuSrbewxjHi/nAGQakm10N8clOpazbPSNR lIBifUR2mXRgH9GFHn4rvtOP1K59dHCI2ZLXwLmsvugfWbBfQvOzkTvixFagLlABn24U egjeuNOKUCQlhncsqpHtOmq3d3oIc/I7Ls1bBJZawAeSqjj4b6eSPc0B5oTFOeTBxmHo vA+anQiIFFcKaEQaKe0/iuplfWK8+FQoA11OP10RidJuTMA7f/I+vtBYp4TFp3o0Tmx8 QHzg== 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:in-reply-to :references:mime-version:content-transfer-encoding; bh=jIu187CAIZWeKrLC7f7bJBDeDn6BYiLCW3ajyUqfOmw=; b=VuIs+EF+w3MMQmUlgtuQReWnDklnG9iWLsNdc8AsHoSMUc7Kml7e4RiEL2Dv4nsRGI dapCeVPBFH4bQs0Km7fwgyykQp+PdPBx6WYliYExKyWNGjpY3zx6gvczOcIEoHTAlYcV fuCUdVcCjGML0dkDe77HwEMgj23A2DndKNSL/Fw0n8Z2ge0TPb24l1Bp2I0CeReHlhVs WCJ1Ta90s8abSwqhKizGOIxN6EyIiDavOt5Yv3wS0oukL4nz6pB7GJnVXyXsFGwpmhAF cZCH+6pxJddGgC16Jv3kzHsjGC6km3pIT8+JLukSW65g0o61zDfvKWzRUXWrCLr+8okl rGpg== X-Gm-Message-State: AOAM533Agegd4JygV8pZUsntbmpvjkCmCXXm/QLMA2+TPs2HWTMgZvEF HtjDWC/qqkXXJGsXKPMv4g2K3O4L X-Google-Smtp-Source: ABdhPJw41jCqqobN0V4UNe/S5NCT0Ew1CELaX1XpLkzWJ7uarXj3WhkeTxpZUiF6EAFyLf1xDixRHg== X-Received: by 2002:a1c:307:: with SMTP id 7mr4893477wmd.104.1589886596353; Tue, 19 May 2020 04:09:56 -0700 (PDT) Received: from sblaptop.fritz.box (ipbcc1ab57.dynamic.kabel-deutschland.de. [188.193.171.87]) by smtp.gmail.com with ESMTPSA id m23sm3511998wmg.45.2020.05.19.04.09.55 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 19 May 2020 04:09:55 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Tue, 19 May 2020 12:45:59 +0200 Message-Id: <20200519104601.12817-3-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200519104601.12817-1-andreas.rheinhardt@gmail.com> References: <20200519104601.12817-1-andreas.rheinhardt@gmail.com> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH 3/5] avformat/id3v2: Avoid allocations for ID3v2ExtraMeta 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" Up until now, the ID3v2ExtraMeta structure (which is used when parsing ID3v2 tags containing attached pictures, chapters etc.) contained a pointer to separately allocated data that depended on the type of the tag. Yet the difference of the sizes of the largest and the smallest of these structures is fairly small, so that it is better to simply include a union of all the possible types of tag-dependent structures in ID3v2ExtraMeta. This commit implements this. Signed-off-by: Andreas Rheinhardt --- If only one were allowed to use anonymous unions! libavformat/hls.c | 4 ++-- libavformat/id3v2.c | 50 ++++++++++++++++---------------------------- libavformat/id3v2.h | 17 +++++++++------ libavformat/omadec.c | 13 ++++++------ 4 files changed, 37 insertions(+), 47 deletions(-) diff --git a/libavformat/hls.c b/libavformat/hls.c index fc45719d1c..d9e09013e6 100644 --- a/libavformat/hls.c +++ b/libavformat/hls.c @@ -1004,7 +1004,7 @@ static void parse_id3(AVFormatContext *s, AVIOContext *pb, ff_id3v2_read_dict(pb, metadata, ID3v2_DEFAULT_MAGIC, extra_meta); for (meta = *extra_meta; meta; meta = meta->next) { if (!strcmp(meta->tag, "PRIV")) { - ID3v2ExtraMetaPRIV *priv = meta->data; + ID3v2ExtraMetaPRIV *priv = &meta->data.priv; if (priv->datasize == 8 && !strcmp(priv->owner, id3_priv_owner_ts)) { /* 33-bit MPEG timestamp */ int64_t ts = AV_RB64(priv->data); @@ -1015,7 +1015,7 @@ static void parse_id3(AVFormatContext *s, AVIOContext *pb, av_log(s, AV_LOG_ERROR, "Invalid HLS ID3 audio timestamp %"PRId64"\n", ts); } } else if (!strcmp(meta->tag, "APIC") && apic) - *apic = meta->data; + *apic = &meta->data.apic; } } diff --git a/libavformat/id3v2.c b/libavformat/id3v2.c index abe073dcc1..2ba5c3857d 100644 --- a/libavformat/id3v2.c +++ b/libavformat/id3v2.c @@ -225,7 +225,6 @@ static void free_geobtag(void *obj) av_freep(&geob->file_name); av_freep(&geob->description); av_freep(&geob->data); - av_free(geob); } /** @@ -459,20 +458,15 @@ static void read_geobtag(AVFormatContext *s, AVIOContext *pb, int taglen, if (taglen < 1) return; - geob_data = av_mallocz(sizeof(ID3v2ExtraMetaGEOB)); - if (!geob_data) { - av_log(s, AV_LOG_ERROR, "Failed to alloc %"SIZE_SPECIFIER" bytes\n", - sizeof(ID3v2ExtraMetaGEOB)); - return; - } - new_extra = av_mallocz(sizeof(ID3v2ExtraMeta)); if (!new_extra) { av_log(s, AV_LOG_ERROR, "Failed to alloc %"SIZE_SPECIFIER" bytes\n", sizeof(ID3v2ExtraMeta)); - goto fail; + return; } + geob_data = &new_extra->data.geob; + /* read encoding type byte */ encoding = avio_r8(pb); taglen--; @@ -511,7 +505,6 @@ static void read_geobtag(AVFormatContext *s, AVIOContext *pb, int taglen, /* add data to the list */ new_extra->tag = "GEOB"; - new_extra->data = geob_data; new_extra->next = *extra_meta; *extra_meta = new_extra; @@ -577,7 +570,6 @@ static void free_apic(void *obj) ID3v2ExtraMetaAPIC *apic = obj; av_buffer_unref(&apic->buf); av_freep(&apic->description); - av_freep(&apic); } static void rstrip_spaces(char *buf) @@ -603,10 +595,11 @@ static void read_apic(AVFormatContext *s, AVIOContext *pb, int taglen, goto fail; new_extra = av_mallocz(sizeof(*new_extra)); - apic = av_mallocz(sizeof(*apic)); - if (!new_extra || !apic) + if (!new_extra) goto fail; + apic = &new_extra->data.apic; + enc = avio_r8(pb); taglen--; @@ -658,7 +651,6 @@ static void read_apic(AVFormatContext *s, AVIOContext *pb, int taglen, memset(apic->buf->data + taglen, 0, AV_INPUT_BUFFER_PADDING_SIZE); new_extra->tag = "APIC"; - new_extra->data = apic; new_extra->next = *extra_meta; *extra_meta = new_extra; @@ -680,7 +672,6 @@ static void free_chapter(void *obj) ID3v2ExtraMetaCHAP *chap = obj; av_freep(&chap->element_id); av_dict_free(&chap->meta); - av_freep(&chap); } static void read_chapter(AVFormatContext *s, AVIOContext *pb, int len, const char *ttag, ID3v2ExtraMeta **extra_meta, int isv34) @@ -691,10 +682,10 @@ static void read_chapter(AVFormatContext *s, AVIOContext *pb, int len, const cha ID3v2ExtraMetaCHAP *chap = NULL; new_extra = av_mallocz(sizeof(*new_extra)); - chap = av_mallocz(sizeof(*chap)); + if (!new_extra) + return; - if (!new_extra || !chap) - goto fail; + chap = &new_extra->data.chap; if (decode_str(s, pb, 0, &chap->element_id, &len) < 0) goto fail; @@ -727,15 +718,13 @@ static void read_chapter(AVFormatContext *s, AVIOContext *pb, int len, const cha ff_metadata_conv(&chap->meta, NULL, ff_id3v2_4_metadata_conv); new_extra->tag = "CHAP"; - new_extra->data = chap; new_extra->next = *extra_meta; *extra_meta = new_extra; return; fail: - if (chap) - free_chapter(chap); + free_chapter(chap); av_freep(&new_extra); } @@ -744,7 +733,6 @@ static void free_priv(void *obj) ID3v2ExtraMetaPRIV *priv = obj; av_freep(&priv->owner); av_freep(&priv->data); - av_freep(&priv); } static void read_priv(AVFormatContext *s, AVIOContext *pb, int taglen, @@ -754,10 +742,10 @@ static void read_priv(AVFormatContext *s, AVIOContext *pb, int taglen, ID3v2ExtraMetaPRIV *priv; meta = av_mallocz(sizeof(*meta)); - priv = av_mallocz(sizeof(*priv)); + if (!meta) + return; - if (!meta || !priv) - goto fail; + priv = &meta->data.priv; if (decode_str(s, pb, ID3v2_ENCODING_ISO8859, &priv->owner, &taglen) < 0) goto fail; @@ -772,15 +760,13 @@ static void read_priv(AVFormatContext *s, AVIOContext *pb, int taglen, goto fail; meta->tag = "PRIV"; - meta->data = priv; meta->next = *extra_meta; *extra_meta = meta; return; fail: - if (priv) - free_priv(priv); + free_priv(priv); av_freep(&meta); } @@ -1132,7 +1118,7 @@ void ff_id3v2_free_extra_meta(ID3v2ExtraMeta **extra_meta) while (current) { if ((extra_func = get_extra_meta_func(current->tag, 1))) - extra_func->free(current->data); + extra_func->free(¤t->data); next = current->next; av_freep(¤t); current = next; @@ -1151,7 +1137,7 @@ int ff_id3v2_parse_apic(AVFormatContext *s, ID3v2ExtraMeta **extra_meta) if (strcmp(cur->tag, "APIC")) continue; - apic = cur->data; + apic = &cur->data.apic; if (!(st = avformat_new_stream(s, NULL))) return AVERROR(ENOMEM); @@ -1197,7 +1183,7 @@ int ff_id3v2_parse_chapters(AVFormatContext *s, ID3v2ExtraMeta **extra_meta) if (strcmp(cur->tag, "CHAP")) continue; - chap = cur->data; + chap = &cur->data.chap; if ((ret = av_dynarray_add_nofree(&chapters, &num_chapters, chap)) < 0) goto end; @@ -1239,7 +1225,7 @@ int ff_id3v2_parse_priv_dict(AVDictionary **metadata, ID3v2ExtraMeta **extra_met for (cur = *extra_meta; cur; cur = cur->next) { if (!strcmp(cur->tag, "PRIV")) { - ID3v2ExtraMetaPRIV *priv = cur->data; + ID3v2ExtraMetaPRIV *priv = &cur->data.priv; AVBPrint bprint; char *escaped, *key; int i, ret; diff --git a/libavformat/id3v2.h b/libavformat/id3v2.h index 9de0bee374..3e6030c942 100644 --- a/libavformat/id3v2.h +++ b/libavformat/id3v2.h @@ -54,12 +54,6 @@ typedef struct ID3v2EncContext { int len; ///< size of the tag written so far } ID3v2EncContext; -typedef struct ID3v2ExtraMeta { - const char *tag; - void *data; - struct ID3v2ExtraMeta *next; -} ID3v2ExtraMeta; - typedef struct ID3v2ExtraMetaGEOB { uint32_t datasize; uint8_t *mime_type; @@ -87,6 +81,17 @@ typedef struct ID3v2ExtraMetaCHAP { AVDictionary *meta; } ID3v2ExtraMetaCHAP; +typedef struct ID3v2ExtraMeta { + const char *tag; + struct ID3v2ExtraMeta *next; + union { + ID3v2ExtraMetaAPIC apic; + ID3v2ExtraMetaCHAP chap; + ID3v2ExtraMetaGEOB geob; + ID3v2ExtraMetaPRIV priv; + } data; +} ID3v2ExtraMeta; + /** * Detect ID3v2 Header. * @param buf must be ID3v2_HEADER_SIZE byte long diff --git a/libavformat/omadec.c b/libavformat/omadec.c index 9521b6d59e..79896bdf4f 100644 --- a/libavformat/omadec.c +++ b/libavformat/omadec.c @@ -217,14 +217,13 @@ static int decrypt_init(AVFormatContext *s, ID3v2ExtraMeta *em, uint8_t *header) av_log(s, AV_LOG_INFO, "File is encrypted\n"); /* find GEOB metadata */ - while (em) { - if (!strcmp(em->tag, "GEOB") && - (geob = em->data) && - (!strcmp(geob->description, "OMG_LSI") || - !strcmp(geob->description, "OMG_BKLSI"))) { + for (; em; em = em->next) { + if (strcmp(em->tag, "GEOB")) + continue; + geob = &em->data.geob; + if (!strcmp(geob->description, "OMG_LSI") || + !strcmp(geob->description, "OMG_BKLSI")) break; - } - em = em->next; } if (!em) { av_log(s, AV_LOG_ERROR, "No encryption header found\n");