Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* [FFmpeg-devel] [PATCH v2] ogg/vorbis: implement header packet skip in chained ogg bitstreams.
@ 2025-06-04 16:58 Romain Beauxis
  2025-06-10 18:04 ` Romain Beauxis
  2025-06-14 22:57 ` Michael Niedermayer
  0 siblings, 2 replies; 15+ messages in thread
From: Romain Beauxis @ 2025-06-04 16:58 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Romain Beauxis, andreas.rheinhardt

This is a redo of 574f634e49847e2225ee50013afebf0de03ef013 using a flat
memory storage for the extradata.

PR review comments addressed:
* Use flat memory bytestream
* Re-use existing xiph extradata layout

---
 libavcodec/vorbisdec.c                     | 42 ++++++++---
 libavformat/oggparsevorbis.c               | 83 +++++++++++++++++++++-
 tests/ref/fate/ogg-vorbis-chained-meta.txt |  3 -
 3 files changed, 114 insertions(+), 14 deletions(-)

diff --git a/libavcodec/vorbisdec.c b/libavcodec/vorbisdec.c
index adbd726183..84879462a1 100644
--- a/libavcodec/vorbisdec.c
+++ b/libavcodec/vorbisdec.c
@@ -1778,11 +1778,40 @@ 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;
+    const uint8_t *new_extradata;
+    const uint8_t *header_start[3];
+    int header_len[3] = {0, 0, 0};
+    const uint8_t *header;
+    int header_size = 0;
+    const uint8_t *comment;
+    int comment_size = 0;
+    const uint8_t *setup;
+    int setup_size = 0;
 
     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)
+    new_extradata = av_packet_get_side_data(avpkt, AV_PKT_DATA_NEW_EXTRADATA,
+                        &new_extradata_size);
+
+    if (new_extradata) {
+        ret = avpriv_split_xiph_headers(new_extradata, new_extradata_size,
+                                        30, header_start, header_len);
+        if (ret < 0)
+            return ret;
+
+        header = header_start[0];
+        header_size = header_len[0];
+
+        comment = header_start[1];
+        comment_size = header_len[1];
+
+        setup = header_start[2];
+        setup_size = header_len[2];
+    }
+
+    if (header_size > 7 && *header == 1) {
+        if ((ret = init_get_bits8(gb, header + 1, header_size - 1)) < 0)
             return ret;
 
         vorbis_free(vc);
@@ -1801,16 +1830,14 @@ static int vorbis_decode_frame(AVCodecContext *avctx, AVFrame *frame,
         }
 
         avctx->sample_rate = vc->audio_samplerate;
-        return buf_size;
     }
 
-    if (*buf == 3 && buf_size > 7) {
+    if (comment_size > 7 && *comment == 3) {
         av_log(avctx, AV_LOG_DEBUG, "Ignoring comment header\n");
-        return buf_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 (setup_size > 7 && *setup == 5 && vc->channel_residues && !vc->modes) {
+        if ((ret = init_get_bits8(gb, setup + 1, setup_size - 1)) < 0)
             return ret;
 
         if ((ret = vorbis_parse_setup_hdr(vc))) {
@@ -1818,7 +1845,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 62cc2da6de..7859ec5a51 100644
--- a/libavformat/oggparsevorbis.c
+++ b/libavformat/oggparsevorbis.c
@@ -215,6 +215,12 @@ struct oggvorbis_private {
     AVVorbisParseContext *vp;
     int64_t final_pts;
     int final_duration;
+    uint8_t *header;
+    int header_size;
+    uint8_t *comment;
+    int comment_size;
+    uint8_t *setup;
+    int setup_size;
 };
 
 static int fixup_vorbis_headers(AVFormatContext *as,
@@ -260,6 +266,10 @@ static void vorbis_cleanup(AVFormatContext *s, int idx)
         av_vorbis_parse_free(&priv->vp);
         for (i = 0; i < 3; i++)
             av_freep(&priv->packet[i]);
+
+        av_freep(&priv->header);
+        av_freep(&priv->comment);
+        av_freep(&priv->setup);
     }
 }
 
@@ -434,6 +444,9 @@ static int vorbis_packet(AVFormatContext *s, int idx)
     struct ogg_stream *os = ogg->streams + idx;
     struct oggvorbis_private *priv = os->private;
     int duration, flags = 0;
+    int skip_packet = 0;
+    int ret, new_extradata_size;
+    PutByteContext pb;
 
     if (!priv->vp)
         return AVERROR_INVALIDDATA;
@@ -496,10 +509,50 @@ 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(&priv->header, os->psize);
+            if (ret < 0)
+                return ret;
+
+            memcpy(priv->header, os->buf + os->pstart, os->psize);
+            priv->header_size = os->psize;
+
+            skip_packet = 1;
+        }
+
+        if (flags & VORBIS_FLAG_COMMENT) {
+            ret = vorbis_update_metadata(s, idx);
+            if (ret < 0)
+                return ret;
+
+            ret = av_reallocp(&priv->comment, os->psize);
+            if (ret < 0)
+                return ret;
+
+            memcpy(priv->comment, os->buf + os->pstart, os->psize);
+            priv->comment_size = os->psize;
+
             flags = 0;
+            skip_packet = 1;
+        }
+
+        if (flags & VORBIS_FLAG_SETUP) {
+            ret = av_reallocp(&priv->setup, os->psize);
+            if (ret < 0)
+                return ret;
+
+            memcpy(priv->setup, os->buf + os->pstart, os->psize);
+            priv->setup_size = os->psize;
+
+            skip_packet = 1;
         }
+
         os->pduration = duration;
     }
 
@@ -521,7 +574,31 @@ static int vorbis_packet(AVFormatContext *s, int idx)
         priv->final_duration += os->pduration;
     }
 
