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 ESMTPS id 07E4C4EB08 for ; Tue, 13 May 2025 19:23:51 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 2E33968B4C3; Tue, 13 May 2025 22:23:47 +0300 (EEST) Received: from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net [217.70.183.199]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id E266A68AD83 for ; Tue, 13 May 2025 22:23:40 +0300 (EEST) Received: by mail.gandi.net (Postfix) with ESMTPSA id 4920A43A1B for ; Tue, 13 May 2025 19:23:39 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=niedermayer.cc; s=gm1; t=1747164220; 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=3H033zJKvMsvrYQTOrAT5rR+UDcUegoymEB7qjrVUa0=; b=fSbrPrMLLa1jo46fy9a2gqMdWv6HhWFC0NnfKTqdGE5qHROlTRgverVbPikcTccntcs5LH qMjMovR25Mnn7JpkmqCJbh/YdU32vY8jl6tPtke9zAbZq8lt0VLRIPXbwPh35aSmUplWjb eoBx5swyXK4N6WSHq1X+bwrXrW6JhQfQOfPACQcoTu7JFuJB8A0nvYRxIFIdb+P2eVUCAh MEufZUNNNfpD3tlcJYdr7Ufxuf3GAf3CaiA6020xMawALry0JxaOrCE2b/444nTi8mK7OD 4l/OSTaGVQEp4UtdfaFRuX4pVKN2/mk12NJShZdXphN05GiawNKGNQ3BpFOqWA== Date: Tue, 13 May 2025 21:23:39 +0200 From: Michael Niedermayer To: FFmpeg development discussions and patches Message-ID: <20250513192339.GF29660@pb2> References: <20250509234327.71039-1-romain.beauxis@gmail.com> <20250509234327.71039-7-romain.beauxis@gmail.com> MIME-Version: 1.0 In-Reply-To: <20250509234327.71039-7-romain.beauxis@gmail.com> X-GND-State: clean X-GND-Score: -70 X-GND-Cause: gggruggvucftvghtrhhoucdtuddrgeefvddrtddtgdeftdegleegucetufdoteggodetrfdotffvucfrrhhofhhilhgvmecuifetpfffkfdpucggtfgfnhhsuhgsshgtrhhisggvnecuuegrihhlohhuthemuceftddunecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenfghrlhcuvffnffculdeftddmnecujfgurhepfffhvffukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefoihgthhgrvghlucfpihgvuggvrhhmrgihvghruceomhhitghhrggvlhesnhhivgguvghrmhgrhigvrhdrtggtqeenucggtffrrghtthgvrhhnpeeigeektdejudffjefhteegjedtgeettefggedthfejgfevhfetgeekjedtvdfhveenucfkphepgedurdeiiedrieejrdduudefnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehinhgvthepgedurdeiiedrieejrdduudefpdhhvghloheplhhotggrlhhhohhsthdpmhgrihhlfhhrohhmpehmihgthhgrvghlsehnihgvuggvrhhmrgihvghrrdgttgdpnhgspghrtghpthhtohepuddprhgtphhtthhopehffhhmphgvghdquggvvhgvlhesfhhfmhhpvghgrdhorhhg 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="===============7542173327400379137==" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: --===============7542173327400379137== Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="p8BcnzLwh3ipgLRM" Content-Disposition: inline --p8BcnzLwh3ipgLRM Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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(-) >=20 > 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 *av= ctx, 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; > =20 > ff_dlog(NULL, "packet length %d \n", buf_size); > =20 > - 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_layo= uts[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_EXT= RADATA, > + &new_extradata_size); > =20 > - if (*buf =3D=3D 5 && buf_size > 7 && vc->channel_residues && !vc->mo= des) { > - if ((ret =3D init_get_bits8(gb, buf + 1, buf_size - 1)) < 0) > + if (new_extradata && *new_extradata =3D=3D 5 && new_extradata_size >= 7 && > + vc->channel_residues && !vc->modes) { > + if ((ret =3D init_get_bits8(gb, new_extradata + 1, new_extradata= _size - 1)) < 0) > return ret; > =20 > if ((ret =3D vorbis_parse_setup_hdr(vc))) { > @@ -1816,7 +1794,6 @@ static int vorbis_decode_frame(AVCodecContext *avct= x, AVFrame *frame, > vorbis_free(vc); > return ret; > } > - return buf_size; > } > =20 > if (!vc->channel_residues || !vc->modes) { > diff --git a/libavformat/oggparsevorbis.c b/libavformat/oggparsevorbis.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; > } > =20 > +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 bitra= te > + 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; > =20 > if (!os->private) { > os->private =3D av_mallocz(sizeof(struct oggvorbis_private)); > @@ -327,56 +384,18 @@ static int vorbis_header(AVFormatContext *s, int id= x) > =20 > 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 "\001vorb= is" 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); // nominal b= itrate > - 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 ? thx [...] --=20 Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Rewriting code that is poorly written but fully understood is good. Rewriting code that one doesnt understand is a sign that one is less smart than the original author, trying to rewrite it will not make it better. --p8BcnzLwh3ipgLRM Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iF0EABEKAB0WIQSf8hKLFH72cwut8TNhHseHBAsPqwUCaCOcNAAKCRBhHseHBAsP qxVSAKCQD6POqFpXy6DBrFEA2KYCm9mROwCfYS9N6y0YrCH583JJbs8WWYIkb6Y= =2/QL -----END PGP SIGNATURE----- --p8BcnzLwh3ipgLRM-- --===============7542173327400379137== 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". --===============7542173327400379137==--