Re: [FFmpeg-devel] [PATCH v2 2/2] ogg/{vorbis, flac, opus}: Remove header packets from subsequent ogg streams from the demuxer output.

[Michael Niedermayer <michael@niedermayer.cc>]

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

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


> 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

[...]

thx

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I am the wisest man alive, for I know one thing, and that is that I know
nothing. -- Socrates

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