[FFmpeg-devel] [PATCH v7 0/4] Remove chained ogg stream header packets from the demuxer

[FFmpeg-devel] [PATCH v7 0/4] Remove chained ogg stream header packets from the demuxer

[Romain Beauxis]

## Changes since last revision:
* Removed invalid return statement in vorbisdec.c

Romain Beauxis (4):
  libavformat/oggdec.{c,h}: Add new_extradata, use it to pass extradata
    to the next decoded packet.
  ogg/vorbis: factor out header processing logic.
  ogg/vorbis: implement header packet skip in chained ogg bitstreams.
  libavformat/oggdec.h: Change paket function documentation to return 1
    on header packets only.

 libavcodec/vorbis_parser.h                 |  11 ++
 libavcodec/vorbisdec.c                     |  75 +++++----
 libavformat/oggdec.c                       |  11 ++
 libavformat/oggdec.h                       |   6 +-
 libavformat/oggparsevorbis.c               | 167 +++++++++++++++------
 tests/ref/fate/ogg-vorbis-chained-meta.txt |   3 -
 6 files changed, 190 insertions(+), 83 deletions(-)

-- 
2.39.5 (Apple Git-154)

_______________________________________________
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".

[FFmpeg-devel] [PATCH v7 1/4] libavformat/oggdec.{c, h}: Add new_extradata, use it to pass extradata to the next decoded packet.

[Romain Beauxis]

---
 libavformat/oggdec.c | 11 +++++++++++
 libavformat/oggdec.h |  2 ++
 2 files changed, 13 insertions(+)

diff --git a/libavformat/oggdec.c b/libavformat/oggdec.c
index 5557eb4a14..cb77cdd994 100644
--- a/libavformat/oggdec.c
+++ b/libavformat/oggdec.c
@@ -77,6 +77,7 @@ static void free_stream(AVFormatContext *s, int i)
 
     av_freep(&stream->private);
     av_freep(&stream->new_metadata);
+    av_freep(&stream->new_extradata);
 }
 
 //FIXME We could avoid some structure duplication
@@ -888,6 +889,16 @@ retry:
         os->new_metadata_size = 0;
     }
 
+    if (os->new_extradata) {
+        ret = av_packet_add_side_data(pkt, AV_PKT_DATA_NEW_EXTRADATA,
+                                      os->new_extradata, os->new_extradata_size);
+        if (ret < 0)
+            return ret;
+
+        os->new_extradata = NULL;
+        os->new_extradata_size = 0;
+    }
+
     return psize;
 }
 
diff --git a/libavformat/oggdec.h b/libavformat/oggdec.h
index bc670d0f1e..5083de646c 100644
--- a/libavformat/oggdec.h
+++ b/libavformat/oggdec.h
@@ -94,6 +94,8 @@ struct ogg_stream {
     int end_trimming; ///< set the number of packets to drop from the end
     uint8_t *new_metadata;
     size_t new_metadata_size;
+    uint8_t *new_extradata;
+    size_t new_extradata_size;
     void *private;
 };
 
-- 
2.39.5 (Apple Git-154)

_______________________________________________
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".

[FFmpeg-devel] [PATCH v7 2/4] ogg/vorbis: factor out header processing logic.

[Romain Beauxis]

---
 libavformat/oggparsevorbis.c | 104 ++++++++++++++++++++---------------
 1 file changed, 60 insertions(+), 44 deletions(-)

