Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit d3b85f69 authored by Eino-Ville Talvala's avatar Eino-Ville Talvala
Browse files

Camera2: Fix session shutdown race, frequent warning log 

- Make sure that session.close followed by device.createCaptureSession cannot race on
  configureOutputs calls
- Silence warning about RAW_OPAQUE format

Change-Id: I02e4a048e8b26ea61aadcf115b029e9fbb58ad4e
parent a6b5ba56

core/java/android/hardware/camera2/impl/CameraCaptureSessionImpl.java

+16 −7
Original line number Diff line number Diff line
@@ -261,9 +261,12 @@ public class CameraCaptureSessionImpl extends CameraCaptureSession { 
     * <p>The semantics are identical to {@link #close}, except that unconfiguring will be skipped.

     * <p>

     *

     * <p>After this call completes, the session will not call any further methods on the camera

     * device.</p>

     *

     * @see CameraCaptureSession#close

     */

    synchronized void replaceSessionClose(CameraCaptureSession other) {

    synchronized void replaceSessionClose() {

        /*

         * In order for creating new sessions to be fast, the new session should be created

         * before the old session is closed.
@@ -278,13 +281,17 @@ public class CameraCaptureSessionImpl extends CameraCaptureSession { 


        if (VERBOSE) Log.v(TAG, "replaceSessionClose");



        // #close was already called explicitly, keep going the slow route

        if (mClosed) {

            if (VERBOSE) Log.v(TAG, "replaceSessionClose - close was already called");

            return;

        }



        // Set up fast shutdown. Possible alternative paths:

        // - This session is active, so close() below starts the shutdown drain

        // - This session is mid-shutdown drain, and hasn't yet reached the idle drain listener.

        // - This session is already closed and has executed the idle drain listener, and

        //   configureOutputs(null) has already been called.

        //

        // Do not call configureOutputs(null) going forward, since it would race with the

        // configuration for the new session. If it was already called, then we don't care, since it

        // won't get called again.

        mSkipUnconfigure = true;



        close();

    }
@@ -599,6 +606,8 @@ public class CameraCaptureSessionImpl extends CameraCaptureSession { 
                 *

                 * This operation is idempotent; a session will not be closed twice.

                 */

                if (VERBOSE) Log.v(TAG, "Session drain complete, skip unconfigure: " +

                        mSkipUnconfigure);



                // Fast path: A new capture session has replaced this one; don't unconfigure.

                if (mSkipUnconfigure) {

core/java/android/hardware/camera2/impl/CameraDeviceImpl.java

+5 −5
Original line number Diff line number Diff line
@@ -387,7 +387,11 @@ public class CameraDeviceImpl extends CameraDevice { 


            checkIfCameraClosedOrInError();



            // TODO: we must be in UNCONFIGURED mode to begin with, or using another session

            // Notify current session that it's going away, before starting camera operations

            // After this call completes, the session is not allowed to call into CameraDeviceImpl

            if (mCurrentSession != null) {

                mCurrentSession.replaceSessionClose();

            }



            // TODO: dont block for this

            boolean configureSuccess = true;
@@ -407,10 +411,6 @@ public class CameraDeviceImpl extends CameraDevice { 
                    new CameraCaptureSessionImpl(outputs, listener, handler, this, mDeviceHandler,

                            configureSuccess);



            if (mCurrentSession != null) {

                mCurrentSession.replaceSessionClose(newSession);

            }



            // TODO: wait until current session closes, then create the new session

            mCurrentSession = newSession;

core/java/android/hardware/camera2/params/StreamConfigurationMap.java

+1 −6
Original line number Diff line number Diff line
@@ -813,6 +813,7 @@ public final class StreamConfigurationMap { 
        switch (format) {

            case HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED:

            case HAL_PIXEL_FORMAT_BLOB:

            case HAL_PIXEL_FORMAT_RAW_OPAQUE:

                return format;

            case ImageFormat.JPEG:

                throw new IllegalArgumentException(
@@ -843,12 +844,6 @@ public final class StreamConfigurationMap { 
     * @throws IllegalArgumentException if the format was not user-defined

     */

    static int checkArgumentFormat(int format) {

        // TODO: remove this hack , CTS shouldn't have been using internal constants

        if (format == HAL_PIXEL_FORMAT_RAW_OPAQUE) {

            Log.w(TAG, "RAW_OPAQUE is not yet a published format; allowing it anyway");

            return format;

        }



        if (!ImageFormat.isPublicFormat(format) && !PixelFormat.isPublicFormat(format)) {

            throw new IllegalArgumentException(String.format(

                    "format 0x%x was not defined in either ImageFormat or PixelFormat", format));