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 = &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 == 1 && buf_size > 7) { > - if ((ret = init_get_bits8(gb, buf + 1, buf_size - 1)) < 0) > - return ret; > - > - vorbis_free(vc); > - if ((ret = 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 = AV_CHANNEL_ORDER_UNSPEC; > - avctx->ch_layout.nb_channels = vc->audio_channels; > - } else { > - av_channel_layout_copy(&avctx->ch_layout, > &ff_vorbis_ch_layouts[vc->audio_channels - 1]); > - } > - > - avctx->sample_rate = vc->audio_samplerate; > - return buf_size; > - } > - > - if (*buf == 3 && buf_size > 7) { > - av_log(avctx, AV_LOG_DEBUG, "Ignoring comment header\n"); > - return buf_size; > - } > + new_extradata = av_packet_get_side_data(avpkt, AV_PKT_DATA_NEW_EXTRADATA, > + &new_extradata_size); > > - if (*buf == 5 && buf_size > 7 && vc->channel_residues && !vc->modes) { > - if ((ret = init_get_bits8(gb, buf + 1, buf_size - 1)) < 0) > + if (new_extradata && *new_extradata == 5 && new_extradata_size > 7 && > + vc->channel_residues && !vc->modes) { > + if ((ret = init_get_bits8(gb, new_extradata + 1, new_extradata_size > - 1)) < 0) > return ret; > > if ((ret = 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/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; > } > > +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 != 30) > + return AVERROR_INVALIDDATA; > + > + p += 7; /* skip "\001vorbis" tag */ > + > + if (bytestream_get_le32(&p) != 0) /* vorbis_version */ > + return AVERROR_INVALIDDATA; > + > + channels = bytestream_get_byte(&p); > + if (st->codecpar->ch_layout.nb_channels && > + channels != 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 = channels; > + srate = bytestream_get_le32(&p); > + p += 4; // skip maximum bitrate > + st->codecpar->bit_rate = bytestream_get_le32(&p); // nominal bitrate > + p += 4; // skip minimum bitrate > + > + blocksize = bytestream_get_byte(&p); > + bs0 = blocksize & 15; > + bs1 = blocksize >> 4; > + > + if (bs0 > bs1) > + return AVERROR_INVALIDDATA; > + if (bs0 < 6 || bs1 > 13) > + return AVERROR_INVALIDDATA; > + > + if (bytestream_get_byte(&p) != 1) /* framing_flag */ > + return AVERROR_INVALIDDATA; > + > + st->codecpar->codec_type = AVMEDIA_TYPE_AUDIO; > + st->codecpar->codec_id = AV_CODEC_ID_VORBIS; > + > + if (srate > 0) { > + if (st->codecpar->sample_rate && > + srate != st->codecpar->sample_rate) { > + av_log(s, AV_LOG_ERROR, "Sample rate change is not supported\n"); > + return AVERROR_PATCHWELCOME; > + } > + > + st->codecpar->sample_rate = srate; > + avpriv_set_pts_info(st, 64, 1, srate); > + } > + > + return 1; > +} > + > static int vorbis_header(AVFormatContext *s, int idx) > { > struct ogg *ogg = s->priv_data; > @@ -300,6 +356,7 @@ static int vorbis_header(AVFormatContext *s, int idx) > struct ogg_stream *os = ogg->streams + idx; > struct oggvorbis_private *priv; > int pkt_type = os->buf[os->pstart]; > + int ret; > > if (!os->private) { > os->private = av_mallocz(sizeof(struct oggvorbis_private)); > @@ -327,56 +384,18 @@ static int vorbis_header(AVFormatContext *s, int idx) > > priv->len[pkt_type >> 1] = os->psize; > priv->packet[pkt_type >> 1] = av_memdup(os->buf + os->pstart, os->psize); > + > if (!priv->packet[pkt_type >> 1]) > return AVERROR(ENOMEM); > - if (os->buf[os->pstart] == 1) { > - const uint8_t *p = os->buf + os->pstart + 7; /* skip "\001vorbis" > tag */ > - unsigned blocksize, bs0, bs1; > - int srate; > - int channels; > - > - if (os->psize != 30) > - return AVERROR_INVALIDDATA; > - > - if (bytestream_get_le32(&p) != 0) /* vorbis_version */ > - return AVERROR_INVALIDDATA; > - > - channels = bytestream_get_byte(&p); > - if (st->codecpar->ch_layout.nb_channels && > - channels != 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 = channels; > - srate = bytestream_get_le32(&p); > - p += 4; // skip maximum bitrate > - st->codecpar->bit_rate = bytestream_get_le32(&p); // nominal bitrate > - p += 4; // skip minimum bitrate > - > - blocksize = bytestream_get_byte(&p); > - bs0 = blocksize & 15; > - bs1 = blocksize >> 4; > - > - if (bs0 > bs1) > - return AVERROR_INVALIDDATA; > - if (bs0 < 6 || bs1 > 13) > - return AVERROR_INVALIDDATA; > - > - if (bytestream_get_byte(&p) != 1) /* framing_flag */ > - return AVERROR_INVALIDDATA; > - > - st->codecpar->codec_type = AVMEDIA_TYPE_AUDIO; > - st->codecpar->codec_id = AV_CODEC_ID_VORBIS; > - > - if (srate > 0) { > - st->codecpar->sample_rate = srate; > - avpriv_set_pts_info(st, 64, 1, srate); > - } > - } else if (os->buf[os->pstart] == 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 [...] -- 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.
signature.asc
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".