screenrecord fixes

Fixes to issues identified during code review.

Change-Id: I2203694acb5c0544878f64f4347d29ad1a0725c4

5 files changed, 32 insertions, 12 deletions

diff --git a/cmds/screenrecord/Android.mk b/cmds/screenrecord/Android.mk
index 17523c3..d77fdb6 100644
--- a/cmds/screenrecord/Android.mk
+++ b/cmds/screenrecord/Android.mk
@@ -34,6 +34,7 @@ LOCAL_C_INCLUDES := \
 	external/jpeg
 
 LOCAL_CFLAGS += -Wno-multichar
+#LOCAL_CFLAGS += -UNDEBUG
 
 LOCAL_MODULE_TAGS := optional
 
diff --git a/cmds/screenrecord/Overlay.cpp b/cmds/screenrecord/Overlay.cpp
index f2d8b59..96e25b8 100644
--- a/cmds/screenrecord/Overlay.cpp
+++ b/cmds/screenrecord/Overlay.cpp
@@ -28,6 +28,7 @@
 #include <GLES2/gl2ext.h>
 
 #include <stdlib.h>
+#include <assert.h>
 
 #include "screenrecord.h"
 #include "Overlay.h"
@@ -66,10 +67,11 @@ status_t Overlay::start(const sp<IGraphicBufferProducer>& outputSurface,
     mStartMonotonicNsecs = systemTime(CLOCK_MONOTONIC);
     mStartRealtimeNsecs = systemTime(CLOCK_REALTIME);
 
+    Mutex::Autolock _l(mMutex);
+
     // Start the thread.  Traffic begins immediately.
     run("overlay");
 
-    Mutex::Autolock _l(mMutex);
     mState = INIT;
     while (mState == INIT) {
         mStartCond.wait(mMutex);
@@ -79,7 +81,7 @@ status_t Overlay::start(const sp<IGraphicBufferProducer>& outputSurface,
         ALOGE("Failed to start overlay thread: err=%d", mThreadResult);
         return mThreadResult;
     }
-    assert(mState == READY);
+    assert(mState == RUNNING);
 
     ALOGV("Overlay::start successful");
     *pBufferProducer = mBufferQueue;
diff --git a/cmds/screenrecord/TextRenderer.cpp b/cmds/screenrecord/TextRenderer.cpp
index 048d382..784055c 100644
--- a/cmds/screenrecord/TextRenderer.cpp
+++ b/cmds/screenrecord/TextRenderer.cpp
@@ -102,8 +102,9 @@ status_t TextRenderer::loadIntoTexture() {
     }
 
     uint32_t potHeight = powerOfTwoCeil(FontBitmap::height);
-    uint32_t* rgbaPixels = new uint32_t[FontBitmap::width * potHeight];
+    uint8_t* rgbaPixels = new uint8_t[FontBitmap::width * potHeight * 4];
     memset(rgbaPixels, 0, FontBitmap::width * potHeight * 4);
+    uint8_t* pix = rgbaPixels;
 
     for (unsigned int i = 0; i < FontBitmap::width * FontBitmap::height; i++) {
         uint8_t alpha, color;
@@ -116,7 +117,10 @@ status_t TextRenderer::loadIntoTexture() {
             color = FontBitmap::pixels[i] & ~1;
             alpha = 0xff;
         }
-        rgbaPixels[i] = (alpha << 24) | (color << 16) | (color << 8) | color;
+        *pix++ = color;
+        *pix++ = color;
+        *pix++ = color;
+        *pix++ = alpha;
     }
 
     glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, FontBitmap::width, potHeight, 0,
@@ -151,11 +155,20 @@ float TextRenderer::computeScaledStringWidth(const String8& str8) const {
     return computeScaledStringWidth(str, strlen(str));
 }
 
+size_t TextRenderer::glyphIndex(char ch) const {
+    size_t chi = ch - FontBitmap::firstGlyphChar;
+    if (chi >= FontBitmap::numGlyphs) {
+        chi = '?' - FontBitmap::firstGlyphChar;
+    }
+    assert(chi < FontBitmap::numGlyphs);
+    return chi;
+}
+
 float TextRenderer::computeScaledStringWidth(const char* str,
         size_t len) const {
     float width = 0.0f;
     for (size_t i = 0; i < len; i++) {
-        size_t chi = str[i] - FontBitmap::firstGlyphChar;
+        size_t chi = glyphIndex(str[i]);
         float glyphWidth = FontBitmap::glyphWidth[chi];
         width += (glyphWidth - 1 - FontBitmap::outlineWidth) * mScale;
     }
@@ -182,11 +195,7 @@ void TextRenderer::drawString(const Program& program, const float* texMatrix,
     float fullTexWidth = FontBitmap::width;
     float fullTexHeight = powerOfTwoCeil(FontBitmap::height);
     for (size_t i = 0; i < len; i++) {
-        size_t chi = str[i] - FontBitmap::firstGlyphChar;
-        if (chi >= FontBitmap::numGlyphs) {
-            chi = '?' - FontBitmap::firstGlyphChar;
-            assert(chi < FontBitmap::numGlyphs);
-        }
+        size_t chi = glyphIndex(str[i]);
         float glyphWidth = FontBitmap::glyphWidth[chi];
         float glyphHeight = FontBitmap::maxGlyphHeight;
 
diff --git a/cmds/screenrecord/TextRenderer.h b/cmds/screenrecord/TextRenderer.h
index 9a28fcb..03dd2fb 100644
--- a/cmds/screenrecord/TextRenderer.h
+++ b/cmds/screenrecord/TextRenderer.h
@@ -109,6 +109,10 @@ private:
     // Like getGlyphHeight(), but result is scaled.
     float getScaledGlyphHeight() const { return getGlyphHeight() * mScale; }
 
+    // Convert an ASCII character to a glyph index.  Returns the glyph for
+    // '?' if we have no glyph for the specified character.
+    size_t glyphIndex(char ch) const;
+
     GLuint mTextureName;
     float mScale;
 
diff --git a/cmds/screenrecord/screenrecord.cpp b/cmds/screenrecord/screenrecord.cpp
index b13333c..a6652f4 100644
--- a/cmds/screenrecord/screenrecord.cpp
+++ b/cmds/screenrecord/screenrecord.cpp
@@ -45,6 +45,7 @@
 #include <signal.h>
 #include <getopt.h>
 #include <sys/wait.h>
+#include <assert.h>
 
 #include "screenrecord.h"
 #include "Overlay.h"
@@ -532,9 +533,10 @@ static status_t recordScreen(const char* fileName) {
 
     // Configure optional overlay.
     sp<IGraphicBufferProducer> bufferProducer;
-    sp<Overlay> overlay = new Overlay();
+    sp<Overlay> overlay;
     if (gWantFrameTime) {
         // Send virtual display frames to an external texture.
+        overlay = new Overlay();
         err = overlay->start(encoderInputSurface, &bufferProducer);
         if (err != NO_ERROR) {
             encoder->release();
@@ -578,7 +580,9 @@ static status_t recordScreen(const char* fileName) {
     // Shut everything down, starting with the producer side.
     encoderInputSurface = NULL;
     SurfaceComposerClient::destroyDisplay(dpy);
-    overlay->stop();
+    if (overlay != NULL) {
+        overlay->stop();
+    }
     encoder->stop();
     // If we don't stop muxer explicitly, i.e. let the destructor run,
     // it may hang (b/11050628).