-    return 0;
+    if (priv->header && priv->comment && priv->setup) {
+        new_extradata_size = priv->header_size + priv->comment_size + priv->setup_size + 6;
+
+        ret = av_reallocp(&os->new_extradata, new_extradata_size);
+        if (ret < 0)
+            return ret;
+
+        os->new_extradata_size = new_extradata_size;
+        bytestream2_init_writer(&pb, os->new_extradata, new_extradata_size);
+        bytestream2_put_be16(&pb, priv->header_size);
+        bytestream2_put_buffer(&pb, priv->header, priv->header_size);
+        bytestream2_put_be16(&pb, priv->comment_size);
+        bytestream2_put_buffer(&pb, priv->comment, priv->comment_size);
+        bytestream2_put_be16(&pb, priv->setup_size);
+        bytestream2_put_buffer(&pb, priv->setup, priv->setup_size);
+
+        av_freep(&priv->header);
+        priv->header_size = 0;
+        av_freep(&priv->comment);
+        priv->comment_size = 0;
+        av_freep(&priv->setup);
+        priv->comment_size = 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".

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [FFmpeg-devel] [PATCH v2] ogg/vorbis: implement header packet skip in chained ogg bitstreams.
  2025-06-04 16:58 [FFmpeg-devel] [PATCH v2] ogg/vorbis: implement header packet skip in chained ogg bitstreams Romain Beauxis
@ 2025-06-10 18:04 ` Romain Beauxis
  2025-06-12 11:35   ` Michael Niedermayer
  2025-06-14 22:57 ` Michael Niedermayer
  1 sibling, 1 reply; 15+ messages in thread
From: Romain Beauxis @ 2025-06-10 18:04 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: andreas.rheinhardt

Hi all,


Le mer. 4 juin 2025 à 11:58, Romain Beauxis <romain.beauxis@gmail.com> a écrit :
>
> This is a redo of 574f634e49847e2225ee50013afebf0de03ef013 using a flat
> memory storage for the extradata.
>
> PR review comments addressed:
> * Use flat memory bytestream
> * Re-use existing xiph extradata layout

Is there any interest in reviewing this patch?

It's holding the second series that fixes chained ogg stream metadata parsing.

Better support for chained ogg streams would really make life easier
for a bunch of ffmpeg and ffmpeg libraries users.

Thanks,
-- Romain

> ---
>  libavcodec/vorbisdec.c                     | 42 ++++++++---
>  libavformat/oggparsevorbis.c               | 83 +++++++++++++++++++++-
>  tests/ref/fate/ogg-vorbis-chained-meta.txt |  3 -
>  3 files changed, 114 insertions(+), 14 deletions(-)
>
> diff --git a/libavcodec/vorbisdec.c b/libavcodec/vorbisdec.c
> index adbd726183..84879462a1 100644
> --- a/libavcodec/vorbisdec.c
> +++ b/libavcodec/vorbisdec.c
> @@ -1778,11 +1778,40 @@ 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;
> +    const uint8_t *new_extradata;
> +    const uint8_t *header_start[3];
> +    int header_len[3] = {0, 0, 0};
> +    const uint8_t *header;
> +    int header_size = 0;
> +    const uint8_t *comment;
> +    int comment_size = 0;
> +    const uint8_t *setup;
> +    int setup_size = 0;
>
>      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)
> +    new_extradata = av_packet_get_side_data(avpkt, AV_PKT_DATA_NEW_EXTRADATA,
> +                        &new_extradata_size);
> +
> +    if (new_extradata) {
> +        ret = avpriv_split_xiph_headers(new_extradata, new_extradata_size,
> +                                        30, header_start, header_len);
> +        if (ret < 0)
> +            return ret;
> +
> +        header = header_start[0];
> +        header_size = header_len[0];
> +
> +        comment = header_start[1];
> +        comment_size = header_len[1];
> +
> +        setup = header_start[2];
> +        setup_size = header_len[2];
> +    }
> +
> +    if (header_size > 7 && *header == 1) {
> +        if ((ret = init_get_bits8(gb, header + 1, header_size - 1)) < 0)
>              return ret;
>
>          vorbis_free(vc);
> @@ -1801,16 +1830,14 @@ static int vorbis_decode_frame(AVCodecContext *avctx, AVFrame *frame,
>          }
>
>          avctx->sample_rate = vc->audio_samplerate;
> -        return buf_size;
>      }
>
> -    if (*buf == 3 && buf_size > 7) {
> +    if (comment_size > 7 && *comment == 3) {
>          av_log(avctx, AV_LOG_DEBUG, "Ignoring comment header\n");
> -        return buf_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 (setup_size > 7 && *setup == 5 && vc->channel_residues && !vc->modes) {
> +        if ((ret = init_get_bits8(gb, setup + 1, setup_size - 1)) < 0)
>              return ret;
>
>          if ((ret = vorbis_parse_setup_hdr(vc))) {
> @@ -1818,7 +1845,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 62cc2da6de..7859ec5a51 100644
> --- a/libavformat/oggparsevorbis.c
> +++ b/libavformat/oggparsevorbis.c
> @@ -215,6 +215,12 @@ struct oggvorbis_private {
>      AVVorbisParseContext *vp;
>      int64_t final_pts;
>      int final_duration;
> +    uint8_t *header;
> +    int header_size;
> +    uint8_t *comment;
> +    int comment_size;
> +    uint8_t *setup;
> +    int setup_size;
>  };
>
>  static int fixup_vorbis_headers(AVFormatContext *as,
> @@ -260,6 +266,10 @@ static void vorbis_cleanup(AVFormatContext *s, int idx)
>          av_vorbis_parse_free(&priv->vp);
>          for (i = 0; i < 3; i++)
>              av_freep(&priv->packet[i]);
> +
> +        av_freep(&priv->header);
> +        av_freep(&priv->comment);
> +        av_freep(&priv->setup);
>      }
>  }
>
> @@ -434,6 +444,9 @@ static int vorbis_packet(AVFormatContext *s, int idx)
>      struct ogg_stream *os = ogg->streams + idx;
>      struct oggvorbis_private *priv = os->private;
>      int duration, flags = 0;
> +    int skip_packet = 0;
> +    int ret, new_extradata_size;
> +    PutByteContext pb;
>
>      if (!priv->vp)
>          return AVERROR_INVALIDDATA;
> @@ -496,10 +509,50 @@ 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(&priv->header, os->psize);
> +            if (ret < 0)
> +                return ret;
> +
> +            memcpy(priv->header, os->buf + os->pstart, os->psize);
> +            priv->header_size = os->psize;
> +
> +            skip_packet = 1;
> +        }
> +
> +        if (flags & VORBIS_FLAG_COMMENT) {
> +            ret = vorbis_update_metadata(s, idx);
> +            if (ret < 0)
> +                return ret;
> +
> +            ret = av_reallocp(&priv->comment, os->psize);
> +            if (ret < 0)
> +                return ret;
> +
> +            memcpy(priv->comment, os->buf + os->pstart, os->psize);
> +            priv->comment_size = os->psize;
> +
>              flags = 0;
> +            skip_packet = 1;
> +        }
> +
> +        if (flags & VORBIS_FLAG_SETUP) {
> +            ret = av_reallocp(&priv->setup, os->psize);
> +            if (ret < 0)
> +                return ret;
> +
> +            memcpy(priv->setup, os->buf + os->pstart, os->psize);
> +            priv->setup_size = os->psize;
> +
> +            skip_packet = 1;
>          }
> +
>          os->pduration = duration;
>      }
>
> @@ -521,7 +574,31 @@ static int vorbis_packet(AVFormatContext *s, int idx)
>          priv->final_duration += os->pduration;
>      }
>
> -    return 0;
> +    if (priv->header && priv->comment && priv->setup) {
> +        new_extradata_size = priv->header_size + priv->comment_size + priv->setup_size + 6;
> +
> +        ret = av_reallocp(&os->new_extradata, new_extradata_size);
> +        if (ret < 0)
> +            return ret;
> +
> +        os->new_extradata_size = new_extradata_size;
> +        bytestream2_init_writer(&pb, os->new_extradata, new_extradata_size);
> +        bytestream2_put_be16(&pb, priv->header_size);
> +        bytestream2_put_buffer(&pb, priv->header, priv->header_size);
> +        bytestream2_put_be16(&pb, priv->comment_size);
> +        bytestream2_put_buffer(&pb, priv->comment, priv->comment_size);
> +        bytestream2_put_be16(&pb, priv->setup_size);
> +        bytestream2_put_buffer(&pb, priv->setup, priv->setup_size);
> +
> +        av_freep(&priv->header);
> +        priv->header_size = 0;
> +        av_freep(&priv->comment);
> +        priv->comment_size = 0;
> +        av_freep(&priv->setup);
> +        priv->comment_size = 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".

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [FFmpeg-devel] [PATCH v2] ogg/vorbis: implement header packet skip in chained ogg bitstreams.
  2025-06-10 18:04 ` Romain Beauxis
@ 2025-06-12 11:35   ` Michael Niedermayer
  2025-06-14 10:39     ` Romain Beauxis
  0 siblings, 1 reply; 15+ messages in thread
From: Michael Niedermayer @ 2025-06-12 11:35 UTC (permalink / raw)
  To: FFmpeg development discussions and patches


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

Hi Romain

On Tue, Jun 10, 2025 at 01:04:35PM -0500, Romain Beauxis wrote:
> Hi all,
> 
> 
> Le mer. 4 juin 2025 à 11:58, Romain Beauxis <romain.beauxis@gmail.com> a écrit :
> >
> > This is a redo of 574f634e49847e2225ee50013afebf0de03ef013 using a flat
> > memory storage for the extradata.
> >
> > PR review comments addressed:
> > * Use flat memory bytestream
> > * Re-use existing xiph extradata layout
> 
> Is there any interest in reviewing this patch?

maybe you can help review other peoples patches while you wait for someone
to review yours (if everyone does that then reviews should overall occur
quicker)
(i do have a backlog of things atm so i dont think i should add this one
 to my todo)

thx

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

Asymptotically faster algorithms should always be preferred if you have
asymptotical amounts of data

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [FFmpeg-devel] [PATCH v2] ogg/vorbis: implement header packet skip in chained ogg bitstreams.
  2025-06-12 11:35   ` Michael Niedermayer
@ 2025-06-14 10:39     ` Romain Beauxis
  0 siblings, 0 replies; 15+ messages in thread
From: Romain Beauxis @ 2025-06-14 10:39 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

Le jeu. 12 juin 2025 à 13:35, Michael Niedermayer <michael@niedermayer.cc>
a écrit :
>
> Hi Romain

Hi,

> On Tue, Jun 10, 2025 at 01:04:35PM -0500, Romain Beauxis wrote:
> > Hi all,
> >
> >
> > Le mer. 4 juin 2025 à 11:58, Romain Beauxis <romain.beauxis@gmail.com>
a écrit :
> > >
> > > This is a redo of 574f634e49847e2225ee50013afebf0de03ef013 using a
flat
> > > memory storage for the extradata.
> > >
> > > PR review comments addressed:
> > > * Use flat memory bytestream
> > > * Re-use existing xiph extradata layout
> >
> > Is there any interest in reviewing this patch?
>
> maybe you can help review other peoples patches while you wait for someone
> to review yours (if everyone does that then reviews should overall occur
> quicker)
> (i do have a backlog of things atm so i dont think i should add this one
>  to my todo)

I do appreciate the invite to contribute to reviews and will gladly do so.

I understand how the project needs to balance load and how it's nice to
contribute both ways.

However, I'm still learning the ropes of this code base (and this patch
proves it).

