From 0b06a3bc82b8525c357d32b504fe8fd2e183232a Mon Sep 17 00:00:00 2001
From: Andrew Johnson <ajohnson@draster.com>
Date: Thu, 20 Sep 2018 08:23:32 -0400
Subject: [PATCH] Bug 1489757 - Bug 1448863 causes video streams to take a very
 long time to recover from packet loss; r=bwc

This patch sets mDecoderStatus from the GMPThread so we can eventually report
an error back to the caller. Since this done during an asynchronous call, there
is no guarantee that the error will be associated with the correct frame, but
this workaround should eventually cause an error to be signalled, so that a
PLI can be requested and video will not freeze.

--HG--
extra : rebase_source : 2c32de4218b97ce1a47c5ec118cc864fff786060
---
 .../src/media-conduit/WebrtcGmpVideoCodec.cpp | 35 ++++++++++++++++---
 .../src/media-conduit/WebrtcGmpVideoCodec.h   |  2 +-
 2 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/media/webrtc/signaling/src/media-conduit/WebrtcGmpVideoCodec.cpp b/media/webrtc/signaling/src/media-conduit/WebrtcGmpVideoCodec.cpp
index d61ce978a520..2307dffac629 100644
--- a/media/webrtc/signaling/src/media-conduit/WebrtcGmpVideoCodec.cpp
+++ b/media/webrtc/signaling/src/media-conduit/WebrtcGmpVideoCodec.cpp
@@ -789,6 +789,17 @@ WebrtcGmpVideoDecoder::GmpInitDone(GMPVideoDecoderProxy* aGMP,
     }
   }
 
+  // This is an ugly solution to asynchronous decoding errors
+  // from Decode_g() not being returned to the synchronous Decode() method.
+  // If we don't return an error code at this point, our caller ultimately won't know to request
+  // a PLI and the video stream will remain frozen unless an IDR happens to arrive for other reasons.
+  // Bug 1492852 tracks implementing a proper solution.
+  if(mDecoderStatus != GMPNoErr){
+    LOG(LogLevel::Error, ("%s: Decoder status is bad (%u)!",
+          __PRETTY_FUNCTION__, static_cast<unsigned>(mDecoderStatus)));
+    return WEBRTC_VIDEO_CODEC_ERROR;
+  }
+
   return WEBRTC_VIDEO_CODEC_OK;
 }
 
@@ -819,6 +830,11 @@ WebrtcGmpVideoDecoder::Decode(const webrtc::EncodedImage& aInputImage,
     return WEBRTC_VIDEO_CODEC_ERROR;
   }
 
+  // This is an ugly solution to asynchronous decoding errors
+  // from Decode_g() not being returned to the synchronous Decode() method.
+  // If we don't return an error code at this point, our caller ultimately won't know to request
+  // a PLI and the video stream will remain frozen unless an IDR happens to arrive for other reasons.
+  // Bug 1492852 tracks implementing a proper solution.
   nsAutoPtr<GMPDecodeData> decodeData(new GMPDecodeData(aInputImage,
                                                         aMissingFrames,
                                                         aRenderTimeMs));
@@ -828,6 +844,12 @@ WebrtcGmpVideoDecoder::Decode(const webrtc::EncodedImage& aInputImage,
                                       decodeData),
                        NS_DISPATCH_NORMAL);
 
+  if(mDecoderStatus != GMPNoErr){
+    LOG(LogLevel::Error, ("%s: Decoder status is bad (%u)!",
+          __PRETTY_FUNCTION__, static_cast<unsigned>(mDecoderStatus)));
+    return WEBRTC_VIDEO_CODEC_ERROR;
+  }
+
   return WEBRTC_VIDEO_CODEC_OK;
 }
 
@@ -845,6 +867,8 @@ WebrtcGmpVideoDecoder::Decode_g(const RefPtr<WebrtcGmpVideoDecoder>& aThis,
     }
     // destroyed via Terminate(), failed to init, or just not initted yet
     LOGD(("GMP Decode: not initted yet"));
+
+    aThis->mDecoderStatus = GMPDecodeErr;
     return;
   }
 
@@ -856,6 +880,7 @@ WebrtcGmpVideoDecoder::Decode_g(const RefPtr<WebrtcGmpVideoDecoder>& aThis,
   if (err != GMPNoErr) {
     LOG(LogLevel::Error, ("%s: CreateFrame failed (%u)!",
         __PRETTY_FUNCTION__, static_cast<unsigned>(err)));
+    aThis->mDecoderStatus = err;
     return;
   }
 
@@ -864,6 +889,7 @@ WebrtcGmpVideoDecoder::Decode_g(const RefPtr<WebrtcGmpVideoDecoder>& aThis,
   if (err != GMPNoErr) {
     LOG(LogLevel::Error, ("%s: CreateEmptyFrame failed (%u)!",
         __PRETTY_FUNCTION__, static_cast<unsigned>(err)));
+    aThis->mDecoderStatus = err;
     return;
   }
 
@@ -885,6 +911,7 @@ WebrtcGmpVideoDecoder::Decode_g(const RefPtr<WebrtcGmpVideoDecoder>& aThis,
   if (ret != WEBRTC_VIDEO_CODEC_OK) {
     LOG(LogLevel::Error, ("%s: WebrtcFrameTypeToGmpFrameType failed (%u)!",
         __PRETTY_FUNCTION__, static_cast<unsigned>(ret)));
+    aThis->mDecoderStatus = GMPDecodeErr;
     return;
   }
 
@@ -906,13 +933,11 @@ WebrtcGmpVideoDecoder::Decode_g(const RefPtr<WebrtcGmpVideoDecoder>& aThis,
   if (NS_FAILED(rv)) {
     LOG(LogLevel::Error, ("%s: Decode failed (rv=%u)!",
         __PRETTY_FUNCTION__, static_cast<unsigned>(rv)));
+    aThis->mDecoderStatus = GMPDecodeErr;
+    return;
   }
 
-  if(aThis->mDecoderStatus != GMPNoErr){
-    aThis->mDecoderStatus = GMPNoErr;
-    LOG(LogLevel::Error, ("%s: Decoder status is bad (%u)!",
-        __PRETTY_FUNCTION__, static_cast<unsigned>(aThis->mDecoderStatus)));
-  }
+  aThis->mDecoderStatus = GMPNoErr;
 }
 
 int32_t
diff --git a/media/webrtc/signaling/src/media-conduit/WebrtcGmpVideoCodec.h b/media/webrtc/signaling/src/media-conduit/WebrtcGmpVideoCodec.h
index 92b5a270267e..f675a62e65f3 100644
--- a/media/webrtc/signaling/src/media-conduit/WebrtcGmpVideoCodec.h
+++ b/media/webrtc/signaling/src/media-conduit/WebrtcGmpVideoCodec.h
@@ -484,7 +484,7 @@ private:
   Mutex mCallbackMutex;
   webrtc::DecodedImageCallback* mCallback;
   Atomic<uint64_t> mCachedPluginId;
-  GMPErr mDecoderStatus;
+  Atomic<GMPErr, ReleaseAcquire> mDecoderStatus;
   std::string mPCHandle;
 };