Bug 495129. Propagate errors better. r+sr=roc

content/media/video/src/nsOggDecoder.cpp

@@ -301,6 +301,11 @@ public:
   // E_OGGPLAY_TIMEOUT        = No frames decoded, timed out
   OggPlayErrorCode DecodeFrame();
 
+  // Handle any errors returned by liboggplay when decoding a frame.
+  // Since this function can change the decoding state it must be called
+  // with the decoder lock held.
+  void HandleDecodeErrors(OggPlayErrorCode r);
+
   // Returns the next decoded frame of data. The caller is responsible
   // for freeing the memory returned. This function must be called
   // only when the current state > DECODING_METADATA. The decode
@@ -653,6 +658,8 @@ public:
         r = oggplay_step_decoding(mPlayer);
         mon.Enter();
 
+        mDecodeStateMachine->HandleDecodeErrors(r);
+
         // Check whether decoding the last frame required us to read data
         // that wasn't available at the start of the frame. That means
         // we should probably start buffering.
@@ -716,7 +723,23 @@ nsOggDecodeStateMachine::~nsOggDecodeStateMachine()
 
 OggPlayErrorCode nsOggDecodeStateMachine::DecodeFrame()
 {
-  return oggplay_step_decoding(mPlayer);
+  OggPlayErrorCode r = oggplay_step_decoding(mPlayer);
+  return r;
+}
+
+void nsOggDecodeStateMachine::HandleDecodeErrors(OggPlayErrorCode aErrorCode)
+{
+  PR_ASSERT_CURRENT_THREAD_IN_MONITOR(mDecoder->GetMonitor());
+
+  if (aErrorCode != E_OGGPLAY_TIMEOUT &&
+      aErrorCode != E_OGGPLAY_OK &&
+      aErrorCode != E_OGGPLAY_USER_INTERRUPT &&
+      aErrorCode != E_OGGPLAY_CONTINUE) {
+    mState = DECODER_STATE_SHUTDOWN;
+    nsCOMPtr<nsIRunnable> event =
+      NS_NEW_RUNNABLE_METHOD(nsOggDecoder, mDecoder, NetworkError);
+    NS_DispatchToMainThread(event, NS_DISPATCH_NORMAL);
+  }
 }
 
 nsOggDecodeStateMachine::FrameData* nsOggDecodeStateMachine::NextFrame()
@@ -1327,6 +1350,8 @@ nsresult nsOggDecodeStateMachine::Run()
           mon.Enter();
         }
 
+        HandleDecodeErrors(r);
+
         if (mState == DECODER_STATE_SHUTDOWN)
           continue;
 
@@ -1523,6 +1548,8 @@ nsresult nsOggDecodeStateMachine::Run()
             mon.Enter();
           } while (mState != DECODER_STATE_SHUTDOWN && r == E_OGGPLAY_TIMEOUT);
 
+          HandleDecodeErrors(r);
+
           if (mState == DECODER_STATE_SHUTDOWN)
             break;
 

media/liboggplay/README_MOZILLA

@@ -34,3 +34,6 @@ bug488951_fix_yuv: Additional fixes to YUV conversion that have not been
 
 bug488951_fix_yuv_2: Additional fix to YUV conversion for odd height videos
                      that has not been upstreamed yet.
+
+bug495129a.patch: Fix from liboggplay commit 6c8e11.
+bug495129b.patch: Fix from liboggplay commit 3602bf.

media/liboggplay/bug495129a.patch

@@ -0,0 +1,22 @@
+commit 6c8e110015c829e04d8f5fb2ac53b23f468d58ed
+Author: Viktor Gal <viktor.gal@maeth.com>
+Date:   Thu May 28 10:02:27 2009 +1000
+
+    Fix for ticket 475: fix NULL pointer dereference in oggplay_seek_cleanup function.
+    The fix proposed by Chris Double in annodex trac could cause another
+    NULL pointer dereference, in case of not enough memory for allocating new buffer.
+
+diff --git a/src/liboggplay/oggplay_seek.c b/src/liboggplay/oggplay_seek.c
+index ef150b8..7d7073d 100644
+--- a/src/liboggplay/oggplay_seek.c
++++ b/src/liboggplay/oggplay_seek.c
+@@ -104,6 +104,9 @@ oggplay_seek_cleanup(OggPlay* me, ogg_int64_t milliseconds)
+   /*
+    * store the old buffer in it next.
+    */
++  if (me->buffer == NULL)
++    return;
++  
+   trash->old_buffer = (OggPlayBuffer *)me->buffer;
+ 
+   /*

media/liboggplay/bug495129b.patch

@@ -0,0 +1,23 @@
+commit 3602bf643830c63f499928623f47ae9635a8db51
+Author: Viktor Gal <viktor.gal@maeth.com>
+Date:   Tue Apr 14 17:20:24 2009 +1000
+
+    Fix for Mozilla 481933.
+    
+    WARNING:
+    You will need the patch of 38b6dffb5ec8b32119704bd048d722a281d9fd79
+    in oggz repository, otherwise you won't be able to play back chopped Ogg content!
+
+diff --git a/src/liboggplay/oggplay.c b/src/liboggplay/oggplay.c
+index 3296fea..0b03a3a 100644
+--- a/src/liboggplay/oggplay.c
++++ b/src/liboggplay/oggplay.c
+@@ -687,6 +687,8 @@ read_more_data:
+       }
+ 
+       return E_OGGPLAY_OK;
++    } else if (r == OGGZ_ERR_HOLE_IN_DATA) {
++      return E_OGGPLAY_BAD_INPUT;
+     }
+ 
+   }

media/liboggplay/src/liboggplay/oggplay.c

@@ -716,6 +716,8 @@ read_more_data:
       }
 
       return E_OGGPLAY_OK;
+    } else if (r == OGGZ_ERR_HOLE_IN_DATA) {
+      return E_OGGPLAY_BAD_INPUT;
     }
 
   }

media/liboggplay/src/liboggplay/oggplay_seek.c

@@ -155,6 +155,9 @@ oggplay_seek_cleanup(OggPlay* me, ogg_int64_t milliseconds)
   /*
    * store the old buffer in it next.
    */
+  if (me->buffer == NULL)
+    return;
+  
   trash->old_buffer = (OggPlayBuffer *)me->buffer;
 
   /*

media/liboggplay/update.sh

@@ -56,3 +56,5 @@ patch -p3 < seek_to_key_frame.patch
 patch -p3 < bug488951.patch
 patch -p3 < bug488951_yuv_fix.patch
 patch -p3 < bug488951_yuv_fix_2.patch
+patch -p3 < bug495129a.patch
+patch -p3 < bug495129b.patch