goheif icon indicating copy to clipboard operation
goheif copied to clipboard

Use git submodules for external code

Open rigon opened this issue 3 years ago • 1 comments

The source code of libde265 is included in the project. I believe this is not the best practice. I think doing the following steps a better approach:

cd libde265
rm -rf libde265
git commit -am "libde265 removed"
git submodule add https://github.com/strukturag/libde265.git

rigon avatar Jan 28 '22 13:01 rigon

I actually spent some time on this, and the rabbit hole goes deeper than you would expect. Here's what I've gleaned:

  • The libde265 repo uses CMake to generate intermediate files based on the target platform.
  • The goheif repo has made enough of these changes directly to the source to allow the build to pass.
  • A SWIG wrapper may be the way to go for automated integration of a CMake-based C++ library into a Go library, but SWIG usage is not simple.

Here are the differences between https://github.com/strukturag/libde265/tree/v1.0.8/libde265 and https://github.com/adrium/goheif/tree/master/libde265/libde265:

diff --git a/libde265/cabac.cc b/libde265/cabac.cc
index 102bc57e..6ea5b5ba 100644
--- a/libde265/cabac.cc
+++ b/libde265/cabac.cc
@@ -416,8 +416,6 @@ int  decode_CABAC_TR_bypass(CABAC_decoder* decoder, int cRiceParam, int cTRMax)
 }


-#define MAX_PREFIX 32
-
 int  decode_CABAC_EGk_bypass(CABAC_decoder* decoder, int k)
 {
   int base=0;
@@ -433,7 +431,7 @@ int  decode_CABAC_EGk_bypass(CABAC_decoder* decoder, int k)
         n++;
       }

-      if (n == k+MAX_PREFIX) {
+      if (n == k+32) {
         return 0; // TODO: error
       }
     }
diff --git a/libde265/fallback-motion.cc b/libde265/fallback-motion.cc
index 1ac41da4..3a386e23 100644
--- a/libde265/fallback-motion.cc
+++ b/libde265/fallback-motion.cc
@@ -475,8 +475,8 @@ void put_qpel_0_0_fallback_16(int16_t *out, ptrdiff_t out_stride,



-static int extra_before[4] = { 0,3,3,2 };
-static int extra_after [4] = { 0,3,4,4 };
+static int fallback_extra_before[4] = { 0,3,3,2 };
+static int fallback_extra_after [4] = { 0,3,4,4 };

 template <class pixel_t>
 void put_qpel_fallback(int16_t *out, ptrdiff_t out_stride,
@@ -484,10 +484,10 @@ void put_qpel_fallback(int16_t *out, ptrdiff_t out_stride,
                        int nPbW, int nPbH, int16_t* mcbuffer,
                        int xFracL, int yFracL, int bit_depth)
 {
-  int extra_left   = extra_before[xFracL];
-  //int extra_right  = extra_after [xFracL];
-  int extra_top    = extra_before[yFracL];
-  int extra_bottom = extra_after [yFracL];
+  int extra_left   = fallback_extra_before[xFracL];
+  //int extra_right  = fallback_extra_after [xFracL];
+  int extra_top    = fallback_extra_before[yFracL];
+  int extra_bottom = fallback_extra_after [yFracL];

   //int nPbW_extra = extra_left + nPbW + extra_right;
   int nPbH_extra = extra_top  + nPbH + extra_bottom;
diff --git a/libde265/slice.cc b/libde265/slice.cc
index e85ecc61..5e8a6f8e 100644
--- a/libde265/slice.cc
+++ b/libde265/slice.cc
@@ -2444,8 +2444,6 @@ static int decode_coeff_abs_level_greater2(thread_context* tctx,
 }


-#define MAX_PREFIX 64
-
 static int decode_coeff_abs_level_remaining(thread_context* tctx,
                                             int cRiceParam)
 {
@@ -2457,7 +2455,7 @@ static int decode_coeff_abs_level_remaining(thread_context* tctx,
     prefix++;
     codeword = decode_CABAC_bypass(&tctx->cabac_decoder);

-    if (prefix>MAX_PREFIX) {
+    if (prefix>64) {
       return 0; // TODO: error
     }
   }

tagatac avatar Jul 13 '22 18:07 tagatac