It seems awkward to make this transactional, there's a discrepancy in what
I can provide in terms of senior review and this patch is rather small and
implements exactly what I was asked to do after multiple consultations.

I was hoping it would be easy to review and merge.

Thanks,
-- Romain
_______________________________________________
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".

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [FFmpeg-devel] [PATCH v2] ogg/vorbis: implement header packet skip in chained ogg bitstreams.
  2025-06-04 16:58 [FFmpeg-devel] [PATCH v2] ogg/vorbis: implement header packet skip in chained ogg bitstreams Romain Beauxis
  2025-06-10 18:04 ` Romain Beauxis
@ 2025-06-14 22:57 ` Michael Niedermayer
  2025-06-21  8:45   ` Romain Beauxis
  1 sibling, 1 reply; 15+ messages in thread
From: Michael Niedermayer @ 2025-06-14 22:57 UTC (permalink / raw)
  To: FFmpeg development discussions and patches


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

On Wed, Jun 04, 2025 at 11:58:52AM -0500, Romain Beauxis wrote:
> This is a redo of 574f634e49847e2225ee50013afebf0de03ef013 using a flat
> memory storage for the extradata.
> 
> PR review comments addressed:
> * Use flat memory bytestream
> * Re-use existing xiph extradata layout
> 
> ---

>  libavcodec/vorbisdec.c                     | 42 ++++++++---
>  libavformat/oggparsevorbis.c               | 83 +++++++++++++++++++++-

patches that change both libraries at the same time are suspect

if one depends on changes in the other it needs
minor API version bump and seperate patches so extension of
API and use of it are properly tracked and testable

have not reveiwed the rest of the patch

thx

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

The real ebay dictionary, page 1
"Used only once"    - "Some unspecified defect prevented a second use"
"In good condition" - "Can be repaird by experienced expert"
"As is" - "You wouldnt want it even if you were payed for it, if you knew ..."

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [FFmpeg-devel] [PATCH v2] ogg/vorbis: implement header packet skip in chained ogg bitstreams.
  2025-06-14 22:57 ` Michael Niedermayer
