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 C54AA43665 for ; Mon, 20 Jun 2022 22:49:31 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 8F54468B635; Tue, 21 Jun 2022 01:49:28 +0300 (EEST) Received: from NAM12-DM6-obe.outbound.protection.outlook.com (mail-dm6nam12olkn2038.outbound.protection.outlook.com [40.92.22.38]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id C693268B522 for ; Tue, 21 Jun 2022 01:49:21 +0300 (EEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=oPIoNOyA+QZARLh6Rd9i+ceW95G/pZtFs5W0JXMpDTGQ09buk5p9ASsmfYbsTyWlhUXgC1xLQIRjEJCqS1j+vjgu8ju4L2ytJWbFOvrwdIVRS9NwDw+ciypnrsUE7MCDA/fwCcC2WCcsf9msAU8DCdBq2EwV4225xZ1iloaOI8eiX92FKqdaYzqsEyF6GMrrgpAjLplqc8Xzi3dMvuoyJTrKCSwjc6BcUCrcTeCH+7P+m0BFR+YouNWXADr8H4v2611zmzm3797cf50caNMsMmEuvyVPOKJqGZKDGwh23Ln3jhHXqyhv3TDx14AS3jjPzUbdswmCZM98PbZJLxLI5Q== 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=rpz2pyYe4QaSBS8qpPiQqt+O0IHNBPunalwH7921Bi0=; b=mYZLRutkf6UHQkvMI5OZi5zghArjKERUM7l1mcx3M82wrahPDtJrWDARe1wK51YF0hLiXIEGzL0dEgdtZh848loExcfl/ghZScGvmK7LsytN7i8i6/eRxujy4reBAwK7EKy2sVDyH3bapPiYnA8wZTwwvOO1FlBbdwNkXD1wm64WjEE5VeGT6VX0PScU1gMCuAaz1jp1LHp0EwSIOVgrkonKYK7G1m/POHMkwhwxyPfD8fI8XGrmXKyoouzCDtJnrDwgitG0RDOBSJLzorO6rg/ratIPjGNt1JZ5OF9LFF9BvLerZzMKg6tM8MeklA5Tyqu4gP3u9NTEw7ZNYt647A== 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=hotmail.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=rpz2pyYe4QaSBS8qpPiQqt+O0IHNBPunalwH7921Bi0=; b=cbJkC/kIHrAs1CvmlVS88xzz4/Lgwxsh5uPb0cXjte0VBSDoMYo1W04JnfrIf2EXQ5zwVzb9xIY+xPKgffT96JXcvBlHS8la7EvzfTMCysjGiWTOdj0sCOr1b72pKWseOc/Kiqm75B+AzPn03pd6FUP4l8t/OItL5WzR4GB/3vsh/YpPpe4NeT6Kry4qkp0V7wuSs06TNE+AVV2xlECxs3T3QgG3iBBdDA/rl7PhhXsl5zpgK+KeqlBmTrrC7xlYe5W8Hexm4h3s9Lx9fjM9etOvqnkTKFTzCU40YM47nnAUrrF5fzWnitdQSavxBT7oa8hKyBqm9Ok0Lk49XnCBrA== Received: from DM8P223MB0365.NAMP223.PROD.OUTLOOK.COM (2603:10b6:8:b::20) by BL3P223MB0097.NAMP223.PROD.OUTLOOK.COM (2603:10b6:208:34d::6) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5353.14; Mon, 20 Jun 2022 22:49:19 +0000 Received: from DM8P223MB0365.NAMP223.PROD.OUTLOOK.COM ([fe80::d08c:b865:df15:6a]) by DM8P223MB0365.NAMP223.PROD.OUTLOOK.COM ([fe80::d08c:b865:df15:6a%6]) with mapi id 15.20.5353.022; Mon, 20 Jun 2022 22:49:18 +0000 From: Soft Works To: FFmpeg development discussions and patches Thread-Topic: [FFmpeg-devel] [PATCH 2/2] lavf/id3v2dec: support multiple values and TIPL frames Thread-Index: AQHYhEZZU40X28HTuUi6kImTyL0JpK1Y3gAAgAAEz4CAAAKTsA== Date: Mon, 20 Jun 2022 22:49:18 +0000 Message-ID: References: <20220620013711.91482-1-rcombs@rcombs.me> <20220620013711.91482-2-rcombs@rcombs.me> <766EA680-FF62-45A2-853A-E7C7B646A8E6@rcombs.me> In-Reply-To: <766EA680-FF62-45A2-853A-E7C7B646A8E6@rcombs.me> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-tmn: [aTBkgJbpUCHtHzr5OubLZBxm+h2I5Glv] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: c355e216-eccd-415e-d227-08da530f1c39 x-ms-traffictypediagnostic: BL3P223MB0097:EE_ x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: hBbE/Eqk0sl92vlDYE855g8xcELRydcDRxXQdxlU9ymBfT9oEtH3QY+7JxOsXXNEbHjGLTVD0mUWOpNpiT6y7DuF8xIe4soGH9KGLFBwmoJkvdl1x/BB7DQPCDWr7aCvSrjp6U3sjaFQC3fd7qOnSUUg6nrqkpCo4GplfoOju0N4BZ9itgFV409w/2iRyxgz0FXpv+464zeAtYEXfCXfAiP8hyf5QVRpe+iiHMDc8Mdm9Tdkp9qLvSlN8eyXDv92n//qObJ1mNNTASGGyOLdA4kxhwLa2AmzVOWdFxKZWqDd1X4secZYgrJt8ieNwLwxdURPVj5+zixGJimnbyvXim+GnGYfAlLt8Ivg1om8NoOmoSOusBAq+Cnu09yLEGI/yhSJkbs0njqKaHYODhJHbLWkax1mQbRqcMkNfZA8AAc0f4dTt6G4mIDY7+pWaU6c9oeX7pMjPO8cHnq/DQJjMPdjhBhZNqzz5QftNKEwjKsMFolJ2YGpV0vWMWylM0E1ao4izCu8Pj4P+UKsVM+10n62uiUVnqn+lkIb6pY3YrnMArehTa/MB47e42Th5DXUDOZa6I1euwlRi+2poN/DY4Uh0DOMOxZHZ0fL0U1yz9xxdK0tKWGWlZO5JMa8vIVI x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?utf-8?B?UzdlQ2pGUFlBT3hGOUkzbCtsdHZWUjZ2TnJHMFJVa1kybE5UUmw1d3B3R2h0?= =?utf-8?B?L3NjNjh6NzRxVTU4eGNYektXMElwQmNKYk52ejlxeDhwdDBnUW5XMVVQOUVz?= =?utf-8?B?ZGRPUnRhV0FKSVBrRFg0YkRJdFJibFh2TU80QVZwaW51R3ZWN2lPR3VLczdy?= =?utf-8?B?MVdNK3dRWTJpZlpvaDV6Ny9WUk9VYmQ0WGViTS8wZnltT3E4dmVSVW5BMVJZ?= =?utf-8?B?MnNFTFRBUjF4YXZHV0JkN3ZyNzkyMWtMT2tJT3l3R2RBR00xQVBSWUNPbUhi?= =?utf-8?B?SjNUcUs0Rkxhdmo0OWdBOGx2NEhhcUJYWkdjdHJ4aU9FeVR5K1FlZklBUWFI?= =?utf-8?B?TVI2QU5LbzdOdEJaZGg2eXdSc0lBMGpKbFpjbkE5NVk1RjV3SDFaSEw2aUFK?= =?utf-8?B?RlRxbmdrMm1jbjY5QzlwSU5jOURZTFI3Z1FqVkNmRGYvOHFWVlFQRzR0RkNl?= =?utf-8?B?SEorZTR6bzMvaVV6T09DdHJVbklMb0hndE5kUFFudDQ1RmQzMk5LZEFMUnNt?= =?utf-8?B?Y2JSU0pVbDdLTmhpMEg0UlVrTDlMM0x5ZnlnT3VUUk1Bb1U2QWNFeHVJRzNo?= =?utf-8?B?V1FCYm96dzNNbktJWXJqayt3NnZId1Q1a1pVT0xZd3VsK1Zmb2JsR2NZY3Fk?= =?utf-8?B?RTREaEtsL3FLcExTcEJrVktHMzlDamwyVm1KcUg4bEpXRHpmd3Bhdzc4TEZx?= =?utf-8?B?MUZaZDloU3VjMHEwRDlRSXlmQndlU3hqRkNVRDZFSy9zWnVVNzk5ZDI1bW1m?= =?utf-8?B?YW9PWGZhazNsbDlrbTlxUXl2c2ROclVnOHBNdklNK05VcHI4WnRHWllMc3hD?= =?utf-8?B?c2NwSndJd1k5TFc1Q216dVd0aW9iM29WbmRpWVplS05IUzRSdnRLVHJHM3JY?= =?utf-8?B?V0ZwQkdWWnQrOG8vYUdNeWhZTWd2ZU1lOXdTWWNKaUhwaEZIU0NaREh2bDg4?= =?utf-8?B?dDhXK3BjcDJ0S1pjd2lKY0VsQnN3RTA2UGVLakFVTTRDWjRzbTBZL0ZKZkJP?= =?utf-8?B?eGJ4OHlTaWlCNkhqUnBsZmtmSng4MmE0Vm1DR0RVVEJtVDB6aHdHU2ZpdGZP?= =?utf-8?B?RGU4ZUZtcmZmamNQbEs0dU0wYjlkMHdhV0U2c0t6d3h4WnBvN2VRTEtLKzVS?= =?utf-8?B?SXNwN2pDSm03akkvbWFHS2duUW56elAydjVQeEZaMjV2WXFmN3hoRnM2MTNP?= =?utf-8?B?NmRxSU1TZ0ZzUmZiL3pvR0hEYlcvL2dMWmZkZmJYejNtZFY0UFFHYXpHTktx?= =?utf-8?B?OGY3OWpPdng3bDh5TUpEeWk0OUcwY1dSTHo5RGZUMk0rallYY08rTmk3bUIx?= =?utf-8?B?NmpHbXVrT2lJbkVHWnlJZm5pS0NQK0xiTkplc1JSZ0lRL0ZWSnVQUzRXU2Qw?= =?utf-8?B?bFV6b3pKTjFya2VLOEtTZHNkQkM4QmZtRjd1VXhtSjMrcDVtUytQVmNmaWU0?= =?utf-8?B?dGNCRkVkTkg4TC84NVc5M3R6dVlOWkdpd3BwUnFVazdwZUVWMCt2Qk5tNFk1?= =?utf-8?B?OEJWNmJ3U3lKK3RuNE1iSjB3ck5SOWZVejhSMGxSRG5pNjJSdGVIMVh5N1JC?= =?utf-8?B?Skswd1FrWGprd2p3azVXMkNyUzljVzY0TzRIZWFQeS9seDJOSVY4UWs4V2FN?= =?utf-8?B?S1U0UGh6SVI5aTE5dm85YURPNVlJd0dlSGwrNUtiU05VMnc4bGdvNyt6WmR1?= =?utf-8?B?TnZqcWN5aDdRakhpSlVycmJwNWNXTEZYNGErNnp4aGZZUnBUZGUzQXdPelVv?= =?utf-8?B?UFRrVkJQelVjR1JnRWdJMkNXUlo2UG82aVVra2J1RTF5TG5JNm1WcjE3LzRF?= =?utf-8?B?K1haWFJDWEdwbjBsMllJRldyakVFalJFeHpCYWYxTW83a1VRNTRYVjBIeTJV?= =?utf-8?B?MktxanVqSzRmN1BTRG1waGErd2M4Y240Smx3c3NXck8wOGU5NmQ4TzVIZG03?= =?utf-8?Q?NahBEGM6sLM=3D?= MIME-Version: 1.0 X-OriginatorOrg: sct-15-20-4755-11-msonline-outlook-1ff67.templateTenant X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: DM8P223MB0365.NAMP223.PROD.OUTLOOK.COM X-MS-Exchange-CrossTenant-RMS-PersistedConsumerOrg: 00000000-0000-0000-0000-000000000000 X-MS-Exchange-CrossTenant-Network-Message-Id: c355e216-eccd-415e-d227-08da530f1c39 X-MS-Exchange-CrossTenant-originalarrivaltime: 20 Jun 2022 22:49:18.9356 (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: BL3P223MB0097 Subject: Re: [FFmpeg-devel] [PATCH 2/2] 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: > -----Original Message----- > From: ffmpeg-devel On Behalf Of > Ridley Combs > Sent: Tuesday, June 21, 2022 12:33 AM > To: ffmpeg-devel > Subject: Re: [FFmpeg-devel] [PATCH 2/2] lavf/id3v2dec: support > multiple values and TIPL frames > > > > On Jun 20, 2022, at 17:15, Andreas Rheinhardt > wrote: > > > > rcombs: > >> 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..667105e9f9 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; > >> 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) { > >> 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) && > >> + genre <= ID3v1_GENRE_MAX) { > >> + av_freep(&dst); > >> + dst = av_strdup(ff_id3v1_genre_str[genre]); > >> + } else if (!(strcmp(key, "TXXX") && strcmp(key, "TXX")) || > >> + (is_tipl && (count & 1))) { > >> + /* dst now contains the key, need to get value */ > >> + av_free(dst_key); > >> + key = dst_key = dst; > >> + continue; > >> + } else if (!*dst) > >> + av_freep(&dst); > >> + > >> + if (dst) > >> + av_dict_set(metadata, key, dst, dict_flags); > >> + } > >> + > >> + av_free(dst_key); > >> } > >> > >> static void read_uslt(AVFormatContext *s, AVIOContext *pb, int > taglen, > >> @@ -1039,7 +1046,7 @@ static void id3v2_parse(AVIOContext *pb, > AVDictionary **metadata, > >> pbx = &pb_local.pub; // read from sync buffer > >> } > >> #endif > >> - if (tag[0] == 'T') > >> + if (tag[0] == 'T' || !strcmp(tag, "IPL")) > >> /* parse text tag */ > >> read_ttag(s, pbx, tlen, metadata, tag); > >> else if (!memcmp(tag, "USLT", 4)) > > > > From avformat.h: > > > > "Keys are unique; there can never be 2 tags with the same key. This > is > > also meant semantically, i.e., a demuxer should not knowingly > produce > > several keys that are literally different but semantically > identical. > > E.g., key=Author5, key=Author6. In this example, all authors must > be > > placed in the same tag." > > > > If this requirement did not exist, I would have fixed #6949 and > #9641 > > long ago. > > > > - Andreas > > Well, we have 3 options, then: > - Export these kinds of duplicate tags concatenated in a single dict > entry (split by slashes or something?) > - Add an AVOption in lavf/options_table.h to opt into multi-key > support > - Remove the requirement altogether in a major API version bump That's the way I would prefer. Doing nothing for formal reasons is not a useful way. Most softwares can read multivalue ID3v2 tags, ffmpeg/probe is more a kind of exception. I think I should also mention that half part of this patch has been submitted before https://patchwork.ffmpeg.org/project/ffmpeg/patch/1118083116.39618.1617751350909@office.mailbox.org/ Best regards, 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".