Updated fix for mozbz#1645671

2 files changed, 53 insertions, 77 deletions

diff --git a/firefox.spec b/firefox.spec
index a88d11b..64f09cb 100644
--- a/firefox.spec
+++ b/firefox.spec
@@ -114,7 +114,7 @@ ExcludeArch: s390x
 Summary:        Mozilla Firefox Web browser
 Name:           firefox
 Version:        79.0
-Release:        3%{?dist}
+Release:        4%{?dist}
 URL:            https://www.mozilla.org/firefox/
 License:        MPLv1.1 or GPLv2+ or LGPLv2+
 Source0:        https://archive.mozilla.org/pub/firefox/releases/%{version}%{?pre_version}/source/firefox-%{version}%{?pre_version}.source.tar.xz
@@ -958,7 +958,10 @@ gtk-update-icon-cache %{_datadir}/icons/hicolor &>/dev/null || :
 #---------------------------------------------------------------------
 
 %changelog
-* Tue Jul 30 2020 Martin Stransky <stransky@redhat.com> - 79.0-3
+* Mon Aug 1 2020 Martin Stransky <stransky@redhat.com> - 79.0-4
+- Updated fix for mozbz#1645671
+
+* Thu Jul 30 2020 Martin Stransky <stransky@redhat.com> - 79.0-3
 - Added VA-API fix for mozbz#1645671
 
 * Wed Jul 29 2020 Martin Stransky <stransky@redhat.com> - 79.0-2
diff --git a/mozilla-1645671.patch b/mozilla-1645671.patch
index 8e9e31e..0943469 100644
--- a/mozilla-1645671.patch
+++ b/mozilla-1645671.patch
@@ -1,94 +1,67 @@
-diff --git a/dom/media/platforms/ffmpeg/FFmpegVideoDecoder.h b/dom/media/platforms/ffmpeg/FFmpegVideoDecoder.h
---- a/dom/media/platforms/ffmpeg/FFmpegVideoDecoder.h
-+++ b/dom/media/platforms/ffmpeg/FFmpegVideoDecoder.h
-@@ -156,7 +156,7 @@
-                                 MediaDataDecoder::DecodedData& aResults);
- 
-   void ReleaseUnusedVAAPIFrames();
--  DMABufSurfaceWrapper* GetUnusedDMABufSurfaceWrapper();
-+  int GetUnusedDMABufSurfaceWrapperIndex();
-   void ReleaseDMABufSurfaces();
- #endif
- 
-@@ -174,7 +174,7 @@
-   const bool mDisableHardwareDecoding;
-   VADisplay mDisplay;
-   bool mUseDMABufSurfaces;
--  nsTArray<DMABufSurfaceWrapper> mDMABufSurfaces;
-+  nsTArray<UniquePtr<DMABufSurfaceWrapper>> mDMABufSurfaces;
- #endif
-   RefPtr<KnowsCompositor> mImageAllocator;
-   RefPtr<ImageContainer> mImageContainer;
+changeset:   544864:a8603f131703
+tag:         tip
+parent:      544861:161920b70ae4
+user:        Martin Stransky <stransky@redhat.com>
+date:        Fri Jul 31 13:39:48 2020 +0200
+files:       dom/media/platforms/ffmpeg/FFmpegVideoDecoder.cpp dom/media/platforms/ffmpeg/FFmpegVideoDecoder.h
+description:
+Bug 1645671 [Linux/VA-API] Create DMABufSurfaceWrapper directly at nsTAttay and disable DMABufSurfaceWrapper class copy and assignment constructors, r?jya
+
+When DMABufSurfaceWrapper is added to nsTArray, a temporary local DMABufSurfaceWrapper object is created. When the temporary
+object is deleted after the adding, associated dmabuf data is released which leads to rendering artifact during VA-API video playback.
+
+As a fix in this patch we create DMABufSurfaceWrapper 'in-place' at nsTAttay.
+We also disable DMABufSurfaceWrapper class copy and assignment constructors to avoid similar potential issues.
+
+Differential Revision: https://phabricator.services.mozilla.com/D85152
+
+
 diff --git a/dom/media/platforms/ffmpeg/FFmpegVideoDecoder.cpp b/dom/media/platforms/ffmpeg/FFmpegVideoDecoder.cpp
 --- a/dom/media/platforms/ffmpeg/FFmpegVideoDecoder.cpp
 +++ b/dom/media/platforms/ffmpeg/FFmpegVideoDecoder.cpp