@ 2025-06-21  8:45   ` Romain Beauxis
  2025-06-21 21:59     ` Michael Niedermayer
  0 siblings, 1 reply; 15+ messages in thread
From: Romain Beauxis @ 2025-06-21  8:45 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

Le dim. 15 juin 2025 à 00:57, Michael Niedermayer
<michael@niedermayer.cc> a écrit :
>
> On Wed, Jun 04, 2025 at 11:58:52AM -0500, Romain Beauxis wrote:
> > This is a redo of 574f634e49847e2225ee50013afebf0de03ef013 using a flat
> > memory storage for the extradata.
> >
> > PR review comments addressed:
> > * Use flat memory bytestream
> > * Re-use existing xiph extradata layout
> >
> > ---
>
> >  libavcodec/vorbisdec.c                     | 42 ++++++++---
> >  libavformat/oggparsevorbis.c               | 83 +++++++++++++++++++++-
>
> patches that change both libraries at the same time are suspect
>
> if one depends on changes in the other it needs
> minor API version bump and seperate patches so extension of
> API and use of it are properly tracked and testable

If I remember well, according to Andreas Rheinhardt there's no need
for an API bump here since the patch is re-using existing extradata
bitstream structures.

> have not reveiwed the rest of the patch
>
> thx
>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> The real ebay dictionary, page 1
> "Used only once"    - "Some unspecified defect prevented a second use"
> "In good condition" - "Can be repaird by experienced expert"
> "As is" - "You wouldnt want it even if you were payed for it, if you knew ..."
> _______________________________________________
> 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".

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [FFmpeg-devel] [PATCH v2] ogg/vorbis: implement header packet skip in chained ogg bitstreams.
  2025-06-21  8:45   ` Romain Beauxis
@ 2025-06-21 21:59     ` Michael Niedermayer
  2025-07-23 19:06       ` Romain Beauxis
  0 siblings, 1 reply; 15+ messages in thread
From: Michael Niedermayer @ 2025-06-21 21:59 UTC (permalink / raw)
  To: FFmpeg development discussions and patches


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

On Sat, Jun 21, 2025 at 10:45:32AM +0200, Romain Beauxis wrote:
> Le dim. 15 juin 2025 à 00:57, Michael Niedermayer
> <michael@niedermayer.cc> a écrit :
> >
> > On Wed, Jun 04, 2025 at 11:58:52AM -0500, Romain Beauxis wrote:
> > > This is a redo of 574f634e49847e2225ee50013afebf0de03ef013 using a flat
> > > memory storage for the extradata.
> > >
> > > PR review comments addressed:
> > > * Use flat memory bytestream
> > > * Re-use existing xiph extradata layout
> > >
> > > ---
> >
> > >  libavcodec/vorbisdec.c                     | 42 ++++++++---
> > >  libavformat/oggparsevorbis.c               | 83 +++++++++++++++++++++-
> >
> > patches that change both libraries at the same time are suspect
> >
> > if one depends on changes in the other it needs
> > minor API version bump and seperate patches so extension of
> > API and use of it are properly tracked and testable
> 
> If I remember well, according to Andreas Rheinhardt there's no need
> for an API bump here since the patch is re-using existing extradata
> bitstream structures.

If there is really no API extension then the micro versions should be bumped
so a user knows if the specific version he uses has teh fix.
Also it may be usefull in bugreports about ogg to know if its prior or
after this change

thx

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

Awnsering whenever a program halts or runs forever is
On a turing machine, in general impossible (turings halting problem).
On any real computer, always possible as a real computer has a finite number
of states N, and will either halt in less than N cycles or never halt.

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [FFmpeg-devel] [PATCH v2] ogg/vorbis: implement header packet skip in chained ogg bitstreams.
  2025-06-21 21:59     ` Michael Niedermayer
@ 2025-07-23 19:06       ` Romain Beauxis
  2025-07-28  0:22         ` Michael Niedermayer
  0 siblings, 1 reply; 15+ messages in thread
