From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.ffmpeg.org (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTPS id B7F8C4F21E for ; Sat, 17 May 2025 22:08:04 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.ffmpeg.org (Postfix) with ESMTP id E76DA68D9A7; Sun, 18 May 2025 01:07:55 +0300 (EEST) Received: from relay4-d.mail.gandi.net (relay4-d.mail.gandi.net [217.70.183.196]) by ffbox0-bg.ffmpeg.org (Postfix) with ESMTPS id E248268D993 for ; Sun, 18 May 2025 01:07:49 +0300 (EEST) Received: by mail.gandi.net (Postfix) with ESMTPSA id 1A3FD433C9 for ; Sat, 17 May 2025 22:07:48 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=niedermayer.cc; s=gm1; t=1747519669; 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=4xb4td2qbiWXqSfujRO8BIkmB12q5Oe+wemN0T0Ja6U=; b=irJ44UvB1H2MgqqPIkawcdkC8Okj3cxQuc13k17nRsiJLpVbbacPIKEAmuqiI0GhbGMcRY 7ZQtsE3b3odiq50xDfuwVowf6kMirkhPhK5Sw19mwCLCRrLxKfCXbvcqIVXL+krd1KUC4t zjoFYSfOHiJ9GxVlR7Q1rXCsKIClbF2FBlhBARKrww+LDaax8aYxW51cDppedZyt/U9F8l iPmXJhCPKhbXbMT6FopCETgEV5zPujjTfxmmpjxZXSdhQv2eaeugTzBwwXly0NwPso6Lls XXGCtHp2jSewrVjRRtGWEbah3UimKcCKr74bAwicQwFGRrphUxBQBQvCNWKcmg== Date: Sun, 18 May 2025 00:07:48 +0200 From: Michael Niedermayer To: FFmpeg development discussions and patches Message-ID: <20250517220748.GC29660@pb2> References: <20250509234327.71039-1-romain.beauxis@gmail.com> <20250509234327.71039-7-romain.beauxis@gmail.com> <20250513192339.GF29660@pb2> MIME-Version: 1.0 In-Reply-To: X-GND-State: clean X-GND-Score: -70 X-GND-Cause: gggruggvucftvghtrhhoucdtuddrgeefvddrtddtgdefudeijeekucetufdoteggodetrfdotffvucfrrhhofhhilhgvmecuifetpfffkfdpucggtfgfnhhsuhgsshgtrhhisggvnecuuegrihhlohhuthemuceftddunecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenfghrlhcuvffnffculdeftddmnecujfgurhepfffhvffukfhfgggtuggjsehgtderredttddunecuhfhrohhmpefoihgthhgrvghlucfpihgvuggvrhhmrgihvghruceomhhitghhrggvlhesnhhivgguvghrmhgrhigvrhdrtggtqeenucggtffrrghtthgvrhhnpeevueefgeefudetuefggfeivdetteekgfdukedtjeeuffevheegleduleffudfgheenucffohhmrghinhepfhhfmhhpvghgrdhorhhgnecukfhppeeguddrieeirdeijedruddufeenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepihhnvghtpeeguddrieeirdeijedruddufedphhgvlhhopehlohgtrghlhhhoshhtpdhmrghilhhfrhhomhepmhhitghhrggvlhesnhhivgguvghrmhgrhigvrhdrtggtpdhnsggprhgtphhtthhopedupdhrtghpthhtohepfhhfmhhpvghgqdguvghvvghlsehffhhmphgvghdrohhrgh X-GND-Sasl: michael@niedermayer.cc Subject: Re: [FFmpeg-devel] [PATCH v5 6/7] ogg/vorbis: implement header packet skip in chained ogg bitstreams. 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="===============0396432675855398868==" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: --===============0396432675855398868== Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="S7pq8suDAU0LBjBQ" Content-Disposition: inline --S7pq8suDAU0LBjBQ Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sat, May 17, 2025 at 01:10:26PM -0500, Romain Beauxis wrote: > Le mar. 13 mai 2025 =E0 14:23, Michael Niedermayer a > =E9crit : > > > > On Fri, May 09, 2025 at 06:43:26PM -0500, Romain Beauxis wrote: > > > --- > > > libavcodec/vorbisdec.c | 37 +---- > > > libavformat/oggparsevorbis.c | 174 +++++++++++++------= -- > > > tests/ref/fate/ogg-vorbis-chained-meta.txt | 3 - > > > 3 files changed, 117 insertions(+), 97 deletions(-) > > > > > > diff --git a/libavcodec/vorbisdec.c b/libavcodec/vorbisdec.c > > > index a778dc6b58..f069ac6ab3 100644 > > > --- a/libavcodec/vorbisdec.c > > > +++ b/libavcodec/vorbisdec.c > > > @@ -1776,39 +1776,17 @@ static int vorbis_decode_frame(AVCodecContext > *avctx, AVFrame *frame, > > > GetBitContext *gb =3D &vc->gb; > > > float *channel_ptrs[255]; > > > int i, len, ret; > > > + const int8_t *new_extradata; > > > + size_t new_extradata_size; > > > > > > ff_dlog(NULL, "packet length %d \n", buf_size); > > > > > > - if (*buf =3D=3D 1 && buf_size > 7) { > > > - if ((ret =3D init_get_bits8(gb, buf + 1, buf_size - 1)) < 0) > > > - return ret; > > > - > > > - vorbis_free(vc); > > > - if ((ret =3D vorbis_parse_id_hdr(vc))) { > > > - av_log(avctx, AV_LOG_ERROR, "Id header corrupt.\n"); > > > - vorbis_free(vc); > > > - return ret; > > > - } > > > - > > > - av_channel_layout_uninit(&avctx->ch_layout); > > > - if (vc->audio_channels > 8) { > > > - avctx->ch_layout.order =3D AV_CHANNEL_ORDER_UNSPEC; > > > - avctx->ch_layout.nb_channels =3D vc->audio_channels; > > > - } else { > > > - av_channel_layout_copy(&avctx->ch_layout, > &ff_vorbis_ch_layouts[vc->audio_channels - 1]); > > > - } > > > - > > > - avctx->sample_rate =3D vc->audio_samplerate; > > > - return buf_size; > > > - } > > > - > > > - if (*buf =3D=3D 3 && buf_size > 7) { > > > - av_log(avctx, AV_LOG_DEBUG, "Ignoring comment header\n"); > > > - return buf_size; > > > - } > > > + new_extradata =3D av_packet_get_side_data(avpkt, > AV_PKT_DATA_NEW_EXTRADATA, > > > + &new_extradata_size); > > > > > > - if (*buf =3D=3D 5 && buf_size > 7 && vc->channel_residues && > !vc->modes) { > > > - if ((ret =3D init_get_bits8(gb, buf + 1, buf_size - 1)) < 0) > > > + if (new_extradata && *new_extradata =3D=3D 5 && new_extradata_si= ze > 7 > && > > > + vc->channel_residues && !vc->modes) { > > > + if ((ret =3D init_get_bits8(gb, new_extradata + 1, > new_extradata_size - 1)) < 0) > > > return ret; > > > > > > if ((ret =3D vorbis_parse_setup_hdr(vc))) { > > > @@ -1816,7 +1794,6 @@ static int vorbis_decode_frame(AVCodecContext > *avctx, AVFrame *frame, > > > vorbis_free(vc); > > > return ret; > > > } > > > - return buf_size; > > > } > > > > > > if (!vc->channel_residues || !vc->modes) { > > > diff --git a/libavformat/oggparsevorbis.c b/libavformat/oggparsevorbi= s.c > > > index 9f50ab9ffc..452728b54d 100644 > > > --- a/libavformat/oggparsevorbis.c > > > +++ b/libavformat/oggparsevorbis.c > > > @@ -293,6 +293,62 @@ static int vorbis_update_metadata(AVFormatContext > *s, int idx) > > > return ret; > > > } > > > > > > +static int vorbis_parse_header(AVFormatContext *s, AVStream *st, > > > + const uint8_t *p, unsigned int psize) > > > +{ > > > + unsigned blocksize, bs0, bs1; > > > + int srate; > > > + int channels; > > > + > > > + if (psize !=3D 30) > > > + return AVERROR_INVALIDDATA; > > > + > > > + p +=3D 7; /* skip "\001vorbis" tag */ > > > + > > > + if (bytestream_get_le32(&p) !=3D 0) /* vorbis_version */ > > > + return AVERROR_INVALIDDATA; > > > + > > > + channels =3D bytestream_get_byte(&p); > > > + if (st->codecpar->ch_layout.nb_channels && > > > + channels !=3D st->codecpar->ch_layout.nb_channels) { > > > + av_log(s, AV_LOG_ERROR, "Channel change is not supported\n"); > > > + return AVERROR_PATCHWELCOME; > > > + } > > > + st->codecpar->ch_layout.nb_channels =3D channels; > > > + srate =3D bytestream_get_le32(&p); > > > + p +=3D 4; // skip maximum bitrate > > > + st->codecpar->bit_rate =3D bytestream_get_le32(&p); // nominal > bitrate > > > + p +=3D 4; // skip minimum bitrate > > > + > > > + blocksize =3D bytestream_get_byte(&p); > > > + bs0 =3D blocksize & 15; > > > + bs1 =3D blocksize >> 4; > > > + > > > + if (bs0 > bs1) > > > + return AVERROR_INVALIDDATA; > > > + if (bs0 < 6 || bs1 > 13) > > > + return AVERROR_INVALIDDATA; > > > + > > > + if (bytestream_get_byte(&p) !=3D 1) /* framing_flag */ > > > + return AVERROR_INVALIDDATA; > > > + > > > + st->codecpar->codec_type =3D AVMEDIA_TYPE_AUDIO; > > > + st->codecpar->codec_id =3D AV_CODEC_ID_VORBIS; > > > + > > > + if (srate > 0) { > > > + if (st->codecpar->sample_rate && > > > + srate !=3D st->codecpar->sample_rate) { > > > + av_log(s, AV_LOG_ERROR, "Sample rate change is not > supported\n"); > > > + return AVERROR_PATCHWELCOME; > > > + } > > > + > > > + st->codecpar->sample_rate =3D srate; > > > + avpriv_set_pts_info(st, 64, 1, srate); > > > + } > > > + > > > + return 1; > > > +} > > > + > > > static int vorbis_header(AVFormatContext *s, int idx) > > > { > > > struct ogg *ogg =3D s->priv_data; > > > @@ -300,6 +356,7 @@ static int vorbis_header(AVFormatContext *s, int > idx) > > > struct ogg_stream *os =3D ogg->streams + idx; > > > struct oggvorbis_private *priv; > > > int pkt_type =3D os->buf[os->pstart]; > > > + int ret; > > > > > > if (!os->private) { > > > os->private =3D av_mallocz(sizeof(struct oggvorbis_private)); > > > @@ -327,56 +384,18 @@ static int vorbis_header(AVFormatContext *s, int > idx) > > > > > > priv->len[pkt_type >> 1] =3D os->psize; > > > priv->packet[pkt_type >> 1] =3D av_memdup(os->buf + os->pstart, > os->psize); > > > + > > > if (!priv->packet[pkt_type >> 1]) > > > return AVERROR(ENOMEM); > > > - if (os->buf[os->pstart] =3D=3D 1) { > > > - const uint8_t *p =3D os->buf + os->pstart + 7; /* skip > "\001vorbis" tag */ > > > - unsigned blocksize, bs0, bs1; > > > - int srate; > > > - int channels; > > > - > > > - if (os->psize !=3D 30) > > > - return AVERROR_INVALIDDATA; > > > - > > > - if (bytestream_get_le32(&p) !=3D 0) /* vorbis_version */ > > > - return AVERROR_INVALIDDATA; > > > - > > > - channels =3D bytestream_get_byte(&p); > > > - if (st->codecpar->ch_layout.nb_channels && > > > - channels !=3D st->codecpar->ch_layout.nb_channels) { > > > - av_log(s, AV_LOG_ERROR, "Channel change is not > supported\n"); > > > - return AVERROR_PATCHWELCOME; > > > - } > > > - st->codecpar->ch_layout.nb_channels =3D channels; > > > - srate =3D bytestream_get_le32(&p); > > > - p +=3D 4; // skip maximum bitrate > > > - st->codecpar->bit_rate =3D bytestream_get_le32(&p); // nomin= al > bitrate > > > - p +=3D 4; // skip minimum bitrate > > > - > > > - blocksize =3D bytestream_get_byte(&p); > > > - bs0 =3D blocksize & 15; > > > - bs1 =3D blocksize >> 4; > > > - > > > - if (bs0 > bs1) > > > - return AVERROR_INVALIDDATA; > > > - if (bs0 < 6 || bs1 > 13) > > > - return AVERROR_INVALIDDATA; > > > - > > > - if (bytestream_get_byte(&p) !=3D 1) /* framing_flag */ > > > - return AVERROR_INVALIDDATA; > > > - > > > - st->codecpar->codec_type =3D AVMEDIA_TYPE_AUDIO; > > > - st->codecpar->codec_id =3D AV_CODEC_ID_VORBIS; > > > - > > > - if (srate > 0) { > > > - st->codecpar->sample_rate =3D srate; > > > - avpriv_set_pts_info(st, 64, 1, srate); > > > - } > > > - } else if (os->buf[os->pstart] =3D=3D 3) { > > > > > > moving code should be in a seperate patch so any changes can be clearly > > seen in the diff which does not move the code. > > > > The output from: > > libavformat/tests/seek $TICKETS/2739/lavf_ogm_seeking_borked.ogm > > > > chanegs by: > > @@ -273,7 +273,7 @@ > > ret: 0 st: 2 flags:0 ts: 0.365000 > > ret: 0 st: 2 flags:1 dts: 0.332000 pts: 0.332000 pos: 18959 > size: 271 > > ret: 0 st: 2 flags:1 ts:-0.740833 > > -ret: 0 st: 2 flags:1 dts: 0.000000 pts: 0.000000 pos: 7691 > size: 2519 > > +ret: 0 st: 2 flags:1 dts:-0.002667 pts:-0.002667 pos: 14580 > size: 39 > > ret: 0 st: 3 flags:0 ts: 2.153000 > > ret: 0 st: 3 flags:1 dts: 113.023000 pts: 113.023000 > pos:25335211 size: 47 > > ret: 0 st: 3 flags:1 ts: 1.048000 > > @@ -295,7 +295,7 @@ > > ret: 0 st: 1 flags:1 ts: 0.200833 > > ret: 0 st: 1 flags:1 dts:-0.002667 pts:-0.002667 pos: 10315 > size: 32 > > ret: 0 st: 2 flags:0 ts:-0.905000 > > -ret: 0 st: 2 flags:1 dts: 0.000000 pts: 0.000000 pos: 7691 > size: 2519 > > +ret: 0 st: 2 flags:1 dts:-0.002667 pts:-0.002667 pos: 14580 > size: 39 > > ret: 0 st: 2 flags:1 ts: 1.989167 > > ret: 0 st: 2 flags:1 dts: 1.782667 pts: 1.782667 pos: 321518 > size: 241 > > ret: 0 st: 3 flags:0 ts: 0.883000 > > Command exited with non-zero status 1 > > > > Is this correct&intended ? >=20 > Sorry I thought that this sample was part of FATE but it isn't their nor = is > it linked in the corresponding ticket. samples.ffmpeg.org/ffmpeg-bugs/trac/ticket2739/lavf_ogm_seeking_borked.ogm >=20 > It would be nice if there was a definite set of samples/tests to check > before sending patches, yes, ideally every (or most) fixed tickets would result in a new fate test It is something that someone should do as a STF task or as a volunteer. Its not something i have teh time or patience for thx [...] --=20 Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Republics decline into democracies and democracies degenerate into despotisms. -- Aristotle --S7pq8suDAU0LBjBQ Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iF0EABEKAB0WIQSf8hKLFH72cwut8TNhHseHBAsPqwUCaCkIrQAKCRBhHseHBAsP q7+xAKCEuHA0HvzEMaPtqV+MPSc5vBRD4wCgiI3D2t3JXjivbRPnv5EbKAlw6uA= =R3rA -----END PGP SIGNATURE----- --S7pq8suDAU0LBjBQ-- --===============0396432675855398868== 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". --===============0396432675855398868==--