commit 95ba0164b3fd9b2a64870026913ea6c71b11e5f6
author Wonsik Kim <wonsik@google.com> Tue Mar 19 15:51:54 2019 -0700
committer Wonsik Kim <wonsik@google.com> Tue Mar 19 15:53:49 2019 -0700
tree 691894e9b969c1a2c9b773e5d5a00207e8631669
parent 4579c809566bd682aec2f9351f60d9ddb2dd677f

CCodec: fix MAX_PTS_GAP behavior

- Fix MAX_PTS_GAP calculation to avoid undefined behavior.
- Restore input timestamp for input surface use case.

Bug: 128934821
Test: atest CtsMediaTestCases:SurfaceEncodeTimestampTest
Test: atest CtsMediaTestCases -- --module-arg CtsMediaTestCases:size:small
Change-Id: I381d9c5e2d097961388c04d7ef5e8aacd51624a2

media/codec2/sfplugin/CCodec.cpp

diff --git a/media/codec2/sfplugin/CCodec.cpp b/media/codec2/sfplugin/CCodec.cpp
index 2d10c67..5f60378 100644
--- a/media/codec2/sfplugin/CCodec.cpp
+++ b/media/codec2/sfplugin/CCodec.cpp
@@ -271,9 +271,12 @@
             if (mNode != nullptr) {
                 OMX_PARAM_U32TYPE ptrGapParam = {};
                 ptrGapParam.nSize = sizeof(OMX_PARAM_U32TYPE);
-                ptrGapParam.nU32 = (config.mMinAdjustedFps > 0)
+                float gap = (config.mMinAdjustedFps > 0)
                         ? c2_min(INT32_MAX + 0., 1e6 / config.mMinAdjustedFps + 0.5)
                         : c2_max(0. - INT32_MAX, -1e6 / config.mFixedAdjustedFps - 0.5);
+                // float -> uint32_t is undefined if the value is negative.
+                // First convert to int32_t to ensure the expected behavior.
+                ptrGapParam.nU32 = int32_t(gap);
                 (void)mNode->setParameter(
                         (OMX_INDEXTYPE)OMX_IndexParamMaxFrameDurationForBitrateControl,
                         &ptrGapParam, sizeof(ptrGapParam));
@@ -282,7 +285,7 @@
 
         // max fps
         // TRICKY: we do not unset max fps to 0 unless using fixed fps
-        if ((config.mMaxFps > 0 || (config.mFixedAdjustedFps > 0 && config.mMaxFps == 0))
+        if ((config.mMaxFps > 0 || (config.mFixedAdjustedFps > 0 && config.mMaxFps == -1))
                 && config.mMaxFps != mConfig.mMaxFps) {
             status_t res = GetStatus(mSource->setMaxFps(config.mMaxFps));
             status << " maxFps=" << config.mMaxFps;
@@ -764,13 +767,16 @@
                 if (msg->findInt64(KEY_REPEAT_PREVIOUS_FRAME_AFTER, &value) && value > 0) {
                     config->mISConfig->mMinFps = 1e6 / value;
                 }
-                (void)msg->findFloat(
-                        KEY_MAX_FPS_TO_ENCODER, &config->mISConfig->mMaxFps);
+                if (!msg->findFloat(
+                        KEY_MAX_FPS_TO_ENCODER, &config->mISConfig->mMaxFps)) {
+                    config->mISConfig->mMaxFps = -1;
+                }
                 config->mISConfig->mMinAdjustedFps = 0;
                 config->mISConfig->mFixedAdjustedFps = 0;
                 if (msg->findInt64(KEY_MAX_PTS_GAP_TO_ENCODER, &value)) {
                     if (value < 0 && value >= INT32_MIN) {
                         config->mISConfig->mFixedAdjustedFps = -1e6 / value;
+                        config->mISConfig->mMaxFps = -1;
                     } else if (value > 0 && value <= INT32_MAX) {
                         config->mISConfig->mMinAdjustedFps = 1e6 / value;
                     }