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 C08094A48C for ; Thu, 30 May 2024 19:26:48 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 2968868D2A5; Thu, 30 May 2024 22:26:45 +0300 (EEST) Received: from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net [217.70.183.194]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 1255168C2E4 for ; Thu, 30 May 2024 22:26:39 +0300 (EEST) Received: by mail.gandi.net (Postfix) with ESMTPSA id 5D25840005 for ; Thu, 30 May 2024 19:26:38 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=niedermayer.cc; s=gm1; t=1717097198; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=w7A/qatKWcywv8cee2/QoDq4FdNwS619JcpXA3wiUsE=; b=KrawPboY9Ih9jETkTT1ZAHG5SNflsTvbIiK8kqBUaMHsYeu0j5hOfXJ8luPtTz2UoH9nTb R6klWCRR1mOXF6L49BWP7ue5wdb/O7JCRkqEQB97USsTnOBk3eV0Ak+j7IzUyvKQqhDErm 3cmUs4/Za1pePaSf1Fl2rMMFCYfAQqGr08TTrBqAEtEhzCSnxSzmF5YYNz6v/gp7FA3E2V EPUcMNvgSY/2DKD1p5oTdfFnVK14vT0QWrlV2ztiS/5h2HsAO3UQketnBmxvXWitorwQAj p5y8NNLWoCVakI5gR0nSraza1NtX0Xv8DBwI2j59rY2NKv0XEntVFMthHJKZvw== Date: Thu, 30 May 2024 21:26:37 +0200 From: Michael Niedermayer To: FFmpeg development discussions and patches Message-ID: <20240530192637.GC2821752@pb2> References: <20240530174429.GZ2821752@pb2> <20240530180435.GA2821752@pb2> MIME-Version: 1.0 In-Reply-To: X-GND-Sasl: michael@niedermayer.cc 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: multipart/mixed; boundary="===============8942473617078142660==" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: --===============8942473617078142660== Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="ROA1rv1+fHr2QGor" Content-Disposition: inline --ROA1rv1+fHr2QGor Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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, AVIO= Context *bc, AVPacket *pkt, int > >>>> int count =3D ffio_read_varlen(bc); > >>>> int skip_start =3D 0; > >>>> int skip_end =3D 0; > >>>> - int channels =3D 0; > >>>> - int64_t channel_layout =3D 0; > >>>> int sample_rate =3D 0; > >>>> int width =3D 0; > >>>> int height =3D 0; > >>>> @@ -930,7 +928,7 @@ static int read_sm_data(AVFormatContext *s, AVIO= Context *bc, AVPacket *pkt, int > >>>> AV_WB64(dst, v64); > >>>> dst +=3D 8; > >>>> } else if (!strcmp(name, "ChannelLayout") && value_len = =3D=3D 8) { > >>>> - channel_layout =3D 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, AVIO= Context *bc, AVPacket *pkt, int > >>>> } else if (!strcmp(name, "SkipEnd")) { > >>>> skip_end =3D value; > >>>> } else if (!strcmp(name, "Channels")) { > >>>> - channels =3D value; > >>>> + // Ignored > >>>> } else if (!strcmp(name, "SampleRate")) { > >>>> sample_rate =3D value; > >>>> } else if (!strcmp(name, "Width")) { > >>>> @@ -965,18 +963,14 @@ static int read_sm_data(AVFormatContext *s, AV= IOContext *bc, AVPacket *pkt, int > >>>> } > >>>> } > >>>> =20 > >>>> - if (channels || channel_layout || sample_rate || width || heigh= t) { > >>>> - uint8_t *dst =3D av_packet_new_side_data(pkt, AV_PKT_DATA_P= ARAM_CHANGE, 28); > >>>> + if (sample_rate || width || height) { > >>>> + uint8_t *dst =3D av_packet_new_side_data(pkt, AV_PKT_DATA_P= ARAM_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 stre= am 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. > >=20 > > i honestly belive that we should support changing channel(layout) for > > cases like PCM in nut > >=20 >=20 > 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 chang= es 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 thx [...] --=20 Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB If the United States is serious about tackling the national security threat= s=20 related to an insecure 5G network, it needs to rethink the extent to which = it values corporate profits and government espionage over security.-Bruce Schn= eier --ROA1rv1+fHr2QGor Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iF0EABEKAB0WIQSf8hKLFH72cwut8TNhHseHBAsPqwUCZljS5gAKCRBhHseHBAsP q+wjAJsFIQQ/d5fQk8NbJchKF0kS2xfZrgCcDNvoYsM85HKSOj7fxjU6c+h/d6U= =P/ZC -----END PGP SIGNATURE----- --ROA1rv1+fHr2QGor-- --===============8942473617078142660== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ 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". --===============8942473617078142660==--