From c78626b15e9f29a5bcf85447ceafb17dcbf58b69 Mon Sep 17 00:00:00 2001 From: Emilian Peev Date: Mon, 21 May 2012 12:38:23 +0300 Subject: CameraHal: Avoids possible race conditions while accessing 'mParams' - Direct access of 'mParams' outside of 'get-/setParameters()' should be avoided. The underlying strings can get invalidated with each call to 'setParameters()', which can lead to instabilities. - This change also removes legacy stereo code, which is not used any more. Bug: 6509329 Change-Id: Ief6df206c33fbdc666644cea8630e0bce6a36c00 Signed-off-by: Emilian Peev --- camera/Encoder_libjpeg.cpp | 22 +++++----- camera/OMXCameraAdapter/OMXAlgo.cpp | 4 +- camera/OMXCameraAdapter/OMXCameraAdapter.cpp | 28 ++++++------ camera/OMXCameraAdapter/OMXCapture.cpp | 61 +++++++++----------------- camera/OMXCameraAdapter/OMXExif.cpp | 52 +++++++++++++--------- camera/inc/Encoder_libjpeg.h | 2 +- camera/inc/OMXCameraAdapter/OMXCameraAdapter.h | 6 +++ 7 files changed, 84 insertions(+), 91 deletions(-) diff --git a/camera/Encoder_libjpeg.cpp b/camera/Encoder_libjpeg.cpp index d50b2ea4..c7da1153 100644 --- a/camera/Encoder_libjpeg.cpp +++ b/camera/Encoder_libjpeg.cpp @@ -47,17 +47,17 @@ extern "C" { #define MIN(x,y) ((x < y) ? x : y) namespace android { -struct string_pair { - const char* string1; - const char* string2; +struct integer_string_pair { + unsigned int integer; + const char* string; }; -static string_pair degress_to_exif_lut [] = { +static integer_string_pair degress_to_exif_lut [] = { // degrees, exif_orientation - {"0", "1"}, - {"90", "6"}, - {"180", "3"}, - {"270", "8"}, + {0, "1"}, + {90, "6"}, + {180, "3"}, + {270, "8"}, }; struct libjpeg_destination_mgr : jpeg_destination_mgr { libjpeg_destination_mgr(uint8_t* input, int size); @@ -200,10 +200,10 @@ static void resize_nv12(Encoder_libjpeg::params* params, uint8_t* dst_buffer) { } /* public static functions */ -const char* ExifElementsTable::degreesToExifOrientation(const char* degrees) { +const char* ExifElementsTable::degreesToExifOrientation(unsigned int degrees) { for (unsigned int i = 0; i < ARRAY_SIZE(degress_to_exif_lut); i++) { - if (!strcmp(degrees, degress_to_exif_lut[i].string1)) { - return degress_to_exif_lut[i].string2; + if (degrees == degress_to_exif_lut[i].integer) { + return degress_to_exif_lut[i].string; } } return NULL; diff --git a/camera/OMXCameraAdapter/OMXAlgo.cpp b/camera/OMXCameraAdapter/OMXAlgo.cpp index ff68cc93..12b9058f 100644 --- a/camera/OMXCameraAdapter/OMXAlgo.cpp +++ b/camera/OMXCameraAdapter/OMXAlgo.cpp @@ -641,10 +641,10 @@ status_t OMXCameraAdapter::setCaptureMode(OMXCameraAdapter::CaptureMode mode) CAMHAL_LOGDA("Camera mode: HIGH QUALITY_ZSL"); camMode.eCamOperatingMode = OMX_TI_CaptureImageProfileZeroShutterLag; - valstr = mParams.get(TICameraParameters::KEY_RECORDING_HINT); - if (!valstr || (valstr && (strcmp(valstr, "false")))) { + if ( !mIternalRecordingHint ) { zslHistoryLen.nHistoryLen = 5; } + } else if( OMXCameraAdapter::VIDEO_MODE == mode ) { diff --git a/camera/OMXCameraAdapter/OMXCameraAdapter.cpp b/camera/OMXCameraAdapter/OMXCameraAdapter.cpp index 11ecf0ff..392de932 100755 --- a/camera/OMXCameraAdapter/OMXCameraAdapter.cpp +++ b/camera/OMXCameraAdapter/OMXCameraAdapter.cpp @@ -191,6 +191,7 @@ status_t OMXCameraAdapter::initialize(CameraProperties::Properties* caps) mZoomParameterIdx = 0; mExposureBracketingValidEntries = 0; mSensorOverclock = false; + mIternalRecordingHint = false; mDeviceOrientation = 0; mCapabilities = caps; @@ -536,6 +537,13 @@ status_t OMXCameraAdapter::setParameters(const CameraParameters ¶ms) mOMXStateSwitch = true; } + valstr = params.get(TICameraParameters::KEY_RECORDING_HINT); + if (!valstr || (valstr && (strcmp(valstr, CameraParameters::FALSE)))) { + mIternalRecordingHint = false; + } else { + mIternalRecordingHint = true; + } + #ifdef OMAP_ENHANCEMENT if ( (valstr = params.get(TICameraParameters::KEY_MEASUREMENT_ENABLE)) != NULL ) @@ -1465,13 +1473,6 @@ status_t OMXCameraAdapter::UseBuffersPreview(void* bufArr, int num) LOG_FUNCTION_NAME; - ///Flag to determine whether it is 3D camera or not - bool isS3d = false; - const char *valstr = NULL; - if ( (valstr = mParams.get(TICameraParameters::KEY_S3D_SUPPORTED)) != NULL) { - isS3d = (strcmp(valstr, "true") == 0); - } - if(!bufArr) { CAMHAL_LOGEA("NULL pointer passed for buffArr"); @@ -1531,8 +1532,7 @@ status_t OMXCameraAdapter::UseBuffersPreview(void* bufArr, int num) CAMHAL_LOGDB("Camera Mode = %d", mCapMode); - if( ( mCapMode == OMXCameraAdapter::VIDEO_MODE ) || - ( isS3d && (mCapMode == OMXCameraAdapter::HIGH_QUALITY)) ) + if( mCapMode == OMXCameraAdapter::VIDEO_MODE ) { ///Enable/Disable Video Noise Filter ret = enableVideoNoiseFilter(mVnfEnabled); @@ -3085,17 +3085,15 @@ OMX_ERRORTYPE OMXCameraAdapter::OMXCameraAdapterFillBufferDone(OMX_IN OMX_HANDLE const char *valstr = NULL; pixFormat = mCameraAdapterParameters.mCameraPortParams[mCameraAdapterParameters.mImagePortIndex].mColorFormat; - valstr = mParams.getPictureFormat(); if ( OMX_COLOR_FormatUnused == pixFormat ) { typeOfFrame = CameraFrame::IMAGE_FRAME; mask = (unsigned int) CameraFrame::IMAGE_FRAME; - } - else if ( pixFormat == OMX_COLOR_FormatCbYCrY && - ((valstr && !strcmp(valstr, CameraParameters::PIXEL_FORMAT_JPEG)) || - !valstr) ) - { + } else if ( pixFormat == OMX_COLOR_FormatCbYCrY && + ((mPictureFormatFromClient && + !strcmp(mPictureFormatFromClient, CameraParameters::PIXEL_FORMAT_JPEG)) || + !mPictureFormatFromClient) ) { // signals to callbacks that this needs to be coverted to jpeg // before returning to framework typeOfFrame = CameraFrame::IMAGE_FRAME; diff --git a/camera/OMXCameraAdapter/OMXCapture.cpp b/camera/OMXCameraAdapter/OMXCapture.cpp index 93a0dae5..6ad5f88e 100644 --- a/camera/OMXCameraAdapter/OMXCapture.cpp +++ b/camera/OMXCameraAdapter/OMXCapture.cpp @@ -63,69 +63,48 @@ status_t OMXCameraAdapter::setParametersCapture(const CameraParameters ¶ms, CAMHAL_LOGVB("Image: cap.mWidth = %d", (int)cap->mWidth); CAMHAL_LOGVB("Image: cap.mHeight = %d", (int)cap->mHeight); - if ( (valstr = params.getPictureFormat()) != NULL ) - { - if (strcmp(valstr, (const char *) CameraParameters::PIXEL_FORMAT_YUV422I) == 0) - { + if ((valstr = params.getPictureFormat()) != NULL) { + if (strcmp(valstr, (const char *) CameraParameters::PIXEL_FORMAT_YUV422I) == 0) { CAMHAL_LOGDA("CbYCrY format selected"); pixFormat = OMX_COLOR_FormatCbYCrY; - } - else if(strcmp(valstr, (const char *) CameraParameters::PIXEL_FORMAT_YUV420SP) == 0) - { + mPictureFormatFromClient = CameraParameters::PIXEL_FORMAT_YUV422I; + } else if(strcmp(valstr, (const char *) CameraParameters::PIXEL_FORMAT_YUV420SP) == 0) { CAMHAL_LOGDA("YUV420SP format selected"); pixFormat = OMX_COLOR_FormatYUV420SemiPlanar; - } - else if(strcmp(valstr, (const char *) CameraParameters::PIXEL_FORMAT_RGB565) == 0) - { + mPictureFormatFromClient = CameraParameters::PIXEL_FORMAT_YUV420SP; + } else if(strcmp(valstr, (const char *) CameraParameters::PIXEL_FORMAT_RGB565) == 0) { CAMHAL_LOGDA("RGB565 format selected"); pixFormat = OMX_COLOR_Format16bitRGB565; - } - else if(strcmp(valstr, (const char *) CameraParameters::PIXEL_FORMAT_JPEG) == 0) - { + mPictureFormatFromClient = CameraParameters::PIXEL_FORMAT_RGB565; + } else if (strcmp(valstr, (const char *) CameraParameters::PIXEL_FORMAT_JPEG) == 0) { CAMHAL_LOGDA("JPEG format selected"); pixFormat = OMX_COLOR_FormatUnused; mCodingMode = CodingNone; - } - else if(strcmp(valstr, (const char *) TICameraParameters::PIXEL_FORMAT_JPS) == 0) - { + mPictureFormatFromClient = CameraParameters::PIXEL_FORMAT_JPEG; + } else if (strcmp(valstr, (const char *) TICameraParameters::PIXEL_FORMAT_JPS) == 0) { CAMHAL_LOGDA("JPS format selected"); pixFormat = OMX_COLOR_FormatUnused; mCodingMode = CodingJPS; - } - else if(strcmp(valstr, (const char *) TICameraParameters::PIXEL_FORMAT_MPO) == 0) - { + mPictureFormatFromClient = TICameraParameters::PIXEL_FORMAT_JPS; + } else if (strcmp(valstr, (const char *) TICameraParameters::PIXEL_FORMAT_MPO) == 0) { CAMHAL_LOGDA("MPO format selected"); pixFormat = OMX_COLOR_FormatUnused; mCodingMode = CodingMPO; - } - else if(strcmp(valstr, (const char *) TICameraParameters::PIXEL_FORMAT_RAW_JPEG) == 0) - { - CAMHAL_LOGDA("RAW + JPEG format selected"); - pixFormat = OMX_COLOR_FormatUnused; - mCodingMode = CodingRAWJPEG; - } - else if(strcmp(valstr, (const char *) TICameraParameters::PIXEL_FORMAT_RAW_MPO) == 0) - { - CAMHAL_LOGDA("RAW + MPO format selected"); - pixFormat = OMX_COLOR_FormatUnused; - mCodingMode = CodingRAWMPO; - } - else if(strcmp(valstr, (const char *) TICameraParameters::PIXEL_FORMAT_RAW) == 0) - { + mPictureFormatFromClient = TICameraParameters::PIXEL_FORMAT_MPO; + } else if (strcmp(valstr, (const char *) TICameraParameters::PIXEL_FORMAT_RAW) == 0) { CAMHAL_LOGDA("RAW Picture format selected"); pixFormat = OMX_COLOR_FormatRawBayer10bit; - } - else - { + mPictureFormatFromClient = TICameraParameters::PIXEL_FORMAT_RAW; + } else { CAMHAL_LOGEA("Invalid format, JPEG format selected as default"); pixFormat = OMX_COLOR_FormatUnused; - } + mPictureFormatFromClient = NULL; } - else - { + } else { CAMHAL_LOGEA("Picture format is NULL, defaulting to JPEG"); pixFormat = OMX_COLOR_FormatUnused; - } + mPictureFormatFromClient = NULL; + } // JPEG capture is not supported in video mode by OMX Camera // Set capture format to yuv422i...jpeg encode will diff --git a/camera/OMXCameraAdapter/OMXExif.cpp b/camera/OMXCameraAdapter/OMXExif.cpp index 70fb3db7..76d94bdd 100644 --- a/camera/OMXCameraAdapter/OMXExif.cpp +++ b/camera/OMXCameraAdapter/OMXExif.cpp @@ -193,6 +193,7 @@ status_t OMXCameraAdapter::setParametersEXIF(const CameraParameters ¶ms, if( ( valstr = params.get(TICameraParameters::KEY_EXIF_MODEL ) ) != NULL ) { CAMHAL_LOGVB("EXIF Model: %s", valstr); + strncpy(mEXIFData.mModel, valstr, EXIF_MODEL_SIZE - 1); mEXIFData.mModelValid= true; } else @@ -203,6 +204,7 @@ status_t OMXCameraAdapter::setParametersEXIF(const CameraParameters ¶ms, if( ( valstr = params.get(TICameraParameters::KEY_EXIF_MAKE ) ) != NULL ) { CAMHAL_LOGVB("EXIF Make: %s", valstr); + strncpy(mEXIFData.mMake, valstr, EXIF_MAKE_SIZE - 1); mEXIFData.mMakeValid = true; } else @@ -210,6 +212,18 @@ status_t OMXCameraAdapter::setParametersEXIF(const CameraParameters ¶ms, mEXIFData.mMakeValid= false; } + + if( ( valstr = params.get(CameraParameters::KEY_FOCAL_LENGTH) ) != NULL ) { + CAMHAL_LOGVB("EXIF Focal length: %s", valstr); + ExifElementsTable::stringToRational(valstr, + &mEXIFData.mFocalNum, + &mEXIFData.mFocalDen); + } else { + mEXIFData.mFocalNum = 0; + mEXIFData.mFocalDen = 0; + } + + LOG_FUNCTION_NAME_EXIT; return ret; @@ -293,7 +307,7 @@ status_t OMXCameraAdapter::setupEXIF() ( mEXIFData.mModelValid ) ) { strncpy(( char * ) sharedPtr, - ( char * ) mParams.get(TICameraParameters::KEY_EXIF_MODEL ), + mEXIFData.mModel, EXIF_MODEL_SIZE - 1); exifTags->pModelBuff = ( OMX_S8 * ) ( sharedPtr - sharedBuffer.pSharedBuff ); @@ -306,8 +320,8 @@ status_t OMXCameraAdapter::setupEXIF() ( mEXIFData.mMakeValid ) ) { strncpy( ( char * ) sharedPtr, - ( char * ) mParams.get(TICameraParameters::KEY_EXIF_MAKE ), - EXIF_MAKE_SIZE - 1); + mEXIFData.mMake, + EXIF_MAKE_SIZE - 1); exifTags->pMakeBuff = ( OMX_S8 * ) ( sharedPtr - sharedBuffer.pSharedBuff ); exifTags->ulMakeBuffSizeBytes = strlen((char*)sharedPtr) + 1; @@ -317,12 +331,9 @@ status_t OMXCameraAdapter::setupEXIF() if ( ( OMX_TI_TagReadWrite == exifTags->eStatusFocalLength )) { - unsigned int numerator = 0, denominator = 0; - ExifElementsTable::stringToRational(mParams.get(CameraParameters::KEY_FOCAL_LENGTH), - &numerator, &denominator); - if (numerator || denominator) { - exifTags->ulFocalLength[0] = (OMX_U32) numerator; - exifTags->ulFocalLength[1] = (OMX_U32) denominator; + if (mEXIFData.mFocalNum || mEXIFData.mFocalDen ) { + exifTags->ulFocalLength[0] = (OMX_U32) mEXIFData.mFocalNum; + exifTags->ulFocalLength[1] = (OMX_U32) mEXIFData.mFocalDen; CAMHAL_LOGVB("exifTags->ulFocalLength = [%u] [%u]", (unsigned int)(exifTags->ulFocalLength[0]), (unsigned int)(exifTags->ulFocalLength[1])); @@ -512,22 +523,21 @@ status_t OMXCameraAdapter::setupEXIF_libjpeg(ExifElementsTable* exifTable, capData = &mCameraAdapterParameters.mCameraPortParams[mCameraAdapterParameters.mImagePortIndex]; if ((NO_ERROR == ret) && (mEXIFData.mModelValid)) { - ret = exifTable->insertElement(TAG_MODEL, mParams.get(TICameraParameters::KEY_EXIF_MODEL)); + ret = exifTable->insertElement(TAG_MODEL, mEXIFData.mModel); } if ((NO_ERROR == ret) && (mEXIFData.mMakeValid)) { - ret = exifTable->insertElement(TAG_MAKE, mParams.get(TICameraParameters::KEY_EXIF_MAKE)); + ret = exifTable->insertElement(TAG_MAKE, mEXIFData.mMake); } if ((NO_ERROR == ret)) { - unsigned int numerator = 0, denominator = 0; - ExifElementsTable::stringToRational(mParams.get(CameraParameters::KEY_FOCAL_LENGTH), - &numerator, &denominator); - if (numerator || denominator) { + if (mEXIFData.mFocalNum || mEXIFData.mFocalDen) { char temp_value[256]; // arbitrarily long string snprintf(temp_value, - sizeof(temp_value)/sizeof(char), - "%u/%u", numerator, denominator); + sizeof(temp_value)/sizeof(char), + "%u/%u", + mEXIFData.mFocalNum, + mEXIFData.mFocalDen); ret = exifTable->insertElement(TAG_FOCALLENGTH, temp_value); } @@ -618,8 +628,8 @@ status_t OMXCameraAdapter::setupEXIF_libjpeg(ExifElementsTable* exifTable, memcpy(temp_value, ExifAsciiPrefix, sizeof(ExifAsciiPrefix)); memcpy(temp_value + sizeof(ExifAsciiPrefix), - mParams.get(CameraParameters::KEY_GPS_PROCESSING_METHOD), - (GPS_PROCESSING_SIZE - sizeof(ExifAsciiPrefix))); + mEXIFData.mGPSData.mProcMethod, + (GPS_PROCESSING_SIZE - sizeof(ExifAsciiPrefix))); ret = exifTable->insertElement(TAG_GPS_PROCESSING_METHOD, temp_value); } @@ -650,9 +660,9 @@ status_t OMXCameraAdapter::setupEXIF_libjpeg(ExifElementsTable* exifTable, ret = exifTable->insertElement(TAG_GPS_DATESTAMP, mEXIFData.mGPSData.mDatestamp); } - if ((NO_ERROR == ret) && mParams.get(CameraParameters::KEY_ROTATION) ) { + if (NO_ERROR == ret) { const char* exif_orient = - ExifElementsTable::degreesToExifOrientation(mParams.get(CameraParameters::KEY_ROTATION)); + ExifElementsTable::degreesToExifOrientation(mPictureRotation); if (exif_orient) { ret = exifTable->insertElement(TAG_ORIENTATION, exif_orient); diff --git a/camera/inc/Encoder_libjpeg.h b/camera/inc/Encoder_libjpeg.h index 727dd92f..fb9a8946 100644 --- a/camera/inc/Encoder_libjpeg.h +++ b/camera/inc/Encoder_libjpeg.h @@ -93,7 +93,7 @@ class ExifElementsTable { void insertExifToJpeg(unsigned char* jpeg, size_t jpeg_size); status_t insertExifThumbnailImage(const char*, int); void saveJpeg(unsigned char* picture, size_t jpeg_size); - static const char* degreesToExifOrientation(const char*); + static const char* degreesToExifOrientation(unsigned int); static void stringToRational(const char*, unsigned int*, unsigned int*); static bool isAsciiTag(const char* tag); private: diff --git a/camera/inc/OMXCameraAdapter/OMXCameraAdapter.h b/camera/inc/OMXCameraAdapter/OMXCameraAdapter.h index 0fdc7705..59c5efc9 100644 --- a/camera/inc/OMXCameraAdapter/OMXCameraAdapter.h +++ b/camera/inc/OMXCameraAdapter/OMXCameraAdapter.h @@ -298,6 +298,9 @@ public: { public: GPSData mGPSData; + char mMake[EXIF_MODEL_SIZE]; + char mModel[EXIF_MAKE_SIZE]; + unsigned int mFocalNum, mFocalDen; bool mMakeValid; bool mModelValid; }; @@ -867,6 +870,7 @@ private: unsigned int mPending3Asettings; Mutex m3ASettingsUpdateLock; Gen3A_settings mParameters3A; + const char *mPictureFormatFromClient; OMX_TI_CONFIG_3A_FACE_PRIORITY mFacePriority; OMX_TI_CONFIG_3A_REGION_PRIORITY mRegionPriority; @@ -889,6 +893,8 @@ private: bool mBracketingEnabled; int mBracketingRange; + bool mIternalRecordingHint; + CameraParameters mParameters; bool mOmxInitialized; OMXCameraAdapterComponentContext mCameraAdapterParameters; -- cgit v1.2.3