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 313424B208 for ; Thu, 30 May 2024 22:02:11 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 13D2568D355; Fri, 31 May 2024 01:02:09 +0300 (EEST) Received: from EUR05-AM6-obe.outbound.protection.outlook.com (mail-am6eur05olkn2106.outbound.protection.outlook.com [40.92.91.106]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 3B1F468D14E for ; Fri, 31 May 2024 01:02:02 +0300 (EEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Fmk5mlUdACLPd/0f9zSHofK9RmeprhSAV1VWJB1og8YYLFKk3ChotX7sA/+y+8gpT7WqZH7Q1IZhZ/PDGGrO7rGG4PQXCwl65U9R7r0QFhS4pXQuWtG+nPGY9qLmSK35J+3ROd44FyBXnHWmtL+kN8WL0N6yoELsFimNDod51H7N8a1zPbv1+sONCDeV+xGDC4yw7Y6aXY9vq+09Yotd+FnzU8DI3D7tfw4EU2xiuzowmAJdOD0f2aSz2ZD5zuNBSx+CKhMaPM22M4CALD/4gqhLIKr80L55XWLfijZXZNPgrrSx0lGyAPXcrb0B3ghwJhrnJS7J7/7M3dJqaYYDDw== 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=nujME1RGbCnyhGXpVhK4FbAumX3fOTi59XbWhTPwbs0=; b=Zw1PqbDoMN7BS4QwSWHI1rrgz5aciXcv9ijaEdDjLuQpx9rUuk25wVdVa9feTUUSVX37ficIslRTZ3zWJf5PTZW3EhohodVJg8cuG5MEYHjiD8UhhOv139p7UkOXVEfivH1I9o2QlaXSELEHDDO0pXFxu8WazUTQhOB7HU6A1swuKbIs3TuI3k682aM0zFwcERBceSoAQgNBc6g+nFphqHmIpYYIwvEvTJy+bW5rLQhC5769uZRPUh9fklebXXzHpfHqK5exzR+BC/cmIc67CbRRkniioVOySMK+xvh8YdnEievvYSKy3mChDe0iJy4CCE8Ve1Bwk66Miv6vGarV/w== 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=nujME1RGbCnyhGXpVhK4FbAumX3fOTi59XbWhTPwbs0=; b=mXz3j5QKy+yCNQANaQOreyOM3/O1U+nNQGTIut1tzrtkvzXqBjWRmFKyultNcP3EW19cC+Yh5mzsXsSH+6YNzQ2UKLarGiJJfDqxJm9X5cgFRiWakAZohxAZlvt0283tuFiXFfcii3LKXA5trSCcPFOb7hYH+jp00uykhzF5l8cuRl9c3QZBHwPn0qRFhywm9jBMwYEP84Cg6tLNryiWAM+e98EN3hzXaBDPKRy34f6az7O1W9cAmBPFK3XAAV7HbM6Mhf1QbuK6r+F6LrNBPG1PGtVy3EzyAnteNCZ6peYnmTmyPH0jGqQEJO49QtWUaKOxThfQlA4TsFmSZm06RA== Received: from AS8P250MB0744.EURP250.PROD.OUTLOOK.COM (2603:10a6:20b:541::14) by AS1P250MB0454.EURP250.PROD.OUTLOOK.COM (2603:10a6:20b:4a8::17) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7633.21; Thu, 30 May 2024 22:01:59 +0000 Received: from AS8P250MB0744.EURP250.PROD.OUTLOOK.COM ([fe80::384d:40d4:ecb7:1c9]) by AS8P250MB0744.EURP250.PROD.OUTLOOK.COM ([fe80::384d:40d4:ecb7:1c9%4]) with mapi id 15.20.7611.025; Thu, 30 May 2024 22:01:59 +0000 Message-ID: Date: Fri, 31 May 2024 00:01:56 +0200 User-Agent: Mozilla Thunderbird To: ffmpeg-devel@ffmpeg.org References: <20240530174429.GZ2821752@pb2> <20240530180435.GA2821752@pb2> <20240530192637.GC2821752@pb2> <20240530214657.GF2821752@pb2> Content-Language: en-US From: Andreas Rheinhardt In-Reply-To: <20240530214657.GF2821752@pb2> X-TMN: [Mrq+0+w8CXpt8tc43LYFClFMZlqav/vzJAwXNlWj/j4=] X-ClientProxiedBy: ZR0P278CA0135.CHEP278.PROD.OUTLOOK.COM (2603:10a6:910:40::14) To AS8P250MB0744.EURP250.PROD.OUTLOOK.COM (2603:10a6:20b:541::14) X-Microsoft-Original-Message-ID: <38e249bf-31fd-41f7-abef-c3564b8649ec@outlook.com> MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: AS8P250MB0744:EE_|AS1P250MB0454:EE_ X-MS-Office365-Filtering-Correlation-Id: eb5c8556-5b4c-4603-f35c-08dc80f420aa X-Microsoft-Antispam: BCL:0;ARA:14566002|461199019|3412199016|440099019; X-Microsoft-Antispam-Message-Info: 8S6NpuqnbND8RcZKVqRFYcaefOTgloJsvhaJUfeBpRTahvy7/JOsbKYDyXr79HnMjt8/e6F7B1qa3gPT3u8gQ5+pMnyQ4l8KKgc9DCiDxUfUri2cp8lj4ZuB0TuX4Ep4/kBD5obHQxZSbTFwnMfHk13zEIgPkKaZX+hJUxcI/xEiamKQTz0frgJCXcojjapmRxuNDf80HAsZ7vVqOuOGzp/uEFk27PjZsxcACgLsFiC+c0wO/11stcM2gNTMQFcA2UrHUzfvMQv9kNQCBJ06Jgxx2AR/qdbAMHi3SZ2emdh0Mu/gqBPfo7LucfdPHZIrDLYA1nuhZZkvaf08hbyo0cklmZBmHm1hg402nUcoGuLQEPHIytKUTkd0WzEqdR4FrzEfEzzbSvIwmEkgoC/rglJ5R/vCjwDwbITkbllAfl6UPHOIN7pZtCoSdOos3TASWPYfMgW+E0rnKIDl16WCk9qyUtR4qenX/dsTJcVA3GwdMtDd5/f3ObRyT4owB4G2X9g8YPRsrIJoz9cUsNbnFIZRuJnUwXrywXmk3bY4zA0gkVtkHd/4IDtKNtlqPJly X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?a01VT1p4SGU1Y3AzUDd2WWt5MmZ3dDd0dGhRYm42akRJSDdWZU8ybG5aWU9Q?= =?utf-8?B?WGo0NDN1bFZ5emE4T0JTb0xOL2MyMEJ5K01PZEEyd201b292SkVpQ0hsMWRM?= =?utf-8?B?MHNCY0FtSUFOaTAyMCtSSzgvcEg5VEhYYWxaK21BeDVCT0hVSWpSc2dLY2V1?= =?utf-8?B?RkJuVnNzdlNFd29hQ0RPSmhaTkJPQ2FscXBrRVFFT0tNNlBNeFJuSGZvYXJm?= =?utf-8?B?bUVVK2xkSGp2NkNBYSsxbHA2VUY5cENUUURYZWx1TXFVVzFBTW1BRjA5Q2k0?= =?utf-8?B?WUltY3BhTW0yTnFOMjBTZDV2WTQ0cDJXbkRtK3VNYlU1WHNmRUd3SURKYmhX?= =?utf-8?B?aW1SR3lpK2NFL1RrWlA3bmg2dzN1NFNkVUh2a2kwUWM4UUhsbVVOcjIxVVZP?= =?utf-8?B?elFlMkhYTDh3eU5sTXNXeDdUQWdzNzZMUFFYbWg2cGFiaDBMeWFYajg3OUoy?= =?utf-8?B?dEZNRDU2ZUV0OGtFdmZLZW81aFRUeFJtWmRuSElHdXlWOC93a0JkekpGSDV4?= =?utf-8?B?NXV5bXdCWi9JYllzOVV1TFdCOWZmYkRlZEx4VjAyVlJwUmRwYjdzK3Y2bjdQ?= =?utf-8?B?SDB3dy9YcXpEVU5tTmp0Q3E1Wnh6Sll1NVllcCt0eVAyQi9GU2RQTHhEWWdB?= =?utf-8?B?MkdpZGtRVDJVRG5TY0w5bDlNdFFSQVM2djE5bVhFNjE3cWFZTUgvYzM4eHBK?= =?utf-8?B?OVdlS1dJSVVHZ1dMTlJMdnc5aUYrOUZyNysvdjZYeU5yZFVWbDJtSkNhbHcz?= =?utf-8?B?bFlWWG9jQ0x3dUhQamcyNU9zeVdReE9XNC83OG03K1NPMzZYSVR0MHBMSDll?= =?utf-8?B?Y1k2NXJteXp6dHg3Y0FvSkxkUTJESEJPa1VqdzVtM2ovNTZCUjBKWWI4MWg2?= =?utf-8?B?d3BHQWR0eXJBNlZLV1NxOFh5VDhpdDJPdytybjBaZGhzWGJ5a3d4WGdJUkRY?= =?utf-8?B?a1dNTGhVR3N0S1FseGEyT1czLzdqTmluSXkrSEx3TzdpWGlzYlQyUm1yb0sx?= =?utf-8?B?eDdCK0V2Ym92T0p2T29MQWROY2d0aGprOEx4eXpVTzI2bzVuemRvb2lhTDFk?= =?utf-8?B?ejAzMHR2b1pXejdyeDF6SXVWS0xRMUpZcUQ3UmhNUDY1V2E1NW9FaG5Ua1Ex?= =?utf-8?B?VUljVkxIdGpKcTNIcHRIZXJuNDEyc1U5cGJ3MGVGekxLZm93aEpuUDNyQ1JS?= =?utf-8?B?bmE2cU03T0NJTDdaOUxFcDFpRmdkZzFSb1p0Yk9YTk1qeE9Fc1k5Vk5keHVC?= =?utf-8?B?cnAwQTBvUFlGOTlFMHlURmptOThOaE9IVDZIM0Z5cTFGdTB6RnovSjlBZjVW?= =?utf-8?B?eUkzUGk3VEtXcXVQenhBS1plaXlhVjk4U3dTTnJ5VUMrOVluNXR0dGgxNFVX?= =?utf-8?B?Qk5IUjUvc3V2OVUrVkVMdHZDNUJlcEM3b0dUbWhTRFB6ZE5QREN1OHlrNUlF?= =?utf-8?B?UHlZbVNRVjFpSWRhenZ3cEFiN3dPOEk5dEdLbWd1cTR5ZEpId0o0L2Q4My9X?= =?utf-8?B?aWJOUk1oT0xPTXBSYWxsTTJ4aVFkci9FR3pIeXNETndYTGcvbytLV21yaitk?= =?utf-8?B?dDdtOTM0SlVsQ1N1YjFPVkFTU3VLUWlpVWJRUDhEV3orNUljNmo0SWRHaWdz?= =?utf-8?B?MDk5b0VCSDZIKzBjR0VKbFM4RENxYXdrcktkS3BGMi9lNGFhcXQ5SjlyNStx?= =?utf-8?B?YmVUVkNFSmlOWkdCOG9ueDlPNThxVWxLcWFQZ3dZZUxnamxyS0M5LzFBPT0=?= X-OriginatorOrg: outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: eb5c8556-5b4c-4603-f35c-08dc80f420aa X-MS-Exchange-CrossTenant-AuthSource: AS8P250MB0744.EURP250.PROD.OUTLOOK.COM X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 May 2024 22:01:58.9958 (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: AS1P250MB0454 Subject: Re: [FFmpeg-devel] [PATCH v3] avformat/nutdec: Don't create inconsistent side data 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: Michael Niedermayer: > On Thu, May 30, 2024 at 09:33:51PM +0200, Andreas Rheinhardt wrote: >> Michael Niedermayer: >>> On Thu, May 30, 2024 at 08:07:48PM +0200, Andreas Rheinhardt wrote: >>>> Michael Niedermayer: >>>>> On Thu, May 30, 2024 at 07:53:42PM +0200, Andreas Rheinhardt wrote: >>>>>> Michael Niedermayer: >>>>>>> On Thu, May 30, 2024 at 02:14:20AM +0200, Andreas Rheinhardt wrote: >>>>>>>> Forgotten in 65ddc74988245a01421a63c5cffa4d900c47117c. >>>>>>>> >>>>>>>> Signed-off-by: Andreas Rheinhardt >>>>>>>> --- >>>>>>>> libavformat/nutdec.c | 14 ++++---------- >>>>>>>> 1 file changed, 4 insertions(+), 10 deletions(-) >>>>>>>> >>>>>>>> diff --git a/libavformat/nutdec.c b/libavformat/nutdec.c >>>>>>>> index 0bb7f154db..34b7e3cb9a 100644 >>>>>>>> --- a/libavformat/nutdec.c >>>>>>>> +++ b/libavformat/nutdec.c >>>>>>>> @@ -881,8 +881,6 @@ static int read_sm_data(AVFormatContext *s, AVIOContext *bc, AVPacket *pkt, int >>>>>>>> int count = ffio_read_varlen(bc); >>>>>>>> int skip_start = 0; >>>>>>>> int skip_end = 0; >>>>>>>> - int channels = 0; >>>>>>>> - int64_t channel_layout = 0; >>>>>>>> int sample_rate = 0; >>>>>>>> int width = 0; >>>>>>>> int height = 0; >>>>>>>> @@ -930,7 +928,7 @@ static int read_sm_data(AVFormatContext *s, AVIOContext *bc, AVPacket *pkt, int >>>>>>>> AV_WB64(dst, v64); >>>>>>>> dst += 8; >>>>>>>> } else if (!strcmp(name, "ChannelLayout") && value_len == 8) { >>>>>>>> - channel_layout = avio_rl64(bc); >>>>>>>> + // Ignored >>>>>>>> continue; >>>>>>>> } else { >>>>>>>> av_log(s, AV_LOG_WARNING, "Unknown data %s / %s\n", name, type_str); >>>>>>>> @@ -952,7 +950,7 @@ static int read_sm_data(AVFormatContext *s, AVIOContext *bc, AVPacket *pkt, int >>>>>>>> } else if (!strcmp(name, "SkipEnd")) { >>>>>>>> skip_end = value; >>>>>>>> } else if (!strcmp(name, "Channels")) { >>>>>>>> - channels = value; >>>>>>>> + // Ignored >>>>>>>> } else if (!strcmp(name, "SampleRate")) { >>>>>>>> sample_rate = value; >>>>>>>> } else if (!strcmp(name, "Width")) { >>>>>>>> @@ -965,18 +963,14 @@ static int read_sm_data(AVFormatContext *s, AVIOContext *bc, AVPacket *pkt, int >>>>>>>> } >>>>>>>> } >>>>>>>> >>>>>>>> - if (channels || channel_layout || sample_rate || width || height) { >>>>>>>> - uint8_t *dst = av_packet_new_side_data(pkt, AV_PKT_DATA_PARAM_CHANGE, 28); >>>>>>>> + if (sample_rate || width || height) { >>>>>>>> + uint8_t *dst = av_packet_new_side_data(pkt, AV_PKT_DATA_PARAM_CHANGE, 16); >>>>>>>> if (!dst) >>>>>>>> return AVERROR(ENOMEM); >>>>>>>> bytestream_put_le32(&dst, >>>>>>>> AV_SIDE_DATA_PARAM_CHANGE_SAMPLE_RATE*(!!sample_rate) + >>>>>>>> AV_SIDE_DATA_PARAM_CHANGE_DIMENSIONS*(!!(width|height)) >>>>>>>> ); >>>>>>>> - if (channels) >>>>>>>> - bytestream_put_le32(&dst, channels); >>>>>>>> - if (channel_layout) >>>>>>>> - bytestream_put_le64(&dst, channel_layout); >>>>>>>> if (sample_rate) >>>>>>>> bytestream_put_le32(&dst, sample_rate); >>>>>>>> if (width || height){ >>>>>>> >>>>>>> This would break mid stream changes to the channel layout & channels when it >>>>>>> is carried at format level only >>>>>>> >>>>>>> The commit message also does not adequately explain why such mid stream changes >>>>>>> are ignored >>>>>>> >>>>>> >>>>>> Mid-stream changes like this have been deprecated in >>>>>> 09b5d3fb44ae1036700f80c8c80b15e9074c58c3; >>>>>> 65ddc74988245a01421a63c5cffa4d900c47117c removed it, but only >>>>>> incompletely: The side data flags for channel count and channel layout >>>>>> changes were no longer written (in fact, they were removed from >>>>>> packet.h), yet it still wrote the rest of the side data as if these >>>>>> flags existed and had been written. That is the inconsistency this >>>>>> commit addresses. It does not address whether channel count/layout >>>>>> updates should have been removed, because that has already happened. >>>>> >>>>> i honestly belive that we should support changing channel(layout) for >>>>> cases like PCM in nut >>>>> >>>> >>>> That is orthogonal to this patch (which just wants to not create >>>> inconsistent side data). >>> >>> You can fix the inconsistency in 2 directions >>> 1. remove everyting >>> 2. add the code back that made it inconsistant >>> >>> This line between these 2 points is not orthogonal to what this patch changes >>> It also is not orthoginal to supporting PCM channel changes in NUT >>> nor is the change this patch does from our current state orthogonal >>> to what would be needed to support channel changes >>> >>> IMHO, decide on what the end goal is and work toward it. Not just >>> make something consistent even when its a direction that might be suboptimal >>> >> >> We have a release that is able to create nonsense side data; this needs >> to be fixed and for this to be fixed we need a patch to backport. > > what do you mean by "nonsense side data" ? > > I would assume the side data is what was documented previously > 65ddc74988245a01421a63c5cffa4d900c47117c removed the code that set the bits indicating that new channel layout/count data is present (because the flags have been removed); yet it did not remove the code actually writing said fields. This means that if there is a new channel layout (or simply the old one repeated) and a new sample rate, the demuxer and a user/reader have differing opinions about the side data: The demuxer thinks it wrote the new sample rate at offset 12, a reader thinks it is at offset 4. That is the inconsistency the commit message refers to. - Andreas _______________________________________________ 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".