[vlc-commits] ogg: Fix borken stream resets.

[Timothy B. Terriberry]

vlc/vlc-2.0 | branch: master | Timothy B. Terriberry <tterribe at xiph.org> | Tue Feb 26 18:36:02 2013 -0800| [1e604e1f26edad8cea2ecab42913a982409d81c9] | committer: Jean-Baptiste Kempf

ogg: Fix borken stream resets.

Commit 78a8771f attempted to reset the ogg_stream_state in
find_last_frame(), but it tries to do so using a non-existent page
and then leaks memory (due to using ogg_stream_init() instead of
ogg_stream_reset()). Its caller, Ogg_GetLastPacket(), then tries
to restore the contents of the current packet, but its call to
ogg_stream_pageseek() will immediately fail, leaving the dangling
packet pointers untouched.

Instead, just back up the whole stream state and restore it on
exit. This fixes both problems.

Signed-off-by: Jean-Baptiste Kempf <jb at videolan.org>
(cherry picked from commit d6aeeebe0c7ec9f4defe1751454bc3c1943d84e1)

Signed-off-by: Jean-Baptiste Kempf <jb at videolan.org>

> http://git.videolan.org/gitweb.cgi/vlc/vlc-2.0.git/?a=commit;h=1e604e1f26edad8cea2ecab42913a982409d81c9
---

 modules/demux/ogg.c     |   16 +++++-----------
 modules/demux/oggseek.c |   13 ++++++++++---
 2 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/modules/demux/ogg.c b/modules/demux/ogg.c
index 29b65bc..8a041c6 100644
--- a/modules/demux/ogg.c
+++ b/modules/demux/ogg.c
@@ -135,7 +135,7 @@ static bool Ogg_LogicalStreamResetEsFormat( demux_t *p_demux, logical_stream_t *
 
 /* */
 static void Ogg_ExtractMeta( demux_t *p_demux, vlc_fourcc_t i_codec, const uint8_t *p_headers, int i_headers );
-static int64_t Ogg_GetLastPacket( demux_t *p_demux, logical_stream_t *p_stream, ogg_packet *p_oggpacket, double f_rate );
+static int64_t Ogg_GetLastPacket( demux_t *p_demux, logical_stream_t *p_stream, double f_rate );
 
 /* Logical bitstream headers */
 static void Ogg_ReadTheoraHeader( demux_t *, logical_stream_t *, ogg_packet * );
@@ -1827,15 +1827,9 @@ static void Ogg_ExtractMeta( demux_t *p_demux, vlc_fourcc_t i_codec, const uint8
 }
 
 static int64_t Ogg_GetLastPacket( demux_t *p_demux, logical_stream_t *p_stream,
-                                  ogg_packet *p_oggpacket, double f_rate )
+                                  double f_rate )
 {
     int64_t last_packet = oggseek_get_last_frame( p_demux, p_stream );
-    /*
-     * Since there's quite a good chance that ogg_stream_packetout was called,
-     * the given p_oggpacket may point to invalid data. Fill it with some valid ones
-     */
-    ogg_stream_packetpeek( &p_stream->os, p_oggpacket );
-
     return ( last_packet >= 0 ) ? last_packet / f_rate : -1;
 }
 
@@ -1907,7 +1901,7 @@ static void Ogg_ReadTheoraHeader( demux_t *p_demux, logical_stream_t *p_stream,
     }
     if ( p_demux->p_sys->i_length < 0 )
     {
-        int64_t last_packet = Ogg_GetLastPacket( p_demux, p_stream, p_oggpacket, p_stream->f_rate );
+        int64_t last_packet = Ogg_GetLastPacket( p_demux, p_stream, p_stream->f_rate );
         if ( last_packet >= 0 )
             p_demux->p_sys->i_length = last_packet;
     }
@@ -1938,7 +1932,7 @@ static void Ogg_ReadVorbisHeader( demux_t *p_demux, logical_stream_t *p_stream,
 
     if ( p_demux->p_sys->i_length < 0 )
     {
-        int64_t last_packet = Ogg_GetLastPacket( p_demux, p_stream, p_oggpacket, p_stream->f_rate );
+        int64_t last_packet = Ogg_GetLastPacket( p_demux, p_stream, p_stream->f_rate );
         if ( last_packet >= 0 )
             p_demux->p_sys->i_length = last_packet;
     }
@@ -1997,7 +1991,7 @@ static void Ogg_ReadOpusHeader( demux_t *p_demux,
 
     if ( p_demux->p_sys->i_length < 0 )
     {
-        int64_t last_packet = Ogg_GetLastPacket( p_demux, p_stream, p_oggpacket, p_stream->f_rate );
+        int64_t last_packet = Ogg_GetLastPacket( p_demux, p_stream, p_stream->f_rate );
         if ( last_packet >= 0 )
             p_demux->p_sys->i_length = last_packet;
     }
diff --git a/modules/demux/oggseek.c b/modules/demux/oggseek.c
index 42d08ee..e22f66f 100644
--- a/modules/demux/oggseek.c
+++ b/modules/demux/oggseek.c
@@ -696,6 +696,14 @@ static int64_t get_last_frame ( demux_t *p_demux, logical_stream_t *p_stream )
 {
     demux_sys_t *p_sys  = p_demux->p_sys;
     int64_t i_frame;
+    ogg_stream_state os;
+
+    /* Backup the stream state. We get called during header processing, and our
+     * caller expects its header packet to remain valid after the call. If we
+     * let find_last_frame() reuse the same stream state, then it will
+     * invalidate the pointers in that packet. */
+    memcpy(&os, &p_stream->os, sizeof(os));
+    ogg_stream_init( &p_stream->os, p_stream->i_serial_no );
 
     i_frame = find_last_frame ( p_demux, p_stream );
 
@@ -708,9 +716,8 @@ static int64_t get_last_frame ( demux_t *p_demux, logical_stream_t *p_stream )
     seek_byte( p_demux, 0 );
     /* Reset stream states */
     p_sys->i_streams = 0;
-    p_stream->i_serial_no = ogg_page_serialno( &p_sys->current_page );
-    ogg_stream_init( &p_stream->os, p_stream->i_serial_no );
-    ogg_stream_pagein( &p_stream->os, &p_sys->current_page );
+    ogg_stream_clear( &p_stream->os );
+    memcpy( &p_stream->os, &os, sizeof(os) );
 
     return i_frame;
 }