From: Romain Beauxis @ 2025-07-23 19:06 UTC (permalink / raw)
  To: FFmpeg development discussions and patches; +Cc: Michael Niedermayer

Le sam. 21 juin 2025 à 16:59, Michael Niedermayer <michael@niedermayer.cc>
a écrit :
>
> On Sat, Jun 21, 2025 at 10:45:32AM +0200, Romain Beauxis wrote:
> > Le dim. 15 juin 2025 à 00:57, Michael Niedermayer
> > <michael@niedermayer.cc> a écrit :
> > >
> > > On Wed, Jun 04, 2025 at 11:58:52AM -0500, Romain Beauxis wrote:
> > > > This is a redo of 574f634e49847e2225ee50013afebf0de03ef013 using a
flat
> > > > memory storage for the extradata.
> > > >
> > > > PR review comments addressed:
> > > > * Use flat memory bytestream
> > > > * Re-use existing xiph extradata layout
> > > >
> > > > ---
> > >
> > > >  libavcodec/vorbisdec.c                     | 42 ++++++++---
> > > >  libavformat/oggparsevorbis.c               | 83
+++++++++++++++++++++-
> > >
> > > patches that change both libraries at the same time are suspect
> > >
> > > if one depends on changes in the other it needs
> > > minor API version bump and seperate patches so extension of
> > > API and use of it are properly tracked and testable
> >
> > If I remember well, according to Andreas Rheinhardt there's no need
> > for an API bump here since the patch is re-using existing extradata
> > bitstream structures.
>
> If there is really no API extension then the micro versions should be
bumped
> so a user knows if the specific version he uses has teh fix.
> Also it may be usefull in bugreports about ogg to know if its prior or
> after this change

Hi Michael,

I have created a PR on code.ffmpeg.org here:
https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/20026

Would you have a minute to have a look?

This patch is a redo of a patch that was reverted:
https://code.ffmpeg.org/FFmpeg/FFmpeg/commit/848ceb1329cb6102df49379430b277dbb3f07569

It would be great to see if it could be included in the next release,
otherwise the round of work on ogg parsing would be incomplete for it.

I'm hoping that the PR can help gather and keep review feedback in one
place.

Thanks,
-- Romain

> thx
>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Awnsering whenever a program halts or runs forever is
> On a turing machine, in general impossible (turings halting problem).
> On any real computer, always possible as a real computer has a finite
number
> of states N, and will either halt in less than N cycles or never halt.
> _______________________________________________
> 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".

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [FFmpeg-devel] [PATCH v2] ogg/vorbis: implement header packet skip in chained ogg bitstreams.
  2025-07-23 19:06       ` Romain Beauxis
@ 2025-07-28  0:22         ` Michael Niedermayer
  2025-07-28 21:12           ` Romain Beauxis
  0 siblings, 1 reply; 15+ messages in thread
From: Michael Niedermayer @ 2025-07-28  0:22 UTC (permalink / raw)
  To: FFmpeg development discussions and patches


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

Hi Romain

On Wed, Jul 23, 2025 at 02:06:07PM -0500, Romain Beauxis wrote:
> Le sam. 21 juin 2025 à 16:59, Michael Niedermayer <michael@niedermayer.cc>
> a écrit :
> >
> > On Sat, Jun 21, 2025 at 10:45:32AM +0200, Romain Beauxis wrote:
> > > Le dim. 15 juin 2025 à 00:57, Michael Niedermayer
> > > <michael@niedermayer.cc> a écrit :
> > > >
> > > > On Wed, Jun 04, 2025 at 11:58:52AM -0500, Romain Beauxis wrote:
> > > > > This is a redo of 574f634e49847e2225ee50013afebf0de03ef013 using a
> flat
> > > > > memory storage for the extradata.
> > > > >
> > > > > PR review comments addressed:
> > > > > * Use flat memory bytestream
> > > > > * Re-use existing xiph extradata layout
> > > > >
> > > > > ---
> > > >
> > > > >  libavcodec/vorbisdec.c                     | 42 ++++++++---
> > > > >  libavformat/oggparsevorbis.c               | 83
> +++++++++++++++++++++-
> > > >
> > > > patches that change both libraries at the same time are suspect
> > > >
> > > > if one depends on changes in the other it needs
> > > > minor API version bump and seperate patches so extension of
> > > > API and use of it are properly tracked and testable
> > >
> > > If I remember well, according to Andreas Rheinhardt there's no need
> > > for an API bump here since the patch is re-using existing extradata
> > > bitstream structures.
> >
> > If there is really no API extension then the micro versions should be
> bumped
> > so a user knows if the specific version he uses has teh fix.
> > Also it may be usefull in bugreports about ogg to know if its prior or
> > after this change
> 
> Hi Michael,
> 
> I have created a PR on code.ffmpeg.org here:
> https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/20026
> 

> Would you have a minute to have a look?

I do not have time, ive made a dumb mistake in configuring my inbox
so iam about a week behind with emails without realizing it.
thats on top of release, security, and other things

maybe someome else can look into this one
replying here so noone waits for a review from me

thx

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

Any man who breaks a law that conscience tells him is unjust and willingly 
accepts the penalty by staying in jail in order to arouse the conscience of 
the community on the injustice of the law is at that moment expressing the 
very highest respect for law. - Martin Luther King Jr

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [FFmpeg-devel] [PATCH v2] ogg/vorbis: implement header packet skip in chained ogg bitstreams.
  2025-07-28  0:22         ` Michael Niedermayer
@ 2025-07-28 21:12           ` Romain Beauxis
  2025-08-03 21:36             ` Michael Niedermayer
  0 siblings, 1 reply; 15+ messages in thread
From: Romain Beauxis @ 2025-07-28 21:12 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