diff --git a/libavformat/oggparsevorbis.c b/libavformat/oggparsevorbis.c
index 9f50ab9ffc..62cc2da6de 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;
@@ -329,50 +385,10 @@ static int vorbis_header(AVFormatContext *s, int idx)
     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) {
+    if (pkt_type == 1)
+        return vorbis_parse_header(s, st, os->buf + os->pstart, os->psize);
+
+    if (pkt_type == 3) {
         if (vorbis_update_metadata(s, idx) >= 0 && priv->len[1] > 10) {
             unsigned new_len;
 
-- 
2.39.5 (Apple Git-154)

_______________________________________________
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".

[FFmpeg-devel] [PATCH v7 3/4] ogg/vorbis: implement header packet skip in chained ogg bitstreams.

[Romain Beauxis]

---
 libavcodec/vorbis_parser.h                 | 11 ++++
 libavcodec/vorbisdec.c                     | 75 +++++++++++++---------
 libavformat/oggparsevorbis.c               | 63 +++++++++++++++++-
 tests/ref/fate/ogg-vorbis-chained-meta.txt |  3 -
 4 files changed, 115 insertions(+), 37 deletions(-)

diff --git a/libavcodec/vorbis_parser.h b/libavcodec/vorbis_parser.h
index 789932ac49..b176fe536c 100644
--- a/libavcodec/vorbis_parser.h
+++ b/libavcodec/vorbis_parser.h
@@ -30,6 +30,17 @@
 
 typedef struct AVVorbisParseContext AVVorbisParseContext;
 
+/**
+ * Used by the vorbis parser to pass new chained stream headers
+ * as extradata.
+ */
+typedef struct vorbis_new_extradata {
+    uint8_t *header;
+    size_t   header_size;
+    uint8_t *setup;
+    size_t   setup_size;
+} vorbis_new_extradata;
+
 /**
  * Allocate and initialize the Vorbis parser using headers in the extradata.
  */
diff --git a/libavcodec/vorbisdec.c b/libavcodec/vorbisdec.c
index adbd726183..6c5d3fa50f 100644
--- a/libavcodec/vorbisdec.c
+++ b/libavcodec/vorbisdec.c
@@ -43,6 +43,7 @@
 #include "vorbis.h"
 #include "vorbisdsp.h"
 #include "vorbis_data.h"
+#include "vorbis_parser.h"
 #include "xiph.h"
 
 #define V_NB_BITS 8
@@ -1778,47 +1779,59 @@ static int vorbis_decode_frame(AVCodecContext *avctx, AVFrame *frame,
     GetBitContext *gb = &vc->gb;
     float *channel_ptrs[255];
     int i, len, ret;
+    size_t new_extradata_size;
+    vorbis_new_extradata *new_extradata;
+    const uint8_t *header;
+    const uint8_t *setup;
 
     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;
+    new_extradata = (vorbis_new_extradata *)av_packet_get_side_data(
+        avpkt, AV_PKT_DATA_NEW_EXTRADATA, &new_extradata_size);
 
-        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;
-        }
+    if (new_extradata) {
+        header = new_extradata->header;
+        setup = new_extradata->setup;
 
-        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]);
-        }
+        if (header && *header == 1 && new_extradata->header_size > 7) {
+            if ((ret = init_get_bits8(
+                            gb, header + 1,
+                            new_extradata->header_size - 1)) < 0)
+                return ret;
 
-        avctx->sample_rate = vc->audio_samplerate;
-        return buf_size;
-    }
+            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;
+            }
 
-    if (*buf == 3 && buf_size > 7) {
-        av_log(avctx, AV_LOG_DEBUG, "Ignoring comment header\n");
-        return buf_size;
-    }
+            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]);
+            }
 
-    if (*buf == 5 && buf_size > 7 && vc->channel_residues && !vc->modes) {
-        if ((ret = init_get_bits8(gb, buf + 1, buf_size - 1)) < 0)
-            return ret;
+            avctx->sample_rate = vc->audio_samplerate;
+        }
 
