Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

[camera] Move camera streaming to platform interface #5783

Merged
fluttergithubbot merged 10 commits into from
May 25, 2022
Merged

[camera] Move camera streaming to platform interface #5783

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
6fb3c24
Add platform interface versions of the image data classes
stuartmorgan-g May 12, 2022
d86e912
Platform interface package parts
stuartmorgan-g May 13, 2022
ea8c032
Rewire app-facing package
stuartmorgan-g May 18, 2022
7a84131
Test update
stuartmorgan-g May 18, 2022
3b6865c
Temporary pubspec overrides
stuartmorgan-g May 18, 2022
7414e48
Version bumps
stuartmorgan-g May 18, 2022
b191e34
Future-proof the new API with an options dictionary
stuartmorgan-g May 18, 2022
014f1c0
Review comments
stuartmorgan-g May 19, 2022
701eefe
Revert app-facing portion
stuartmorgan-g May 25, 2022
881786c
Merge branch 'main' into camera-stream-platform-interface
stuartmorgan-g May 25, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Future-proof the new API with an options dictionary
  • Loading branch information
@stuartmorgan-g
stuartmorgan-g committed May 18, 2022
commit b191e3455c02ff738fc5543d394e45c3e3fa624b
3 changes: 2 additions & 1 deletion packages/camera/camera/test/camera_image_stream_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,8 @@ class MockStreamingCameraPlatform extends MockCameraPlatform {
StreamController<CameraImageData>? _streamController;

@override
Stream<CameraImageData> onStreamedFrameAvailable(int cameraId) {
Stream<CameraImageData> onStreamedFrameAvailable(int cameraId,
{CameraImageStreamOptions? options}) {
streamCallLog.add('onStreamedFrameAvailable');
_streamController = StreamController<CameraImageData>(
onListen: _onFrameStreamListen,
Expand Down
3 changes: 2 additions & 1 deletion packages/camera/camera_platform_interface/lib/src/method_channel/method_channel_camera.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,8 @@ class MethodChannelCamera extends CameraPlatform {
);

@override
Stream<CameraImageData> onStreamedFrameAvailable(int cameraId) {
Stream<CameraImageData> onStreamedFrameAvailable(int cameraId,
{CameraImageStreamOptions? options}) {
_frameStreamController = StreamController<CameraImageData>(
onListen: _onFrameStreamListen,
onPause: _onFrameStreamPauseResume,
Expand Down
5 changes: 3 additions & 2 deletions packages/camera/camera_platform_interface/lib/src/platform_interface/camera_platform.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,9 +157,10 @@ abstract class CameraPlatform extends PlatformInterface {
/// listen again later.
///
///
// TODO(bmparr): Add a setter method to control streaming settings (e.g.,
// TODO(bmparr): Add options to control streaming settings (e.g.,
// resolution and FPS).
Stream<CameraImageData> onStreamedFrameAvailable(int cameraId) {
Stream<CameraImageData> onStreamedFrameAvailable(int cameraId,
Copy link
Contributor

@bparrishMines bparrishMines May 18, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm confused by this name. Does the app-facing package notify the platform implementation package that a frame is available? Based on the MethodChannel implementation, it looks like this method creates the stream that the app-facing package listens to. Would a better name be something like createCameraImageDataStream?

Copy link
Contributor Author

@stuartmorgan-g stuartmorgan-g May 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like this name either; I used this name because it's the pattern used by all of the existing Streams in the interface

Do you want to pick a new naming scheme for platform interface streams in this plugin as part of this PR? If so I could make alternate named versions of all of the existing ones, doing passthroughs to the old names (which would be soft-deprecated with a comment), so that we'd get consistency.

Copy link
Contributor

@bparrishMines bparrishMines May 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see. I think I was confused by the call to startImageStream. I understand now that the app-facing package is adding a listener to the stream which is created by the platform implementation. I think the current name is appropriate for how it is being used then. Streams and callbacks in Flutter tend to use the on<event-name> pattern like the one here.

{CameraImageStreamOptions? options}) {
throw UnimplementedError('onStreamedFrameAvailable() is not implemented.');
}

Expand Down
7 changes: 7 additions & 0 deletions packages/camera/camera_platform_interface/lib/src/types/camera_image_data.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,13 @@ import 'package:flutter/foundation.dart';

import '../../camera_platform_interface.dart';

/// Options for configuring camera streaming.
///
/// Currently unused; this exists for future-proofing of the platform interface
/// API.
@immutable
class CameraImageStreamOptions {}

/// A single color plane of image data.
///
/// The number and meaning of the planes in an image are determined by its
Expand Down