Skip to content

Commit 58a5859

Browse files
Treehugger RobotGerrit Code Review
Treehugger Robot
authored and
Gerrit Code Review
committed
Merge "Fix ConcurrentModificationException exception when opening the camera with VideoCapture" into androidx-main
2 parents 027bd31 + 6a1c7de commit 58a5859

File tree

4 files changed

+182
-9
lines changed

4 files changed

+182
-9
lines changed

camera/camera-core/src/androidTest/java/androidx/camera/core/impl/CaptureConfigTest.java

+45
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import static org.mockito.Mockito.mock;
2424

2525
import android.hardware.camera2.CameraDevice;
26+
import android.util.Range;
2627
import android.view.Surface;
2728

2829
import androidx.annotation.NonNull;
@@ -276,6 +277,50 @@ public void cameraCaptureCallbacks_areImmutable() {
276277
configuration.getCameraCaptureCallbacks().add(mock(CameraCaptureCallback.class));
277278
}
278279

280+
@Test
281+
public void builderChange_doNotChangeEarlierBuiltInstance() {
282+
// 1. Arrange
283+
CameraCaptureCallback callback1 = mock(CameraCaptureCallback.class);
284+
CameraCaptureCallback callback2 = mock(CameraCaptureCallback.class);
285+
DeferrableSurface deferrableSurface1 = mock(DeferrableSurface.class);
286+
DeferrableSurface deferrableSurface2 = mock(DeferrableSurface.class);
287+
Range<Integer> fpsRange1 = new Range<>(30, 30);
288+
Range<Integer> fpsRange2 = new Range<>(15, 30);
289+
int optionValue1 = 1;
290+
int optionValue2 = 2;
291+
int tagValue1 = 1;
292+
int tagValue2 = 2;
293+
int template1 = CameraDevice.TEMPLATE_PREVIEW;
294+
int template2 = CameraDevice.TEMPLATE_RECORD;
295+
296+
CaptureConfig.Builder builder = new CaptureConfig.Builder();
297+
builder.addSurface(deferrableSurface1);
298+
builder.setExpectedFrameRateRange(fpsRange1);
299+
builder.addCameraCaptureCallback(callback1);
300+
builder.setTemplateType(template1);
301+
builder.addTag("KEY", tagValue1);
302+
builder.addImplementationOption(OPTION, optionValue1);
303+
CaptureConfig captureConfig = builder.build();
304+
305+
// 2. Act
306+
// builder change should not affect the instance built earlier.
307+
builder.addSurface(deferrableSurface2);
308+
builder.setExpectedFrameRateRange(fpsRange2);
309+
builder.addCameraCaptureCallback(callback2);
310+
builder.setTemplateType(template2);
311+
builder.addTag("KEY", tagValue2);
312+
builder.addImplementationOption(OPTION, optionValue2);
313+
314+
// 3. Verify
315+
assertThat(captureConfig.getSurfaces()).containsExactly(deferrableSurface1);
316+
assertThat(captureConfig.getExpectedFrameRateRange()).isEqualTo(fpsRange1);
317+
assertThat(captureConfig.getCameraCaptureCallbacks()).containsExactly(callback1);
318+
assertThat(captureConfig.getTemplateType()).isEqualTo(template1);
319+
assertThat(captureConfig.getTagBundle().getTag("KEY")).isEqualTo(tagValue1);
320+
assertThat(captureConfig.getImplementationOptions().retrieveOption(OPTION))
321+
.isEqualTo(optionValue1);
322+
}
323+
279324
/**
280325
* A fake {@link MultiValueSet}.
281326
*/

camera/camera-core/src/androidTest/java/androidx/camera/core/impl/SessionConfigTest.java

+128
Original file line numberDiff line numberDiff line change
@@ -840,6 +840,134 @@ public void combineTwoSessionsTagsValid() {
840840
assertThat(tag.getTag("TEST01")).isEqualTo("String");
841841
}
842842