-        if ((ret = vorbis_parse_setup_hdr(vc))) {
-            av_log(avctx, AV_LOG_ERROR, "Setup header corrupt.\n");
-            vorbis_free(vc);
-            return ret;
+        if (setup && *setup == 5 && new_extradata->setup_size > 7 &&
+            vc->channel_residues && !vc->modes) {
+            if ((ret = init_get_bits8(
+                           gb, setup + 1,
+                           new_extradata->setup_size - 1)) < 0)
+                return ret;
+
+            if ((ret = vorbis_parse_setup_hdr(vc))) {
+                av_log(avctx, AV_LOG_ERROR, "Setup header corrupt.\n");
+                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 62cc2da6de..ee2e01f468 100644
--- a/libavformat/oggparsevorbis.c
+++ b/libavformat/oggparsevorbis.c
@@ -255,12 +255,19 @@ static void vorbis_cleanup(AVFormatContext *s, int idx)
     struct ogg *ogg = s->priv_data;
     struct ogg_stream *os = ogg->streams + idx;
     struct oggvorbis_private *priv = os->private;
+    vorbis_new_extradata *new_extradata;
     int i;
     if (os->private) {
         av_vorbis_parse_free(&priv->vp);
         for (i = 0; i < 3; i++)
             av_freep(&priv->packet[i]);
     }
+
+    if (os->new_extradata) {
+        new_extradata = (vorbis_new_extradata *)os->new_extradata;
+        av_freep(&new_extradata->header);
+        av_freep(&new_extradata->setup);
+    }
 }
 
 static int vorbis_update_metadata(AVFormatContext *s, int idx)
@@ -433,7 +440,10 @@ static int vorbis_packet(AVFormatContext *s, int idx)
     struct ogg *ogg = s->priv_data;
     struct ogg_stream *os = ogg->streams + idx;
     struct oggvorbis_private *priv = os->private;
+    vorbis_new_extradata *new_extradata;
     int duration, flags = 0;
+    int skip_packet = 0;
+    int ret;
 
     if (!priv->vp)
         return AVERROR_INVALIDDATA;
@@ -496,10 +506,57 @@ static int vorbis_packet(AVFormatContext *s, int idx)
         if (duration < 0) {
             os->pflags |= AV_PKT_FLAG_CORRUPT;
             return 0;
-        } else if (flags & VORBIS_FLAG_COMMENT) {
-            vorbis_update_metadata(s, idx);
+        }
+
+        if (flags & VORBIS_FLAG_HEADER) {
+            ret = vorbis_parse_header(s, s->streams[idx], os->buf + os->pstart, os->psize);
+            if (ret < 0)
+                return ret;
+
+            ret = av_reallocp(&os->new_extradata, sizeof(vorbis_new_extradata));
+            if (ret < 0)
+                return ret;
+
+            os->new_extradata_size = sizeof(vorbis_new_extradata);
+            new_extradata = (vorbis_new_extradata *)os->new_extradata;
+
+            ret = av_reallocp(&new_extradata->header, os->psize);
+            if (ret < 0)
+                return ret;
+
+            memcpy(new_extradata->header,  os->buf + os->pstart, os->psize);
+            new_extradata->header_size = os->psize;
+
+            skip_packet = 1;
+        }
+
+        if (flags & VORBIS_FLAG_COMMENT) {
+            ret = vorbis_update_metadata(s, idx);
+            if (ret < 0)
+                return ret;
+
             flags = 0;
+            skip_packet = 1;
         }
+
+        if (flags & VORBIS_FLAG_SETUP) {
+            ret = av_reallocp(&os->new_extradata, sizeof(vorbis_new_extradata));
+            if (ret < 0)
+                return ret;
+
+            os->new_extradata_size = sizeof(vorbis_new_extradata);
+            new_extradata = (vorbis_new_extradata *)os->new_extradata;
+
+            ret = av_reallocp(&new_extradata->setup, os->psize);
+            if (ret < 0)
+                return ret;
+
+            memcpy(new_extradata->setup, os->buf + os->pstart, os->psize);
+            new_extradata->setup_size = os->psize;
+
+            skip_packet = 1;
+        }
+
         os->pduration = duration;
     }
 
@@ -521,7 +578,7 @@ static int vorbis_packet(AVFormatContext *s, int idx)
         priv->final_duration += os->pduration;
     }
 