Le dim. 27 juil. 2025 à 19:22, Michael Niedermayer <michael@niedermayer.cc>
a écrit :
>
> Hi Romain
>
> On Wed, Jul 23, 2025 at 02:06:07PM -0500, Romain Beauxis wrote:
> > Le sam. 21 juin 2025 à 16:59, Michael Niedermayer <
michael@niedermayer.cc>
> > a écrit :
> > >
> > > On Sat, Jun 21, 2025 at 10:45:32AM +0200, Romain Beauxis wrote:
> > > > Le dim. 15 juin 2025 à 00:57, Michael Niedermayer
> > > > <michael@niedermayer.cc> a écrit :
> > > > >
> > > > > On Wed, Jun 04, 2025 at 11:58:52AM -0500, Romain Beauxis wrote:
> > > > > > This is a redo of 574f634e49847e2225ee50013afebf0de03ef013
using a
> > flat
> > > > > > memory storage for the extradata.
> > > > > >
> > > > > > PR review comments addressed:
> > > > > > * Use flat memory bytestream
> > > > > > * Re-use existing xiph extradata layout
> > > > > >
> > > > > > ---
> > > > >
> > > > > >  libavcodec/vorbisdec.c                     | 42 ++++++++---
> > > > > >  libavformat/oggparsevorbis.c               | 83
> > +++++++++++++++++++++-
> > > > >
> > > > > patches that change both libraries at the same time are suspect
> > > > >
> > > > > if one depends on changes in the other it needs
> > > > > minor API version bump and seperate patches so extension of
> > > > > API and use of it are properly tracked and testable
> > > >
> > > > If I remember well, according to Andreas Rheinhardt there's no need
> > > > for an API bump here since the patch is re-using existing extradata
> > > > bitstream structures.
> > >
> > > If there is really no API extension then the micro versions should be
> > bumped
> > > so a user knows if the specific version he uses has teh fix.
> > > Also it may be usefull in bugreports about ogg to know if its prior or
> > > after this change
> >
> > Hi Michael,
> >
> > I have created a PR on code.ffmpeg.org here:
> > https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/20026
> >
>
> > Would you have a minute to have a look?
>
> I do not have time, ive made a dumb mistake in configuring my inbox
> so iam about a week behind with emails without realizing it.
> thats on top of release, security, and other things
>
> maybe someome else can look into this one
> replying here so noone waits for a review from me

Thanks for letting me know and sorry about your issues with your email
inbox.

Do you have any advice on how to look for a reviewer for this path?

> thx
>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Any man who breaks a law that conscience tells him is unjust and willingly
> accepts the penalty by staying in jail in order to arouse the conscience
of
> the community on the injustice of the law is at that moment expressing the
> very highest respect for law. - Martin Luther King Jr
> _______________________________________________
> 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".

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [FFmpeg-devel] [PATCH v2] ogg/vorbis: implement header packet skip in chained ogg bitstreams.
  2025-07-28 21:12           ` Romain Beauxis
@ 2025-08-03 21:36             ` Michael Niedermayer
  2025-08-03 22:50               ` Romain Beauxis
  0 siblings, 1 reply; 15+ messages in thread
From: Michael Niedermayer @ 2025-08-03 21:36 UTC (permalink / raw)
  To: FFmpeg development discussions and patches


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

Hi

On Mon, Jul 28, 2025 at 04:12:13PM -0500, Romain Beauxis wrote:
> Le dim. 27 juil. 2025 à 19:22, Michael Niedermayer <michael@niedermayer.cc>
> a écrit :
> >
> > Hi Romain
> >
> > On Wed, Jul 23, 2025 at 02:06:07PM -0500, Romain Beauxis wrote:
> > > Le sam. 21 juin 2025 à 16:59, Michael Niedermayer <
> michael@niedermayer.cc>
> > > a écrit :
> > > >
> > > > On Sat, Jun 21, 2025 at 10:45:32AM +0200, Romain Beauxis wrote:
> > > > > Le dim. 15 juin 2025 à 00:57, Michael Niedermayer
> > > > > <michael@niedermayer.cc> a écrit :
> > > > > >
> > > > > > On Wed, Jun 04, 2025 at 11:58:52AM -0500, Romain Beauxis wrote:
> > > > > > > This is a redo of 574f634e49847e2225ee50013afebf0de03ef013
> using a
> > > flat
> > > > > > > memory storage for the extradata.
> > > > > > >
> > > > > > > PR review comments addressed:
> > > > > > > * Use flat memory bytestream
> > > > > > > * Re-use existing xiph extradata layout
> > > > > > >
> > > > > > > ---
> > > > > >
> > > > > > >  libavcodec/vorbisdec.c                     | 42 ++++++++---
> > > > > > >  libavformat/oggparsevorbis.c               | 83
> > > +++++++++++++++++++++-
> > > > > >
> > > > > > patches that change both libraries at the same time are suspect
> > > > > >
> > > > > > if one depends on changes in the other it needs
> > > > > > minor API version bump and seperate patches so extension of
> > > > > > API and use of it are properly tracked and testable
> > > > >
> > > > > If I remember well, according to Andreas Rheinhardt there's no need
> > > > > for an API bump here since the patch is re-using existing extradata
> > > > > bitstream structures.
> > > >
> > > > If there is really no API extension then the micro versions should be
> > > bumped
> > > > so a user knows if the specific version he uses has teh fix.
> > > > Also it may be usefull in bugreports about ogg to know if its prior or
> > > > after this change
> > >
> > > Hi Michael,
> > >
> > > I have created a PR on code.ffmpeg.org here:
> > > https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/20026
> > >
> >
> > > Would you have a minute to have a look?
> >
> > I do not have time, ive made a dumb mistake in configuring my inbox
> > so iam about a week behind with emails without realizing it.
> > thats on top of release, security, and other things
> >
> > maybe someome else can look into this one
> > replying here so noone waits for a review from me
> 
> Thanks for letting me know and sorry about your issues with your email
> inbox.
> 
> Do you have any advice on how to look for a reviewer for this path?

social media, like twitter

also if it has a positive vibe then we can retweet

thx

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

Those who are too smart to engage in politics are punished by being
governed by those who are dumber. -- Plato 

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [FFmpeg-devel] [PATCH v2] ogg/vorbis: implement header packet skip in chained ogg bitstreams.
  2025-08-03 21:36             ` Michael Niedermayer