-@@ -698,21 +698,20 @@
- void FFmpegVideoDecoder<LIBAV_VER>::ReleaseUnusedVAAPIFrames() {
-   int len = mDMABufSurfaces.Length();
-   for (int i = 0; i < len; i++) {
--    if (!mDMABufSurfaces[i].IsUsed()) {
--      mDMABufSurfaces[i].ReleaseVAAPIData();
-+    if (!mDMABufSurfaces[i]->IsUsed()) {
-+      mDMABufSurfaces[i]->ReleaseVAAPIData();
-     }
-   }
- }
- 
--DMABufSurfaceWrapper*
--FFmpegVideoDecoder<LIBAV_VER>::GetUnusedDMABufSurfaceWrapper() {
-+int FFmpegVideoDecoder<LIBAV_VER>::GetUnusedDMABufSurfaceWrapperIndex() {
-   int len = mDMABufSurfaces.Length();
-   for (int i = 0; i < len; i++) {
--    if (!mDMABufSurfaces[i].IsUsed()) {
--      return &(mDMABufSurfaces[i]);
-+    if (!mDMABufSurfaces[i]->IsUsed()) {
-+      return i;
-     }
-   }
--  return nullptr;
-+  return -1;
- }
- 
- void FFmpegVideoDecoder<LIBAV_VER>::ReleaseDMABufSurfaces() {
-@@ -763,8 +762,8 @@
- 
-   RefPtr<DMABufSurfaceYUV> surface;
- 
--  DMABufSurfaceWrapper* surfaceWrapper = GetUnusedDMABufSurfaceWrapper();
--  if (!surfaceWrapper) {
-+  int surfaceWrapperIndex = GetUnusedDMABufSurfaceWrapperIndex();
-+  if (surfaceWrapperIndex < 0) {
-     if (mVAAPIDeviceContext) {
-       surface = DMABufSurfaceYUV::CreateYUVSurface(vaDesc);
-     } else {
-@@ -776,16 +775,16 @@
-       return MediaResult(NS_ERROR_OUT_OF_MEMORY,
+@@ -777,17 +777,17 @@ MediaResult FFmpegVideoDecoder<LIBAV_VER
                           RESULT_DETAIL("Unable to get DMABufSurfaceYUV"));
      }
--
+ 
  #  ifdef MOZ_LOGGING
      static int uid = 0;
      surface->SetUID(++uid);
      FFMPEG_LOG("Created new DMABufSurface UID = %d", uid);
  #  endif
 -    mDMABufSurfaces.AppendElement(DMABufSurfaceWrapper(surface, mLib));
--    surfaceWrapper = &(mDMABufSurfaces[mDMABufSurfaces.Length() - 1]);
-+    mDMABufSurfaces.AppendElement(
-+        MakeUnique<DMABufSurfaceWrapper>(surface, mLib));
-+    surfaceWrapperIndex = mDMABufSurfaces.Length() - 1;
++    mDMABufSurfaces.EmplaceBack(surface, mLib);
+     surfaceWrapper = &(mDMABufSurfaces[mDMABufSurfaces.Length() - 1]);
    } else {
--    surface = surfaceWrapper->GetDMABufSurface();
-+    surface = mDMABufSurfaces[surfaceWrapperIndex]->GetDMABufSurface();
+     surface = surfaceWrapper->GetDMABufSurface();
      bool ret;
  
      if (mVAAPIDeviceContext) {
-@@ -803,7 +802,7 @@
-   }
+       ret = surface->UpdateYUVData(vaDesc);
+     } else {
+diff --git a/dom/media/platforms/ffmpeg/FFmpegVideoDecoder.h b/dom/media/platforms/ffmpeg/FFmpegVideoDecoder.h
+--- a/dom/media/platforms/ffmpeg/FFmpegVideoDecoder.h
++++ b/dom/media/platforms/ffmpeg/FFmpegVideoDecoder.h
+@@ -70,16 +70,22 @@ class DMABufSurfaceWrapper final {
+   // Check if DMABufSurface is used by any gecko rendering process
+   // (WebRender or GL compositor) or by DMABUFSurfaceImage/VideoData.
+   bool IsUsed() const { return mSurface->IsGlobalRefSet(); }
  
-   if (mVAAPIDeviceContext) {
--    surfaceWrapper->LockVAAPIData(mCodecContext, mFrame);
-+    mDMABufSurfaces[surfaceWrapperIndex]->LockVAAPIData(mCodecContext, mFrame);
+   RefPtr<DMABufSurfaceYUV> GetDMABufSurface() const {
+     return mSurface->GetAsDMABufSurfaceYUV();
    }
  
-   surface->SetYUVColorSpace(GetFrameColorSpace());
++  // Don't allow DMABufSurfaceWrapper plain copy as it leads to
++  // enexpected DMABufSurface/HW buffer releases and we don't want to
++  // deep copy them.
++  DMABufSurfaceWrapper(const DMABufSurfaceWrapper&) = delete;
++  const DMABufSurfaceWrapper& operator=(DMABufSurfaceWrapper const&) = delete;
++
+  private:
+   const RefPtr<DMABufSurface> mSurface;
+   const FFmpegLibWrapper* mLib;
+   AVBufferRef* mAVHWFramesContext;
+   AVBufferRef* mHWAVBuffer;
+ };
+ #endif
+