[FFmpeg-devel] [PATCH] oggdec: do not set time_base to invalid value

[Reimar Döffinger]

On Thu, Jan 20, 2011 at 09:53:35PM -0500, Ronald S. Bultje wrote:
> On Thu, Jan 20, 2011 at 5:38 PM, M?ns Rullg?rd <mans at mansr.com> wrote:
> > Reimar D?ffinger <Reimar.Doeffinger at gmx.de> writes:
> >> possible we should consider catching this at a higher level, and possibly
> >> st->codec->sample_rate should not be assigned an invalid value either...
> >> Vorbis ogg parser may set st->time_base.den to 0, causing an
> >> assertion failure in av_rescale_* during av_update_stream_timings.
> >
> > This is a recurring theme. ?It needs to be addressed in a better way.
> 
> I thought Daniel Kang had submitted a patch that does this globally,
> so formats don't have to do it per-format?  Why does that fail to
> catch this case and error out properly?

It does catch it, but that check is only for sample-rate.
The time_base is still set to something invalid.
Setting both num and den to 0 results in the same assert.
Only setting num to 0 feels wrong to me, I'd rather keep the
default value in that case, particularly since there's at least
the theoretical possibility that there are multiple headers
so there might already be a valid value there (though I think the
demuxer will refuse such a file anyway and it would be better not
to change sample_rate in that case either).
Index: libavformat/oggparsevorbis.c
===================================================================
--- libavformat/oggparsevorbis.c        (revision 26402)
+++ libavformat/oggparsevorbis.c        (working copy)
@@ -221,6 +221,7 @@
     if (os->buf[os->pstart] == 1) {
         const uint8_t *p = os->buf + os->pstart + 7; /* skip "\001vorbis" tag */
         unsigned blocksize, bs0, bs1;
+        int srate;
 
         if (os->psize != 30)
             return -1;
@@ -229,7 +230,7 @@
             return -1;
 
         st->codec->channels = bytestream_get_byte(&p);
-        st->codec->sample_rate = bytestream_get_le32(&p);
+        srate = bytestream_get_le32(&p);
         p += 4; // skip maximum bitrate
         st->codec->bit_rate = bytestream_get_le32(&p); // nominal bitrate
         p += 4; // skip minimum bitrate
@@ -249,8 +250,11 @@
         st->codec->codec_type = AVMEDIA_TYPE_AUDIO;
         st->codec->codec_id = CODEC_ID_VORBIS;
 
-        st->time_base.num = 1;
-        st->time_base.den = st->codec->sample_rate;
+        if (srate > 0) {
+            st->codec->sample_rate = srate;
+            st->time_base.num = 1;
+            st->time_base.den = srate;
+        }
     } 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);