[FFmpeg-devel] [PATCH] ogg: discard vorbis metadata from extradata

[Reimar Döffinger]

On Fri, Jan 21, 2011 at 06:34:16PM +0000, M?ns Rullg?rd wrote:
> Reimar D?ffinger <Reimar.Doeffinger at gmx.de> writes:
> >> > +    if (pkt_type != 3) {
> >> >      priv->len[pkt_type >> 1] = os->psize;
> >> >      priv->packet[pkt_type >> 1] = av_mallocz(os->psize);
> >> >      memcpy(priv->packet[pkt_type >> 1], os->buf + os->pstart, os->psize);
> >> > +    }
> >> 
> >> IIRC there are apps which expect to find the metadata header in the
> >> extradata in order to send it to libvorbis (which _insists_ on having
> >> it).  This will break such apps unless a dummy metadata header is
> >> inserted here instead.
> >
> > Hard to implement that without having such an application at hand.
> 
> I don't see why.

Because developing without testing doesn't work. However MPlayer can do this
if you force it.

> > Also I thought that libvorbis wouldn't accept this extradata format
> > directly anyway, so such applications already have to rewrite this
> > so it is not that much effort for them to add 
> 
> Vorbis in matroska uses the same extradata format.  How do apps handle
> that wrt metadata?

They convert it back to the original format and call the headerin function.
Anyway below a patch that discards all except for the part that libvorbis
requires.
Which happens to fit quite well since we do not actually parse that part into
metadata and I guess it was intended to allow workarounds for encoder bugs
on the decoder side.

diff --git a/libavformat/oggparsevorbis.c b/libavformat/oggparsevorbis.c
index cdb0266..bf0b4bb 100644
--- a/libavformat/oggparsevorbis.c
+++ b/libavformat/oggparsevorbis.c
@@ -252,8 +252,16 @@ vorbis_header (AVFormatContext * s, int idx)
         st->time_base.num = 1;
         st->time_base.den = st->codec->sample_rate;
     } else if (os->buf[os->pstart] == 3) {
-        if (os->psize > 8)
-            ff_vorbis_comment (s, &st->metadata, os->buf + os->pstart + 7, os->psize - 8);
+        if (os->psize > 8 &&
+            ff_vorbis_comment(s, &st->metadata, os->buf + os->pstart + 7, os->psize - 8) >= 0) {
+            // drop all metadata we parsed and which is not required by libvorbis
+            unsigned new_len = 7 + 4 + AV_RL32(priv->packet[1] + 7) + 4 + 1;
+            if (new_len >= 16 && new_len < os->psize) {
+                AV_WL32(priv->packet[1] + new_len - 5, 0);
+                priv->packet[1][new_len - 1] = 1;
+                priv->len[1] = new_len;
+            }
+        }
     } else {
         st->codec->extradata_size =
             fixup_vorbis_headers(s, priv, &st->codec->extradata);