@ 2025-08-03 22:50               ` Romain Beauxis
  2025-08-04  0:11                 ` Michael Niedermayer
  0 siblings, 1 reply; 15+ messages in thread
From: Romain Beauxis @ 2025-08-03 22:50 UTC (permalink / raw)
  To: FFmpeg development discussions and patches
  Cc: Marvin Scholz, Lynne, Andreas Rheinhardt

Le dim. 3 août 2025 à 16:36, Michael Niedermayer
<michael@niedermayer.cc> a écrit :
>
> Hi
>
> On Mon, Jul 28, 2025 at 04:12:13PM -0500, Romain Beauxis wrote:
> > Le dim. 27 juil. 2025 à 19:22, Michael Niedermayer <michael@niedermayer.cc>
> > a écrit :
> > >
> > > Hi Romain
> > >
> > > On Wed, Jul 23, 2025 at 02:06:07PM -0500, Romain Beauxis wrote:
> > > > Le sam. 21 juin 2025 à 16:59, Michael Niedermayer <
> > michael@niedermayer.cc>
> > > > a écrit :
> > > > >
> > > > > On Sat, Jun 21, 2025 at 10:45:32AM +0200, Romain Beauxis wrote:
> > > > > > Le dim. 15 juin 2025 à 00:57, Michael Niedermayer
> > > > > > <michael@niedermayer.cc> a écrit :
> > > > > > >
> > > > > > > On Wed, Jun 04, 2025 at 11:58:52AM -0500, Romain Beauxis wrote:
> > > > > > > > This is a redo of 574f634e49847e2225ee50013afebf0de03ef013
> > using a
> > > > flat
> > > > > > > > memory storage for the extradata.
> > > > > > > >
> > > > > > > > PR review comments addressed:
> > > > > > > > * Use flat memory bytestream
> > > > > > > > * Re-use existing xiph extradata layout
> > > > > > > >
> > > > > > > > ---
> > > > > > >
> > > > > > > >  libavcodec/vorbisdec.c                     | 42 ++++++++---
> > > > > > > >  libavformat/oggparsevorbis.c               | 83
> > > > +++++++++++++++++++++-
> > > > > > >
> > > > > > > patches that change both libraries at the same time are suspect
> > > > > > >
> > > > > > > if one depends on changes in the other it needs
> > > > > > > minor API version bump and seperate patches so extension of
> > > > > > > API and use of it are properly tracked and testable
> > > > > >
> > > > > > If I remember well, according to Andreas Rheinhardt there's no need
> > > > > > for an API bump here since the patch is re-using existing extradata
> > > > > > bitstream structures.
> > > > >
> > > > > If there is really no API extension then the micro versions should be
> > > > bumped
> > > > > so a user knows if the specific version he uses has teh fix.
> > > > > Also it may be usefull in bugreports about ogg to know if its prior or
> > > > > after this change
> > > >
> > > > Hi Michael,
> > > >
> > > > I have created a PR on code.ffmpeg.org here:
> > > > https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/20026
> > > >
> > >
> > > > Would you have a minute to have a look?
> > >
> > > I do not have time, ive made a dumb mistake in configuring my inbox
> > > so iam about a week behind with emails without realizing it.
> > > thats on top of release, security, and other things
> > >
> > > maybe someome else can look into this one
> > > replying here so noone waits for a review from me
> >
> > Thanks for letting me know and sorry about your issues with your email
> > inbox.
> >
> > Do you have any advice on how to look for a reviewer for this path?
>
> social media, like twitter
>
> also if it has a positive vibe then we can retweet

Respectfully, this does not make sense to me.

I already spend way too much time on social media promoting things
that actually need promotion like music shows. I do not want to have
my contributions to a technical project be judged by the hype. I
strive to do good work that has merit on its own.

But, specifically and in relation to this patch, this also does not
make sense because this patch is a technical fix from a long windy
string of events.

Let me recap:

I initially submitted a one-liner patch to fix chained opus streams:
https://ffmpeg.org/pipermail/ffmpeg-devel/2025-January/338538.html

(This was Jan. 18!)

I received feedback from Michael Niedermayer (you!) to add a fate test
and Marvin Scholz about the right way to do it. Great

This led to several versions of the patchset over which I received
feedback from  Lynne about how to properly remove header packets from
the demuxer and Andreas Rheinhardt about how to properly pack the
binary data from those headers.

This was all great, frankly, and I learned a lot.

Eventually, the patch set was merged in several waves by you and
included a pretty robust fate test application that makes it possible
to follow the subsequent change in the ogg demuxer packets and
metadata output on each change.

However, I had made a mistake on the patch handling ogg/vorbis
extradata packing so that one patch was reverted by Andreas
Rheinhardt.

I communicated with them over IRC and got feedback from Andreas
Rheinhardt. on how to properly re-use the avpriv_split_xiph_headers
function and its associated binary payload.

Again, learned a lot, worked out a fixed patch. Really happy about that.

This one patch is the one missing from this series that is preventing
the whole series from being properly included in the upcoming release
and blocking the rest of the work that I have already done from being
submitted.

(side note: the rest of work is really much needed, fixing PTS/DTS
discontinuity and making ogg streams remuxing work)

But, alas, the patch has been waiting for someone to look at it for months now.

I had several "verbal" (IRC) commitments from Andreas Rheinhardt that
they would review it but it never happened.

I truly do not understand why this is happening. The whole saga has
now involved 4 seasoned project members.