843+
@Test
844+
public void builderChange_doNotChangeEarlierBuiltInstance() {
845+
// 1. Arrange
846+
CameraCaptureCallback callback1 = mock(CameraCaptureCallback.class);
847+
CameraCaptureCallback callback2 = mock(CameraCaptureCallback.class);
848+
DeferrableSurface deferrableSurface1 = mock(DeferrableSurface.class);
849+
DeferrableSurface deferrableSurface2 = mock(DeferrableSurface.class);
850+
CameraDevice.StateCallback deviceStateCallback1 = mock(CameraDevice.StateCallback.class);
851+
CameraDevice.StateCallback deviceStateCallback2 = mock(CameraDevice.StateCallback.class);
852+
CameraCaptureSession.StateCallback sessionCallback1 =
853+
mock(CameraCaptureSession.StateCallback.class);
854+
CameraCaptureSession.StateCallback sessionCallback2 =
855+
mock(CameraCaptureSession.StateCallback.class);
856+
SessionConfig.ErrorListener errorListener1 = mock(SessionConfig.ErrorListener.class);
857+
SessionConfig.ErrorListener errorListener2 = mock(SessionConfig.ErrorListener.class);
858+
Range<Integer> fpsRange1 = new Range<>(30, 30);
859+
Range<Integer> fpsRange2 = new Range<>(15, 30);
860+
MutableOptionsBundle optionsBundle1 = MutableOptionsBundle.create();
861+
optionsBundle1.insertOption(OPTION, 1);
862+
MutableOptionsBundle optionsBundle2 = MutableOptionsBundle.create();
863+
optionsBundle2.insertOption(OPTION, 2);
864+
int template1 = CameraDevice.TEMPLATE_PREVIEW;
865+
int template2 = CameraDevice.TEMPLATE_RECORD;
866+
867+
SessionConfig.Builder builder = new SessionConfig.Builder();
868+
builder.addSurface(deferrableSurface1);
869+
builder.setExpectedFrameRateRange(fpsRange1);
870+
builder.addCameraCaptureCallback(callback1);
871+
builder.addRepeatingCameraCaptureCallback(callback1);
872+
builder.addDeviceStateCallback(deviceStateCallback1);
873+
builder.addSessionStateCallback(sessionCallback1);
874+
builder.setTemplateType(template1);
875+
builder.addImplementationOptions(optionsBundle1);
876+
builder.addErrorListener(errorListener1);
877+
SessionConfig sessionConfig = builder.build();
878+
879+
// 2. Act
880+
// builder change should not affect the instance built earlier.
881+
builder.addSurface(deferrableSurface2);
882+
builder.setExpectedFrameRateRange(fpsRange2);
883+
builder.addCameraCaptureCallback(callback2);
884+
builder.addRepeatingCameraCaptureCallback(callback2);
885+
builder.addDeviceStateCallback(deviceStateCallback2);
886+
builder.addSessionStateCallback(sessionCallback2);
887+
builder.setTemplateType(template2);
888+
builder.addImplementationOptions(optionsBundle2);
889+
builder.addErrorListener(errorListener2);
890+
891+
// 3. Verify
892+
assertThat(sessionConfig.getSurfaces()).containsExactly(deferrableSurface1);
893+
assertThat(sessionConfig.getExpectedFrameRateRange()).isEqualTo(fpsRange1);
894+
assertThat(sessionConfig.getSingleCameraCaptureCallbacks()).containsExactly(callback1);
895+
assertThat(sessionConfig.getRepeatingCaptureConfig().getCameraCaptureCallbacks())
896+
.containsExactly(callback1);
897+
assertThat(sessionConfig.getDeviceStateCallbacks()).containsExactly(deviceStateCallback1);
898+
assertThat(sessionConfig.getSessionStateCallbacks()).containsExactly(sessionCallback1);
899+
assertThat(sessionConfig.getTemplateType()).isEqualTo(template1);
900+
assertThat(sessionConfig.getImplementationOptions().retrieveOption(OPTION)).isEqualTo(1);
901+
assertThat(sessionConfig.getErrorListeners()).containsExactly(errorListener1);
902+
}
903+
904+
@Test
905+
public void validatingBuilderChange_doNotChangeEarlierBuiltInstance() {
906+
// 1. Arrange
907+
CameraCaptureCallback callback1 = mock(CameraCaptureCallback.class);
908+
CameraCaptureCallback callback2 = mock(CameraCaptureCallback.class);
909+
DeferrableSurface deferrableSurface1 = mock(DeferrableSurface.class);
910+
DeferrableSurface deferrableSurface2 = mock(DeferrableSurface.class);
911+
CameraDevice.StateCallback deviceStateCallback1 = mock(CameraDevice.StateCallback.class);
912+
CameraDevice.StateCallback deviceStateCallback2 = mock(CameraDevice.StateCallback.class);
913+
CameraCaptureSession.StateCallback sessionCallback1 =
914+
mock(CameraCaptureSession.StateCallback.class);
915+
CameraCaptureSession.StateCallback sessionCallback2 =
916+
mock(CameraCaptureSession.StateCallback.class);
917+
SessionConfig.ErrorListener errorListener1 = mock(SessionConfig.ErrorListener.class);
918+
SessionConfig.ErrorListener errorListener2 = mock(SessionConfig.ErrorListener.class);
919+
Range<Integer> fpsRange1 = new Range<>(30, 30);
920+
Range<Integer> fpsRange2 = new Range<>(15, 30);
921+
MutableOptionsBundle optionsBundle1 = MutableOptionsBundle.create();
922+
optionsBundle1.insertOption(OPTION, 1);
923+
MutableOptionsBundle optionsBundle2 = MutableOptionsBundle.create();
924+
optionsBundle2.insertOption(OPTION, 2);
925+
int template1 = CameraDevice.TEMPLATE_PREVIEW;
926+
int template2 = CameraDevice.TEMPLATE_RECORD;
927+
928+
SessionConfig.Builder builder = new SessionConfig.Builder();
929+
builder.addSurface(deferrableSurface1);
930+
builder.setExpectedFrameRateRange(fpsRange1);
931+
builder.addCameraCaptureCallback(callback1);
932+
builder.addRepeatingCameraCaptureCallback(callback1);
933+
builder.addDeviceStateCallback(deviceStateCallback1);
934+
builder.addSessionStateCallback(sessionCallback1);
935+
builder.setTemplateType(template1);
936+
builder.addImplementationOptions(optionsBundle1);
937+
builder.addErrorListener(errorListener1);
938+
939+
SessionConfig.ValidatingBuilder validatingBuilder = new SessionConfig.ValidatingBuilder();
940+
validatingBuilder.add(builder.build());
941+
SessionConfig sessionConfig = validatingBuilder.build();
942+
943+
// 2. Act
944+
// add another SessionConfig to ValidatingBuilder. This should not affect the
945+
// instance built earlier.
946+
SessionConfig.Builder builder2 = new SessionConfig.Builder();
947+
builder2.addSurface(deferrableSurface2);
948+
builder2.setExpectedFrameRateRange(fpsRange2);
949+
builder2.addCameraCaptureCallback(callback2);
950+
builder2.addRepeatingCameraCaptureCallback(callback2);
951+
builder2.addDeviceStateCallback(deviceStateCallback2);
952+
builder2.addSessionStateCallback(sessionCallback2);
953+
builder2.setTemplateType(template2);
954+
builder2.addImplementationOptions(optionsBundle2);
955+
builder2.addErrorListener(errorListener2);
956+
validatingBuilder.add(builder2.build());
957+
958+
// 3. Verify
959+
assertThat(sessionConfig.getSurfaces()).containsExactly(deferrableSurface1);
960+
assertThat(sessionConfig.getExpectedFrameRateRange()).isEqualTo(fpsRange1);
961+
assertThat(sessionConfig.getSingleCameraCaptureCallbacks()).containsExactly(callback1);
962+
assertThat(sessionConfig.getRepeatingCaptureConfig().getCameraCaptureCallbacks())
963+
.containsExactly(callback1);
964+
assertThat(sessionConfig.getDeviceStateCallbacks()).containsExactly(deviceStateCallback1);
965+
assertThat(sessionConfig.getSessionStateCallbacks()).containsExactly(sessionCallback1);
966+
assertThat(sessionConfig.getTemplateType()).isEqualTo(template1);
967+
assertThat(sessionConfig.getImplementationOptions().retrieveOption(OPTION)).isEqualTo(1);
968+
assertThat(sessionConfig.getErrorListeners()).containsExactly(errorListener1);
969+
}
970+
843971
private SessionConfig createSessionConfigWithTag(String key, Object tagValue) {
844972
SessionConfig.Builder builder1 = new SessionConfig.Builder();
845973
builder1.addSurface(mMockSurface1);

camera/camera-core/src/main/java/androidx/camera/core/impl/CaptureConfig.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -416,7 +416,7 @@ public CaptureConfig build() {
416416
OptionsBundle.from(mImplementationOptions),
417417
mTemplateType,
418418
mExpectedFrameRateRange,
419-
mCameraCaptureCallbacks,
419+
new ArrayList<>(mCameraCaptureCallbacks),
420420
mUseRepeatingSurface,
421421
TagBundle.from(mMutableTagBundle),
422422
mCameraCaptureResult);

camera/camera-core/src/main/java/androidx/camera/core/impl/SessionConfig.java

+8-8
Original file line numberDiff line numberDiff line change
@@ -694,10 +694,10 @@ public Builder addImplementationOptions(@NonNull Config config) {
694694
public SessionConfig build() {
695695
return new SessionConfig(
696696
new ArrayList<>(mOutputConfigs),
697-
mDeviceStateCallbacks,
698-
mSessionStateCallbacks,
699-
mSingleCameraCaptureCallbacks,
700-
mErrorListeners,
697+
new ArrayList<>(mDeviceStateCallbacks),
698+
new ArrayList<>(mSessionStateCallbacks),
699+
new ArrayList<>(mSingleCameraCaptureCallbacks),
700+
new ArrayList<>(mErrorListeners),
701701
mCaptureConfigBuilder.build(),
702702
mInputConfiguration);
703703
}
@@ -847,10 +847,10 @@ public SessionConfig build() {
847847

848848
return new SessionConfig(
849849
outputConfigs,
850-
mDeviceStateCallbacks,
851-
mSessionStateCallbacks,
852-
mSingleCameraCaptureCallbacks,
853-
mErrorListeners,
850+
new ArrayList<>(mDeviceStateCallbacks),
851+
new ArrayList<>(mSessionStateCallbacks),
852+
new ArrayList<>(mSingleCameraCaptureCallbacks),
853+
new ArrayList<>(mErrorListeners),
854854
mCaptureConfigBuilder.build(),
855855
mInputConfiguration);
856856
}

0 commit comments

Comments
 (0)