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 336D444062 for ; Thu, 25 Aug 2022 00:23:18 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id DBEFF68B903; Thu, 25 Aug 2022 03:23:15 +0300 (EEST) Received: from mail-qv1-f42.google.com (mail-qv1-f42.google.com [209.85.219.42]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id F24CB68B8EC for ; Thu, 25 Aug 2022 03:23:09 +0300 (EEST) Received: by mail-qv1-f42.google.com with SMTP id q8so14168236qvr.9 for ; Wed, 24 Aug 2022 17:23:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=rcombs.me; s=google; h=message-id:in-reply-to:to:references:date:subject:mime-version:from :from:to:cc; bh=/qsW1f/Zp9sCawAYXYYpNOBiCrGyPT2UWqZgbu4vrbo=; b=UJV/yzS+appa0Mx5OP2thh0d19yLXUdNEeLKMYe2o1mGHEe9hlbOdD18zOFssy6gfs TMoMRFnLOY3nsU40YaDHnzYIDQUwcLf1Qno4vZFgPL4ZqU4VwGaGxJGaK1494yTQVpAQ dzdvYjQsnnqiw6YN1g1OtuXrC8hEzXC2A9jTTS4tTVGpKe2ah5K5YmWDjOnT3Exk4OSC l3BrW5poxnwNRutzkNRErXnXfU3p4SYpOZlg6WDN+UZEvxj1qM2wznUyYOG5pk4OgNjQ cuwHf2aE8pabJFVclFcjuE2INLIr9PDt/EFBMdPwgO0Udl8Y4teDjDqdIQvwri/X9fjY PczA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=message-id:in-reply-to:to:references:date:subject:mime-version:from :x-gm-message-state:from:to:cc; bh=/qsW1f/Zp9sCawAYXYYpNOBiCrGyPT2UWqZgbu4vrbo=; b=eZnkd8sD7WlL0R8M16tXuYeeylOqBW2mdMT0VRB//WfWtY3KRiVW12m5x5FdiZzsrf IONEH70mqPkYIQ6/+jfagfIld+9OYddSYJ9JqNPZrc76mK9OHvI9TmK832FdvFVALb2i MtWjuscoKG88rlHfwltkO9s+ZgJyb+QxBBGBtOhZCiuWiOJdr8c7DuwqvC02R494v2FL 41M3h03Mil11CRjmdTW6T9bRO+ybp/orSO1IC0QNBPvOuHFfTfshMOllB/o3AWNlkubO +gnnYPL6thtxxEJ5/0LFWBd1iNQfJ4ERffFQyf+4aaBdCrU9E8TJ4F+MmaIyZYQs6jie F4fg== X-Gm-Message-State: ACgBeo3OMewi4bUSjddpbm8VD7AIMaSkpzI5G/gekRzpDh1W5Y3Q581S GP4zbPB1E1Bvi9JMIyHvFzqN/sRRKAL6 X-Google-Smtp-Source: AA6agR7i+Ij+HMGzyU5yKhIVqVWDD/qNl1Qj6VBqCPScGwSwpb/sGiV0R41zPjyJoFXEPUDWYmfKmw== X-Received: by 2002:a05:6214:2606:b0:496:c5d7:dbd0 with SMTP id gu6-20020a056214260600b00496c5d7dbd0mr1596081qvb.86.1661386988440; Wed, 24 Aug 2022 17:23:08 -0700 (PDT) Received: from smtpclient.apple ([192.210.24.132]) by smtp.gmail.com with ESMTPSA id p123-20020a37bf81000000b006a6d7c3a82esm15754645qkf.15.2022.08.24.17.23.07 for (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Wed, 24 Aug 2022 17:23:07 -0700 (PDT) From: Ridley Combs Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3696.120.41.1.1\)) Date: Wed, 24 Aug 2022 19:23:06 -0500 References: <20220824235200.22312-1-rcombs@rcombs.me> <20220824235200.22312-3-rcombs@rcombs.me> To: ffmpeg-devel In-Reply-To: Message-Id: <59FAE899-3C19-4F17-881E-6E954F00DAA2@rcombs.me> X-Mailer: Apple Mail (2.3696.120.41.1.1) X-Content-Filtered-By: Mailman/MimeDel 2.1.29 Subject: Re: [FFmpeg-devel] [PATCH 3/3] lavf/id3v2dec: support multiple values and TIPL frames 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: > On Aug 24, 2022, at 19:16, Soft Works wrote: > > > >> -----Original Message----- >> From: ffmpeg-devel > On Behalf Of >> rcombs >> Sent: Thursday, August 25, 2022 1:52 AM >> To: ffmpeg-devel@ffmpeg.org >> Subject: [FFmpeg-devel] [PATCH 3/3] lavf/id3v2dec: support multiple >> values and TIPL frames >> >> Fixes https://trac.ffmpeg.org/ticket/6949 >> >> Ordinary text frames in ID3v2 are allowed to have multiple >> (null-separated) values. This technically isn't allowed in TXXX, >> but it's used in practice by Picard, and supporting it is harmless. >> >> TIPL/IPL (Involved People List) and TMCL (Musician Credits List) work >> similarly to TXXX, but alternate key-value-key-value. >> --- >> libavformat/id3v2.c | 49 ++++++++++++++++++++++++++----------------- >> -- >> 1 file changed, 28 insertions(+), 21 deletions(-) >> >> diff --git a/libavformat/id3v2.c b/libavformat/id3v2.c >> index 191a305ffb..335a1436b2 100644 >> --- a/libavformat/id3v2.c >> +++ b/libavformat/id3v2.c >> @@ -321,8 +321,12 @@ static void read_ttag(AVFormatContext *s, >> AVIOContext *pb, int taglen, >> AVDictionary **metadata, const char *key) >> { >> uint8_t *dst; >> - int encoding, dict_flags = AV_DICT_DONT_OVERWRITE | >> AV_DICT_DONT_STRDUP_VAL; >> + uint8_t *dst_key = NULL; >> + int encoding, dict_flags = AV_DICT_MULTIKEY | >> AV_DICT_DONT_STRDUP_VAL | AV_DICT_DEDUP; >> unsigned genre; >> + int count = 0; >> + int is_tipl = !(strcmp(key, "TIPL") && strcmp(key, "TMCL") && >> + strcmp(key, "IPL")); >> >> if (taglen < 1) >> return; >> @@ -330,30 +334,33 @@ static void read_ttag(AVFormatContext *s, >> AVIOContext *pb, int taglen, >> encoding = avio_r8(pb); >> taglen--; /* account for encoding type byte */ >> >> - if (decode_str(s, pb, encoding, &dst, &taglen) < 0) { >> - av_log(s, AV_LOG_ERROR, "Error reading frame %s, skipped\n", >> key); >> - return; >> - } >> - >> - if (!(strcmp(key, "TCON") && strcmp(key, "TCO")) >> && >> - (sscanf(dst, "(%d)", &genre) == 1 || sscanf(dst, "%d", >> &genre) == 1) && >> - genre <= ID3v1_GENRE_MAX) { >> - av_freep(&dst); >> - dst = av_strdup(ff_id3v1_genre_str[genre]); >> - } else if (!(strcmp(key, "TXXX") && strcmp(key, "TXX"))) { >> - /* dst now contains the key, need to get value */ >> - key = dst; >> + while (taglen > 1) { > > int n = 0; > >> if (decode_str(s, pb, encoding, &dst, &taglen) < 0) { >> av_log(s, AV_LOG_ERROR, "Error reading frame %s, >> skipped\n", key); >> - av_freep(&key); >> return; >> } >> - dict_flags |= AV_DICT_DONT_STRDUP_KEY; >> - } else if (!*dst) >> - av_freep(&dst); >> >> - if (dst) >> - av_dict_set(metadata, key, dst, dict_flags); >> + count++; >> + >> + if (!(strcmp(key, "TCON") && strcmp(key, "TCO")) >> && >> + (sscanf(dst, "(%d)", &genre) == 1 || sscanf(dst, "%d", >> &genre) == 1) && > > (sscanf(dst, "(%d)", &genre) == 1 || (sscanf(dst, "%d%n", &genre, &n) == 1 && n == strlen(dst))) && > > avoids parsing genre strings starting with numbers (like '2step') > as genre id. Sounds reasonable, but this isn't new code (it's just reindented); please send this as its own patch. > > > Thanks for resubmitting, > softworkz > > _______________________________________________ > 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". _______________________________________________ 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".