Le mar. 29 avr. 2025 à 16:35, Michael Niedermayer
<mich...@niedermayer.cc> a écrit :
>
> Hi
>
> On Mon, Apr 28, 2025 at 06:31:36PM -0500, Romain Beauxis wrote:
> > ---
> > libavformat/oggdec.c | 26 ++++++++++----------
> > libavformat/oggdec.h | 6 +++++
> > libavformat/oggparseflac.c | 28 ++++++++++++++++++++--
> > libavformat/oggparseopus.c | 12 ++++++++++
> > libavformat/oggparsevorbis.c | 12 ++++++++--
> > tests/ref/fate/ogg-flac-chained-meta.txt | 2 --
> > tests/ref/fate/ogg-opus-chained-meta.txt | 1 -
> > tests/ref/fate/ogg-vorbis-chained-meta.txt | 3 ---
> > 8 files changed, 68 insertions(+), 22 deletions(-)
> >
> > diff --git a/libavformat/oggdec.c b/libavformat/oggdec.c
> > index 5339fdd32c..5557eb4a14 100644
> > --- a/libavformat/oggdec.c
> > +++ b/libavformat/oggdec.c
> > @@ -239,10 +239,6 @@ static int ogg_replace_stream(AVFormatContext *s,
> > uint32_t serial, char *magic,
> > os->start_trimming = 0;
> > os->end_trimming = 0;
> >
> > - /* Chained files have extradata as a new packet */
> > - if (codec == &ff_opus_codec)
> > - os->header = -1;
> > -
> > return i;
> > }
> >
> > @@ -605,20 +601,26 @@ static int ogg_packet(AVFormatContext *s, int *sid,
> > int *dstart, int *dsize,
> > } else {
> > os->pflags = 0;
> > os->pduration = 0;
> > +
> > + ret = 0;
> > if (os->codec && os->codec->packet) {
> > if ((ret = os->codec->packet(s, idx)) < 0) {
> > av_log(s, AV_LOG_ERROR, "Packet processing failed: %s\n",
> > av_err2str(ret));
> > return ret;
> > }
> > }
> > - if (sid)
> > - *sid = idx;
> > - if (dstart)
> > - *dstart = os->pstart;
> > - if (dsize)
> > - *dsize = os->psize;
> > - if (fpos)
> > - *fpos = os->sync_pos;
> > +
> > + if (!ret) {
> > + if (sid)
> > + *sid = idx;
> > + if (dstart)
> > + *dstart = os->pstart;
> > + if (dsize)
> > + *dsize = os->psize;
> > + if (fpos)
> > + *fpos = os->sync_pos;
> > + }
> > +
> > os->pstart += os->psize;
> > os->psize = 0;
> > if(os->pstart == os->bufpos)
>
> > diff --git a/libavformat/oggdec.h b/libavformat/oggdec.h
> > index 43df23f4cb..09f698f99a 100644
> > --- a/libavformat/oggdec.h
> > +++ b/libavformat/oggdec.h
> > @@ -38,6 +38,12 @@ struct ogg_codec {
> > * -1 if an error occurred or for unsupported stream
> > */
> > int (*header)(AVFormatContext *, int);
> > + /**
> > + * Attempt to process a packet as a data packet
> > + * @return 1 if the packet was a header from a chained bitstream.
> > + * 0 if the packet was a regular data packet.
> > + * -1 if an error occurred or for unsupported stream
> > + */
> > int (*packet)(AVFormatContext *, int);
> > /**
> > * Translate a granule into a timestamp.
>
> ok, but seems unrelated
This is a new convention to allow the parser to know when to skip a
packet. Previous to that, return value of 1 did not have specific
meaning.
> > diff --git a/libavformat/oggparseflac.c b/libavformat/oggparseflac.c
> > index f25ed9cc15..d66b85b09e 100644
> > --- a/libavformat/oggparseflac.c
> > +++ b/libavformat/oggparseflac.c
> > @@ -27,6 +27,8 @@
> > #include "oggdec.h"
> >
> > #define OGG_FLAC_METADATA_TYPE_STREAMINFO 0x7F
> > +#define OGG_FLAC_MAGIC "\177FLAC"
> > +#define OGG_FLAC_MAGIC_SIZE sizeof(OGG_FLAC_MAGIC)-1
>
> factoring this out could be a seperate patch
>
>
> >
> > static int
> > flac_header (AVFormatContext * s, int idx)
> > @@ -78,6 +80,27 @@ flac_header (AVFormatContext * s, int idx)
> > return 1;
> > }
> >
> > +static int
> > +flac_packet (AVFormatContext * s, int idx)
> > +{
> > + struct ogg *ogg = s->priv_data;
> > + struct ogg_stream *os = ogg->streams + idx;
> > +
> > + if (os->psize > OGG_FLAC_MAGIC_SIZE &&
> > + !memcmp(
> > + os->buf + os->pstart,
> > + OGG_FLAC_MAGIC,
> > + OGG_FLAC_MAGIC_SIZE))
> > + return 1;
> > +
> > + if (os->psize > 0 &&
> > + ((os->buf[os->pstart] & 0x7F) ==
> > FLAC_METADATA_TYPE_VORBIS_COMMENT)) {
> > + return 1;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > static int
> > old_flac_header (AVFormatContext * s, int idx)
> > {
> > @@ -127,10 +150,11 @@ fail:
> > }
> >
> > const struct ogg_codec ff_flac_codec = {
> > - .magic = "\177FLAC",
> > - .magicsize = 5,
> > + .magic = OGG_FLAC_MAGIC,
> > + .magicsize = OGG_FLAC_MAGIC_SIZE,
> > .header = flac_header,
> > .nb_header = 2,
> > + .packet = flac_packet,
> > };
> >
> > const struct ogg_codec ff_old_flac_codec = {
>
> > diff --git a/libavformat/oggparseopus.c b/libavformat/oggparseopus.c
> > index 218e9df581..07ddba8c82 100644
> > --- a/libavformat/oggparseopus.c
> > +++ b/libavformat/oggparseopus.c
> > @@ -81,6 +81,7 @@ static int opus_header(AVFormatContext *avf, int idx)
> > if (priv->need_comments) {
> > if (os->psize < 8 || memcmp(packet, "OpusTags", 8))
> > return AVERROR_INVALIDDATA;
> > +
> > ff_vorbis_stream_comment(avf, st, packet + 8, os->psize - 8);
> > priv->need_comments--;
> > return 1;
>
> stray change
>
>
> > @@ -125,6 +126,17 @@ static int opus_packet(AVFormatContext *avf, int idx)
> > return AVERROR_INVALIDDATA;
> > }
> >
> > + if (os->psize > 8 && !memcmp(packet, "OpusHead", 8)) {
> > + if ((ret = ff_alloc_extradata(st->codecpar, os->psize)) < 0)
> > + return ret;
> > +
> > + memcpy(st->codecpar->extradata, packet, os->psize);
> > + return 1;
> > + }
> > +
> > + if (os->psize > 8 && !memcmp(packet, "OpusTags", 8))
> > + return 1;
> > +
> > if ((!os->lastpts || os->lastpts == AV_NOPTS_VALUE) && !(os->flags &
> > OGG_FLAG_EOS)) {
> > int seg, d;
> > int duration;
> > diff --git a/libavformat/oggparsevorbis.c b/libavformat/oggparsevorbis.c
> > index 9f50ab9ffc..248ac4a416 100644
> > --- a/libavformat/oggparsevorbis.c
> > +++ b/libavformat/oggparsevorbis.c
> > @@ -418,6 +418,7 @@ 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;
> >
> > if (!priv->vp)
> > return AVERROR_INVALIDDATA;
>
> > @@ -477,10 +478,17 @@ static int vorbis_packet(AVFormatContext *s, int idx)
> > /* parse packet duration */
> > if (os->psize > 0) {
> > duration = av_vorbis_parse_frame_flags(priv->vp, os->buf +
> > os->pstart, 1, &flags);
> > +
> > if (duration < 0) {
>
> stray change
>
> [...]
Thanks for the review. You can find an updated commit here if you want
to avoid another post to the ML:
https://github.com/toots/FFmpeg/commit/140c4cafe660abed67212a4b4c128628b4dd01f5
> thx
>
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> I am the wisest man alive, for I know one thing, and that is that I know
> nothing. -- Socrates
> _______________________________________________
> 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".
_______________________________________________
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".