This patch seems safe, it is thoroughly tested thanks to your
recommendation about adding fate tests and it follows the recommended
guidelines from Lynne and Andreas Rheinhardt.

So, what am I missing here? Do I need to refer to the technical
committee like Rémi Denis-Courmont suggested?

Thanks,
-- Romain

> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Those who are too smart to engage in politics are punished by being
> governed by those who are dumber. -- Plato
> _______________________________________________
> 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".

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [FFmpeg-devel] [PATCH v2] ogg/vorbis: implement header packet skip in chained ogg bitstreams.
  2025-08-03 22:50               ` Romain Beauxis
@ 2025-08-04  0:11                 ` Michael Niedermayer
  2025-08-04  0:19                   ` Kieran Kunhya via ffmpeg-devel
  2025-08-04  8:21                   ` Michael Niedermayer
  0 siblings, 2 replies; 15+ messages in thread
From: Michael Niedermayer @ 2025-08-04  0:11 UTC (permalink / raw)
  To: FFmpeg development discussions and patches
  Cc: Marvin Scholz, Lynne, Andreas Rheinhardt


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

Hi Romain

On Sun, Aug 03, 2025 at 05:50:17PM -0500, Romain Beauxis wrote:
> Le dim. 3 août 2025 à 16:36, Michael Niedermayer
> <michael@niedermayer.cc> a écrit :
[...]
> > > Do you have any advice on how to look for a reviewer for this path?
> >
> > social media, like twitter
> >
> > also if it has a positive vibe then we can retweet
> 
> Respectfully, this does not make sense to me.

i dont have time ATM, and it seems the others also are overloaded

-> so the goal is to attract more reviewing manpower

but surely eventually someone will have time and look at it
also this is not the most trivial patchset


[...]

> So, what am I missing here? Do I need to refer to the technical

you can try, but i think that will cause more work and more delays

you seem to not understand that i have a release to work on thats
months behind, security fixes, a fork i wanted to merge oe cherry pick,
and many other things. You can surely see that your patch is not the only
thing iam not working on, theres many things i want to work on
and do not have the time.

The only solution is more manpower.

thx

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

Dictatorship: All citizens are under surveillance, all their steps and
actions recorded, for the politicians to enforce control.
Democracy: All politicians are under surveillance, all their steps and
actions recorded, for the citizens to enforce control.

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [FFmpeg-devel] [PATCH v2] ogg/vorbis: implement header packet skip in chained ogg bitstreams.
  2025-08-04  0:11                 ` Michael Niedermayer
@ 2025-08-04  0:19                   ` Kieran Kunhya via ffmpeg-devel
  2025-08-04  8:21                   ` Michael Niedermayer
  1 sibling, 0 replies; 15+ messages in thread
From: Kieran Kunhya via ffmpeg-devel @ 2025-08-04  0:19 UTC (permalink / raw)
  To: FFmpeg development discussions and patches; +Cc: Kieran Kunhya

>
> The only solution is more manpower.
>

It's really a huge mystery why developers are leaving.

We may never know the answer.

Kieran

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [FFmpeg-devel] [PATCH v2] ogg/vorbis: implement header packet skip in chained ogg bitstreams.
  2025-08-04  0:11                 ` Michael Niedermayer
  2025-08-04  0:19                   ` Kieran Kunhya via ffmpeg-devel
@ 2025-08-04  8:21                   ` Michael Niedermayer
  1 sibling, 0 replies; 15+ messages in thread
From: Michael Niedermayer @ 2025-08-04  8:21 UTC (permalink / raw)
  To: FFmpeg development discussions and patches
  Cc: Marvin Scholz, Lynne, Andreas Rheinhardt


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

Hi Romain

On Mon, Aug 04, 2025 at 02:11:03AM +0200, Michael Niedermayer wrote:
> Hi Romain
> 
> On Sun, Aug 03, 2025 at 05:50:17PM -0500, Romain Beauxis wrote:
> > Le dim. 3 août 2025 à 16:36, Michael Niedermayer
> > <michael@niedermayer.cc> a écrit :
> [...]
> > > > Do you have any advice on how to look for a reviewer for this path?
> > >
> > > social media, like twitter
> > >
> > > also if it has a positive vibe then we can retweet
> > 
> > Respectfully, this does not make sense to me.
> 
> i dont have time ATM, and it seems the others also are overloaded
> 
> -> so the goal is to attract more reviewing manpower
> 
> but surely eventually someone will have time and look at it
> also this is not the most trivial patchset

I ve reviewed your patch on https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/20026 now

you ignored review comments made previously

The patch also contained a trivial mistake:
(maybe review your own code if you have time and reviewers are all busy)
(also as said previously (IIRC) if reviewers are all busy, help review other peoples
 patches)
        priv->comment_size = 0;
        av_freep(&priv->setup);
        priv->comment_size = 0;

Also its summer vacation time and everyone is busy

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Some Animals are More Equal Than Others. - George Orwell's book Animal Farm

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2025-08-04  8:21 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-06-04 16:58 [FFmpeg-devel] [PATCH v2] ogg/vorbis: implement header packet skip in chained ogg bitstreams Romain Beauxis
2025-06-10 18:04 ` Romain Beauxis
2025-06-12 11:35   ` Michael Niedermayer
2025-06-14 10:39     ` Romain Beauxis
2025-06-14 22:57 ` Michael Niedermayer
2025-06-21  8:45   ` Romain Beauxis
2025-06-21 21:59     ` Michael Niedermayer
2025-07-23 19:06       ` Romain Beauxis
2025-07-28  0:22         ` Michael Niedermayer
2025-07-28 21:12           ` Romain Beauxis
2025-08-03 21:36             ` Michael Niedermayer
2025-08-03 22:50               ` Romain Beauxis
2025-08-04  0:11                 ` Michael Niedermayer
2025-08-04  0:19                   ` Kieran Kunhya via ffmpeg-devel
2025-08-04  8:21                   ` Michael Niedermayer

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git