-    return 0;
+    return skip_packet;
 }
 
 const struct ogg_codec ff_vorbis_codec = {
diff --git a/tests/ref/fate/ogg-vorbis-chained-meta.txt b/tests/ref/fate/ogg-vorbis-chained-meta.txt
index b7a97c90e2..1206f86c1f 100644
--- a/tests/ref/fate/ogg-vorbis-chained-meta.txt
+++ b/tests/ref/fate/ogg-vorbis-chained-meta.txt
@@ -6,10 +6,7 @@ Stream ID: 0, frame PTS: 128, metadata: N/A
 Stream ID: 0, packet PTS: 704, packet DTS: 704
 Stream ID: 0, frame PTS: 704, metadata: N/A
 Stream ID: 0, packet PTS: 0, packet DTS: 0
-Stream ID: 0, packet PTS: 0, packet DTS: 0
 Stream ID: 0, new metadata: encoder=Lavc61.19.100 libvorbis:title=Second Stream
-Stream ID: 0, packet PTS: 0, packet DTS: 0
-Stream ID: 0, packet PTS: 0, packet DTS: 0
 Stream ID: 0, frame PTS: 0, metadata: N/A
 Stream ID: 0, packet PTS: 128, packet DTS: 128
 Stream ID: 0, frame PTS: 128, metadata: N/A
-- 
2.39.5 (Apple Git-154)

_______________________________________________
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".

[FFmpeg-devel] [PATCH v7 4/4] libavformat/oggdec.h: Change paket function documentation to return 1 on header packets only.

[Romain Beauxis]

---
 libavformat/oggdec.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavformat/oggdec.h b/libavformat/oggdec.h
index 5083de646c..c15fbe738e 100644
--- a/libavformat/oggdec.h
+++ b/libavformat/oggdec.h
@@ -42,8 +42,8 @@ struct ogg_codec {
      * Attempt to process a packet as a data packet
      * @return < 0 (AVERROR) code or -1 on error
      *         == 0 if the packet was a regular data packet.
-     *         == 0 or 1 if the packet was a header from a chained bitstream.
-     *           (1 will cause the packet to be skiped in calling code (ogg_packet())
+     *         == 1 if the packet was a header from a chained bitstream.
+     *            This will cause the packet to be skiped in calling code (ogg_packet()
      */
     int (*packet)(AVFormatContext *, int);
     /**
-- 
2.39.5 (Apple Git-154)

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v7 3/4] ogg/vorbis: implement header packet skip in chained ogg bitstreams.

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 2286 bytes --]

Hi Romain

On Wed, May 21, 2025 at 05:05:37PM -0500, Romain Beauxis wrote:
> ---
>  libavcodec/vorbis_parser.h                 | 11 ++++
>  libavcodec/vorbisdec.c                     | 75 +++++++++++++---------
>  libavformat/oggparsevorbis.c               | 63 +++++++++++++++++-
>  tests/ref/fate/ogg-vorbis-chained-meta.txt |  3 -
>  4 files changed, 115 insertions(+), 37 deletions(-)

make -j32 libavformat/tests/seek && libavformat/tests/seek ~/tickets/2739/lavf_ogm_seeking_borked.ogm
CC	libavcodec/libvorbisenc.o
CC	libavcodec/version.o
CC	libavcodec/vorbis_parser.o
CC	libavcodec/vorbisdec.o
CC	libswresample/version.o
CC	libavutil/version.o
CC	libavformat/oggparsevorbis.o
CC	libavformat/version.o
AR	libswresample/libswresample.a
AR	libavutil/libavutil.a
AR	libavcodec/libavcodec.a
AR	libavformat/libavformat.a
LD	libavformat/tests/seek
[ogg @ 0x558349c4f3c0] Headers mismatch for stream 1: expected 3 received 2.
[ogg @ 0x558349c4f3c0] Headers mismatch for stream 2: expected 3 received 2.
ret: 0         st: 0 flags:1 dts:-0.083417 pts: 0.000000 pos:    887 size:  3347
ret: 0         st:-1 flags:0  ts:-1.000000
ret: 0         st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos:    887 size:  3347
ret: 0         st:-1 flags:1  ts: 1.894167
ret: 0         st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos:    887 size:  3347
ret: 0         st: 0 flags:0  ts: 0.792458
ret: 0         st: 1 flags:1 dts: 12.481333 pts: 12.481333 pos:2855209 size:   269
ret: 0         st: 0 flags:1  ts:-0.333666
ret: 0         st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos:    887 size:  3347
ret: 0         st: 1 flags:0  ts: 2.576667
ret: 0         st: 1 flags:1 dts: 2.785333 pts: 2.785333 pos: 559697 size:   255
ret: 0         st: 1 flags:1  ts: 1.470833
ret: 0         st: 1 flags:1 dts: 1.164000 pts: 1.164000 pos: 192278 size:   275
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
Segmentation fault (core dumped)

on samples server: ffmpeg-bugs/trac/ticket2739/lavf_ogm_seeking_borked.ogm

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The worst form of inequality is to try to make unequal things equal.
-- Aristotle

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v7 3/4] ogg/vorbis: implement header packet skip in chained ogg bitstreams.

[Romain Beauxis]

Le ven. 23 mai 2025 à 19:40, Michael Niedermayer <michael@niedermayer.cc> a
écrit :
>
> Hi Romain
>
> On Wed, May 21, 2025 at 05:05:37PM -0500, Romain Beauxis wrote:
> > ---
> >  libavcodec/vorbis_parser.h                 | 11 ++++
> >  libavcodec/vorbisdec.c                     | 75 +++++++++++++---------
> >  libavformat/oggparsevorbis.c               | 63 +++++++++++++++++-
> >  tests/ref/fate/ogg-vorbis-chained-meta.txt |  3 -
> >  4 files changed, 115 insertions(+), 37 deletions(-)
>
> make -j32 libavformat/tests/seek && libavformat/tests/seek
~/tickets/2739/lavf_ogm_seeking_borked.ogm
> CC      libavcodec/libvorbisenc.o
> CC      libavcodec/version.o
> CC      libavcodec/vorbis_parser.o
> CC      libavcodec/vorbisdec.o
> CC      libswresample/version.o
> CC      libavutil/version.o
> CC      libavformat/oggparsevorbis.o
> CC      libavformat/version.o
> AR      libswresample/libswresample.a
> AR      libavutil/libavutil.a
> AR      libavcodec/libavcodec.a
> AR      libavformat/libavformat.a
> LD      libavformat/tests/seek
> [ogg @ 0x558349c4f3c0] Headers mismatch for stream 1: expected 3 received
2.
> [ogg @ 0x558349c4f3c0] Headers mismatch for stream 2: expected 3 received
2.
> ret: 0         st: 0 flags:1 dts:-0.083417 pts: 0.000000 pos:    887
size:  3347
> ret: 0         st:-1 flags:0  ts:-1.000000
> ret: 0         st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos:    887
size:  3347
> ret: 0         st:-1 flags:1  ts: 1.894167
> ret: 0         st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos:    887
size:  3347
> ret: 0         st: 0 flags:0  ts: 0.792458
> ret: 0         st: 1 flags:1 dts: 12.481333 pts: 12.481333 pos:2855209
size:   269
> ret: 0         st: 0 flags:1  ts:-0.333666
> ret: 0         st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos:    887
size:  3347
> ret: 0         st: 1 flags:0  ts: 2.576667
> ret: 0         st: 1 flags:1 dts: 2.785333 pts: 2.785333 pos: 559697
size:   255
> ret: 0         st: 1 flags:1  ts: 1.470833
> ret: 0         st: 1 flags:1 dts: 1.164000 pts: 1.164000 pos: 192278
size:   275
> 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
> Segmentation fault (core dumped)
>
> on samples server: ffmpeg-bugs/trac/ticket2739/lavf_ogm_seeking_borked.ogm

Thanks for pointing this one out. I got tricked by the permissive memory
model on macos where it wouldn't segfault but was able to reproduce and fix
on a debian machine.

I'm gonna send an updated patchset. I've also added a FATE test for this
sample, given it's importance during this review, it seems natural to
include it.

-- Romain

> thx
>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> The worst form of inequality is to try to make unequal things equal.
> -- Aristotle
> _______________________________________________
> 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".
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v7 3/4] ogg/vorbis: implement header packet skip in chained ogg bitstreams.

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 479 bytes --]

Hi Romain

On Sat, May 24, 2025 at 01:08:06PM -0500, Romain Beauxis wrote:
[...]
> I'm gonna send an updated patchset. I've also added a FATE test for this
> sample, given it's importance during this review, it seems natural to
> include it.

yes, tests are important
thank you

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

It is a danger to trust the dream we wish for rather than
the science we have, -- Dr. Kenneth Brown

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".