Merge "Capture MSKPs from RE more reliably" into sc-dev

4 files changed, 42 insertions, 54 deletions

diff --git a/libs/renderengine/include/renderengine/RenderEngine.h b/libs/renderengine/include/renderengine/RenderEngine.h
index ddaa7c7783..d1bbcc55a9 100644
--- a/libs/renderengine/include/renderengine/RenderEngine.h
+++ b/libs/renderengine/include/renderengine/RenderEngine.h
@@ -43,6 +43,11 @@
  */
 #define PROPERTY_DEBUG_RENDERENGINE_CAPTURE_SKIA_MS "debug.renderengine.capture_skia_ms"
 
+/**
+ * Set to the most recently saved file once the capture is finished.
+ */
+#define PROPERTY_DEBUG_RENDERENGINE_CAPTURE_FILENAME "debug.renderengine.capture_filename"
+
 struct ANativeWindowBuffer;
 
 namespace android {
diff --git a/libs/renderengine/skia/debug/SkiaCapture.cpp b/libs/renderengine/skia/debug/SkiaCapture.cpp
index 40f5cf299d..856fff4b01 100644
--- a/libs/renderengine/skia/debug/SkiaCapture.cpp
+++ b/libs/renderengine/skia/debug/SkiaCapture.cpp
@@ -34,7 +34,7 @@ namespace renderengine {
 namespace skia {
 
 // The root of the filename to write a recorded SKP to. In order for this file to
-// be written to /data/user/, user must run 'adb shell setenforce 0' in the device.
+// be written to /data/user/, user must run 'adb shell setenforce 0' on the device.
 static const std::string CAPTURED_FILENAME_BASE = "/data/user/re_skiacapture";
 
 SkiaCapture::~SkiaCapture() {
@@ -152,11 +152,12 @@ void SkiaCapture::writeToFile() {
     // a smart pointer makes the lambda non-copyable. The lambda is only called
     // once, so this is safe.
     SkFILEWStream* stream = mOpenMultiPicStream.release();
-    CommonPool::post([doc = std::move(mMultiPic), stream] {
+    CommonPool::post([doc = std::move(mMultiPic), stream, name = std::move(mCaptureFile)] {
         ALOGD("Finalizing multi frame SKP");
         doc->close();
         delete stream;
-        ALOGD("Multi frame SKP complete.");
+        ALOGD("Multi frame SKP saved to %s.", name.c_str());
+        base::SetProperty(PROPERTY_DEBUG_RENDERENGINE_CAPTURE_FILENAME, name);
     });
     mCaptureRunning = false;
 }
@@ -164,12 +165,14 @@ void SkiaCapture::writeToFile() {
 bool SkiaCapture::setupMultiFrameCapture() {
     ATRACE_CALL();
     ALOGD("Set up multi-frame capture, ms = %llu", mTimerInterval.count());
+    base::SetProperty(PROPERTY_DEBUG_RENDERENGINE_CAPTURE_FILENAME, "");
+    const std::scoped_lock lock(mMutex);
 
-    std::string captureFile;
     // Attach a timestamp to the file.
-    base::StringAppendF(&captureFile, "%s_%lld.mskp", CAPTURED_FILENAME_BASE.c_str(),
+    mCaptureFile.clear();
+    base::StringAppendF(&mCaptureFile, "%s_%lld.mskp", CAPTURED_FILENAME_BASE.c_str(),
                         std::chrono::steady_clock::now().time_since_epoch().count());
-    auto stream = std::make_unique<SkFILEWStream>(captureFile.c_str());
+    auto stream = std::make_unique<SkFILEWStream>(mCaptureFile.c_str());
     // We own this stream and need to hold it until close() finishes.
     if (stream->isValid()) {
         mOpenMultiPicStream = std::move(stream);
@@ -194,7 +197,7 @@ bool SkiaCapture::setupMultiFrameCapture() {
         mCaptureRunning = true;
         return true;
     } else {
-        ALOGE("Could not open \"%s\" for writing.", captureFile.c_str());
+        ALOGE("Could not open \"%s\" for writing.", mCaptureFile.c_str());
         return false;
     }
 }
diff --git a/libs/renderengine/skia/debug/SkiaCapture.h b/libs/renderengine/skia/debug/SkiaCapture.h
index 5e18e60f93..f1946290ca 100644
--- a/libs/renderengine/skia/debug/SkiaCapture.h
+++ b/libs/renderengine/skia/debug/SkiaCapture.h
@@ -85,6 +85,8 @@ private:
     // Mutex to ensure that a frame in progress when the timer fires is allowed to run to
     // completion before we write the file to disk.
     std::mutex mMutex;
+
+    std::string mCaptureFile;
 };
 
 } // namespace skia
diff --git a/libs/renderengine/skia/debug/record.sh b/libs/renderengine/skia/debug/record.sh
index 25c8cefd7d..e99b7ae390 100755
--- a/libs/renderengine/skia/debug/record.sh
+++ b/libs/renderengine/skia/debug/record.sh
@@ -16,14 +16,22 @@ elif [ "$1" == "rootandsetup" ]; then
   # first time use requires these changes
   adb root
   adb shell setenforce 0
-  adb shell setprop debug.renderengine.backend "skiagl"
+  adb shell setprop debug.renderengine.backend "skiaglthreaded"
   adb shell stop
   adb shell start
   exit 1;
 fi
 
-# name of the newest file in /data/user/ before starting
-oldname=$(adb shell ls -cr /data/user/ | head -n 1)
+check_permission() {
+    adb shell getenforce
+}
+
+mode=$(check_permission)
+
+if [ "$mode" != "Permissive" ]; then
+   echo "Cannot write to disk from RenderEngine. run 'record.sh rootandsetup'"
+   exit 5
+fi
 
 # record frames for some number of milliseconds.
 adb shell setprop debug.renderengine.capture_skia_ms $1
@@ -38,26 +46,6 @@ sleep $(($1 / 1000 + 4));
 # the process it is recording.
 # /data/user/re_skiacapture_56204430551705.mskp
 
-# list the files here from newest to oldest, keep only the name of the newest.
-name=$(adb shell ls -cr /data/user/ | head -n 1)
-remote_path=/data/user/$name
-
-if [[ $oldname = $name ]]; then
-  echo "No new file written, probably no RenderEngine activity during recording period."
-  exit 1
-fi
-
-# return the size of a file in bytes
-adb_filesize() {
-    adb shell "wc -c \"$1\"" 2> /dev/null | awk '{print $1}'
-}
-
-mskp_size=$(adb_filesize "/data/user/$name")
-if [[ $mskp_size = "0" ]]; then
-  echo "File opened, but remains empty after recording period + wait. Either there was no RenderEngine activity during recording period, or recording process is still working. Check /data/user/$name manually later."
-  exit 1
-fi
-
 spin() {
     case "$spin" in
          1) printf '\b|';;
@@ -69,38 +57,28 @@ spin() {
     sleep $1
 }
 
-printf "MSKP captured, Waiting for file serialization to finish.\n"
-
-local_path=~/Downloads/$name
+local_path=~/Downloads/
 
-# wait for the file size to stop changing
+get_filename() {
+    adb shell getprop debug.renderengine.capture_filename
+}
 
-timeout=$(( $(date +%s) + 300))
-last_size='0' # output of last size check command
-unstable=true # false once the file size stops changing
-counter=0 # used to perform size check only 1/sec though we update spinner 20/sec
-# loop until the file size is unchanged for 1 second.
-while [ $unstable != 0 ] ; do
+remote_path=""
+counter=0 # used to check only 1/sec though we update spinner 20/sec
+while [ -z $remote_path ] ; do
     spin 0.05
     counter=$(( $counter+1 ))
     if ! (( $counter % 20)) ; then
-        new_size=$(adb_filesize "$remote_path")
-        unstable=$(($new_size != $last_size))
-        last_size=$new_size
-    fi
-    if [ $(date +%s) -gt $timeout ] ; then
-        printf '\bTimed out.\n'
-        exit 3
+        remote_path=$(get_filename)
     fi
 done
 printf '\b'
 
-printf "MSKP file serialized: %s\n" $(echo $last_size | numfmt --to=iec)
+printf "MSKP file serialized to: $remote_path\n"
+
+adb_pull_cmd="adb pull $remote_path $local_path"
+echo $adb_pull_cmd
+$adb_pull_cmd
 
-adb pull "$remote_path" "$local_path"
-if ! [ -f "$local_path" ] ; then
-    printf "something went wrong with `adb pull`."
-    exit 4
-fi
 adb shell rm "$remote_path"
-printf 'SKP saved to %s\n\n' "$local_path"
\ No newline at end of file
+printf 'SKP saved to %s\n\n' "$local_path"