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

[Romain Beauxis <romain.beauxis@gmail.com>]

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