Conversation
| ifeq ($(strip $(BOARD_USE_MOTO_SF)), true) | ||
| USE_MOTO_SF = 1 | ||
| else | ||
| USE_MOTO_SF = 0 |
There was a problem hiding this comment.
Random observation: looks like this USE_MOTO_SF variable is not used for anything
There was a problem hiding this comment.
There was a problem hiding this comment.
Oh wait i see what you mean, yeah you're right
There was a problem hiding this comment.
Also there's no need for this check. Please check the MTK hack that I added for Audiopolicy.
You can do:
ifeq ($(FOO),1)
LOCAL_CPP_FLAGS=-DFOO
endif
Please change.
Thaodan
left a comment
Thaodan
left a comment
There was a problem hiding this comment.
Please add the comment that explains the need for the change to the commit message, not just to the PR comment.
Further you should follow the existing conventions for hacks/workarounds
the variables should be called _<descriptive_name_of_hack> e.g. in this case SURFACEFLINGER_MOTO_INITIALIZE_DISPLAY.
You can also add the description to Android.mk so it can be understood when building
droidmedia without checking git first.
The hack should be also included in the spec file just copy the section for the mtk hack and rename it for your hack.
By doing so they hack will be off by default but can be enabled if so requested.
For further see my comments.
|
|
||
| LOCAL_CLANG := true | ||
|
|
||
| LOCAL_C_INCLUDES := frameworks/native/services/surfaceflinger |
There was a problem hiding this comment.
The LOCAL_C_INCLUDES modified should be inside the section specific to the hack.
| #ifdef USE_MOTO_SF | ||
| ProcessState::self()->setThreadPoolMaxThreadCount(4); | ||
| ALOGI("MOTO_SF SurfaceFlinger: start the thread pool"); | ||
| // start the thread pool |
There was a problem hiding this comment.
I think these should be ALOGD?
| ifeq ($(strip $(BOARD_USE_MOTO_SF)), true) | ||
| USE_MOTO_SF = 1 | ||
| else | ||
| USE_MOTO_SF = 0 |
There was a problem hiding this comment.
Also there's no need for this check. Please check the MTK hack that I added for Audiopolicy.
You can do:
ifeq ($(FOO),1)
LOCAL_CPP_FLAGS=-DFOO
endif
Please change.
| # build minisfservice executable | ||
| include $(CLEAR_VARS) | ||
|
|
||
| LOCAL_CLANG := true |
There was a problem hiding this comment.
Why was this required? It shouldn't be needed.
| include $(BUILD_EXECUTABLE) | ||
|
|
||
| ############################################################### | ||
| # build minisfservice executable |
There was a problem hiding this comment.
No need to add comments here, limit your change to stay on topic.
|
Will do will do |
3 participants
In motorola moto g2 to get display working, we had to first run surfaceflinger to "initialize" the display. This hack originally done by @guhl fixes that via minisf service and it would be nice to get it merged