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 65ABF4B204 for ; Thu, 30 May 2024 21:47:07 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 9A74E68D288; Fri, 31 May 2024 00:47:05 +0300 (EEST) Received: from relay4-d.mail.gandi.net (relay4-d.mail.gandi.net [217.70.183.196]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id BB83E68D14E for ; Fri, 31 May 2024 00:46:58 +0300 (EEST) Received: by mail.gandi.net (Postfix) with ESMTPSA id 1B177E0004 for ; Thu, 30 May 2024 21:46:57 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=niedermayer.cc; s=gm1; t=1717105618; 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=RzWNWsa6DMEAo1xDdonZS4ZesyXGVl50YCqbPu9+f+Q=; b=lxWliQ7F+UydavyU2GqJ2gXyeEnf9WnTOwEOolCxhEimKTzyG9J07tEhlUF4aD1kaAUtfM eqsE/poArDOHVxs1ZZml1McIEGIha5GZHNf6v4HX4OFlq1tPcm6rzk3EmjaoWbqVBRDPPW iBtcn8NjyATStiMaweCCXvAud/mgU6uhtcK7oeNmANtbJqJIg+BS9wx0bIQzbVq237j9s3 bPx9SlwaePGF8dokW35nkFGjr7veQuEl3SJ4RdL7buLvcSpeppxVlnAmRrGVEysq0p+ukL gINhw3kZDuI5vnh+v1Z9Zs717lr6bbYmCGMmS+BajeB2TKDRwQLkBimkqRqZPg== Date: Thu, 30 May 2024 23:46:57 +0200 From: Michael Niedermayer To: FFmpeg development discussions and patches Message-ID: <20240530214657.GF2821752@pb2> References: <20240530174429.GZ2821752@pb2> <20240530180435.GA2821752@pb2> <20240530192637.GC2821752@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="===============2409861167581171833==" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: --===============2409861167581171833== Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="UkJByDtiyhV6M6Gk" Content-Disposition: inline --UkJByDtiyhV6M6Gk Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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, AV= IOContext *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, AV= IOContext *bc, AVPacket *pkt, int > >>>>>> AV_WB64(dst, v64); > >>>>>> dst +=3D 8; > >>>>>> } else if (!strcmp(name, "ChannelLayout") && value_le= n =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, AV= IOContext *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, = AVIOContext *bc, AVPacket *pkt, int > >>>>>> } > >>>>>> } > >>>>>> =20 > >>>>>> - if (channels || channel_layout || sample_rate || width || hei= ght) { > >>>>>> - uint8_t *dst =3D av_packet_new_side_data(pkt, AV_PKT_DATA= _PARAM_CHANGE, 28); > >>>>>> + if (sample_rate || width || height) { > >>>>>> + uint8_t *dst =3D 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 & channel= s when it > >>>>> is carried at format level only > >>>>> > >>>>> The commit message also does not adequately explain why such mid st= ream 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 layo= ut > >>>> 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). > >=20 > > You can fix the inconsistency in 2 directions > > 1. remove everyting > > 2. add the code back that made it inconsistant > >=20 > > This line between these 2 points is not orthogonal to what this patch c= hanges > > 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 > >=20 > > 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 subop= timal > >=20 >=20 > 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 thx [...] --=20 Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Those who are too smart to engage in politics are punished by being governed by those who are dumber. -- Plato=20 --UkJByDtiyhV6M6Gk Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iF0EABEKAB0WIQSf8hKLFH72cwut8TNhHseHBAsPqwUCZljzzQAKCRBhHseHBAsP q+KAAKCLp680lzXitcxOAt3eBCPkUYhwlgCfY9UsrjjePnXGdj1+rF5mpvi6AB4= =Y6rJ -----END PGP SIGNATURE----- --UkJByDtiyhV6M6Gk-- --===============2409861167581171833== 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". --===============2409861167581171833==--