android: camera_device: Rework CameraStream handling

The CameraDevice::streams_ vector of CameraStream instances is
currently mostly accessed by index. The current implementation
creates all the CameraStream during the first loop that inspects the
camera3_stream instances and then update the index of the
StreamConfiguration associated with the CameraStream during a second
loop that inspects MJPEG streams. A third loop creates the JPEG encoder
associated with camera streams that produce MJPEG format.

As the index-based association is hard to follow and rather fragile,
rework the creation and handling of CameraStream:

1) Make the StreamConfiguration index a constructor parameter and a
   private struct member. This disallows the creation of CameraStream
   without a StreamConfiguration index assigned.

2) Create CameraStream only after the associated StreamConfiguration
   has been identified. The first loop creates CameraStream for non-JPEG
   streams, the second for the JPEG ones after having identified the
   associated StreamConfiguration. Since we have just created the
   CameraStream, create the JPEG encoder at the same time instead of
   deferring it.

This change removes all accesses by index to the CameraDevice::streams_
vector.

No functional changes intended, but this change aims to make the code
easier to follow and more robust.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
This commit is contained in:
Jacopo Mondi 2020-09-02 11:58:00 +02:00
parent f868f08c21
commit c82f944399
2 changed files with 64 additions and 53 deletions

View file

@ -169,8 +169,8 @@ MappedCamera3Buffer::MappedCamera3Buffer(const buffer_handle_t camera3buffer,
} }
} }
CameraStream::CameraStream(PixelFormat f, Size s) CameraStream::CameraStream(PixelFormat f, Size s, unsigned int i)
: index(-1), format(f), size(s), jpeg(nullptr) : format(f), size(s), jpeg(nullptr), index_(i)
{ {
} }
@ -1191,6 +1191,7 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)
streams_.reserve(stream_list->num_streams); streams_.reserve(stream_list->num_streams);
/* First handle all non-MJPEG streams. */ /* First handle all non-MJPEG streams. */
camera3_stream_t *jpegStream = nullptr;
for (unsigned int i = 0; i < stream_list->num_streams; ++i) { for (unsigned int i = 0; i < stream_list->num_streams; ++i) {
camera3_stream_t *stream = stream_list->streams[i]; camera3_stream_t *stream = stream_list->streams[i];
Size size(stream->width, stream->height); Size size(stream->width, stream->height);
@ -1207,52 +1208,56 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)
if (!format.isValid()) if (!format.isValid())
return -EINVAL; return -EINVAL;
streams_.emplace_back(format, size);
stream->priv = static_cast<void *>(&streams_[i]);
/* Defer handling of MJPEG streams until all others are known. */ /* Defer handling of MJPEG streams until all others are known. */
if (stream->format == HAL_PIXEL_FORMAT_BLOB) if (stream->format == HAL_PIXEL_FORMAT_BLOB) {
if (jpegStream) {
LOG(HAL, Error)
<< "Multiple JPEG streams are not supported";
return -EINVAL;
}
jpegStream = stream;
continue; continue;
}
StreamConfiguration streamConfiguration; StreamConfiguration streamConfiguration;
streamConfiguration.size = size; streamConfiguration.size = size;
streamConfiguration.pixelFormat = format; streamConfiguration.pixelFormat = format;
config_->addConfiguration(streamConfiguration); config_->addConfiguration(streamConfiguration);
streams_[i].index = config_->size() - 1; unsigned int index = config_->size() - 1;
streams_.emplace_back(format, size, index);
stream->priv = static_cast<void *>(&streams_.back());
} }
/* Now handle MJPEG streams, adding a new stream if required. */ /* Now handle the MJPEG streams, adding a new stream if required. */
for (unsigned int i = 0; i < stream_list->num_streams; ++i) { if (jpegStream) {
camera3_stream_t *stream = stream_list->streams[i]; int index = -1;
bool match = false;
if (stream->format != HAL_PIXEL_FORMAT_BLOB) /* Search for a compatible stream in the non-JPEG ones. */
continue; for (unsigned int i = 0; i < config_->size(); i++) {
StreamConfiguration &cfg = config_->at(i);
/* Search for a compatible stream */
for (unsigned int j = 0; j < config_->size(); j++) {
StreamConfiguration &cfg = config_->at(j);
/* /*
* \todo The PixelFormat must also be compatible with * \todo The PixelFormat must also be compatible with
* the encoder. * the encoder.
*/ */
if (cfg.size == streams_[i].size) { if (cfg.size.width != jpegStream->width ||
LOG(HAL, Info) << "Stream " << i cfg.size.height != jpegStream->height)
<< " using libcamera stream " << j; continue;
match = true; LOG(HAL, Info)
streams_[i].index = j; << "Android JPEG stream mapped to libcamera stream " << i;
}
index = i;
break;
} }
/* /*
* Without a compatible match for JPEG encoding we must * Without a compatible match for JPEG encoding we must
* introduce a new stream to satisfy the request requirements. * introduce a new stream to satisfy the request requirements.
*/ */
if (!match) { if (index < 0) {
StreamConfiguration streamConfiguration; StreamConfiguration streamConfiguration;
/* /*
@ -1261,15 +1266,31 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)
* handled, and should be considered as part of any * handled, and should be considered as part of any
* stream configuration reworks. * stream configuration reworks.
*/ */
streamConfiguration.size.width = stream->width; streamConfiguration.size.width = jpegStream->width;
streamConfiguration.size.height = stream->height; streamConfiguration.size.height = jpegStream->height;
streamConfiguration.pixelFormat = formats::NV12; streamConfiguration.pixelFormat = formats::NV12;
LOG(HAL, Info) << "Adding " << streamConfiguration.toString() LOG(HAL, Info) << "Adding " << streamConfiguration.toString()
<< " for MJPEG support"; << " for MJPEG support";
config_->addConfiguration(streamConfiguration); config_->addConfiguration(streamConfiguration);
streams_[i].index = config_->size() - 1; index = config_->size() - 1;
}
StreamConfiguration &cfg = config_->at(index);
CameraStream &cameraStream =
streams_.emplace_back(formats::MJPEG, cfg.size, index);
jpegStream->priv = static_cast<void *>(&cameraStream);
/*
* Construct a software encoder for the MJPEG streams from the
* chosen libcamera source stream.
*/
cameraStream.jpeg = new EncoderLibJpeg();
int ret = cameraStream.jpeg->configure(cfg);
if (ret) {
LOG(HAL, Error) << "Failed to configure encoder";
return ret;
} }
} }
@ -1292,25 +1313,11 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)
for (unsigned int i = 0; i < stream_list->num_streams; ++i) { for (unsigned int i = 0; i < stream_list->num_streams; ++i) {
camera3_stream_t *stream = stream_list->streams[i]; camera3_stream_t *stream = stream_list->streams[i];
CameraStream *cameraStream = &streams_[i]; CameraStream *cameraStream = static_cast<CameraStream *>(stream->priv);
StreamConfiguration &cfg = config_->at(cameraStream->index); StreamConfiguration &cfg = config_->at(cameraStream->index());
/* Use the bufferCount confirmed by the validation process. */ /* Use the bufferCount confirmed by the validation process. */
stream->max_buffers = cfg.bufferCount; stream->max_buffers = cfg.bufferCount;
/*
* Construct a software encoder for MJPEG streams from the
* chosen libcamera source stream.
*/
if (cameraStream->format == formats::MJPEG) {
cameraStream->jpeg = new EncoderLibJpeg();
int ret = cameraStream->jpeg->configure(cfg);
if (ret) {
LOG(HAL, Error)
<< "Failed to configure encoder";
return ret;
}
}
} }
/* /*
@ -1424,7 +1431,7 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
} }
descriptor->frameBuffers.emplace_back(buffer); descriptor->frameBuffers.emplace_back(buffer);
StreamConfiguration *streamConfiguration = &config_->at(cameraStream->index); StreamConfiguration *streamConfiguration = &config_->at(cameraStream->index());
Stream *stream = streamConfiguration->stream(); Stream *stream = streamConfiguration->stream();
request->addBuffer(stream, buffer); request->addBuffer(stream, buffer);
@ -1479,7 +1486,7 @@ void CameraDevice::requestComplete(Request *request)
continue; continue;
} }
StreamConfiguration *streamConfiguration = &config_->at(cameraStream->index); StreamConfiguration *streamConfiguration = &config_->at(cameraStream->index());
Stream *stream = streamConfiguration->stream(); Stream *stream = streamConfiguration->stream();
FrameBuffer *buffer = request->findBuffer(stream); FrameBuffer *buffer = request->findBuffer(stream);
if (!buffer) { if (!buffer) {

View file

@ -28,20 +28,24 @@
class CameraMetadata; class CameraMetadata;
struct CameraStream { struct CameraStream {
CameraStream(libcamera::PixelFormat, libcamera::Size); public:
CameraStream(libcamera::PixelFormat, libcamera::Size, unsigned int i);
~CameraStream(); ~CameraStream();
/* unsigned int index() const { return index_; }
* The index of the libcamera StreamConfiguration as added during
* configureStreams(). A single libcamera Stream may be used to deliver
* one or more streams to the Android framework.
*/
unsigned int index;
libcamera::PixelFormat format; libcamera::PixelFormat format;
libcamera::Size size; libcamera::Size size;
Encoder *jpeg; Encoder *jpeg;
private:
/*
* The index of the libcamera StreamConfiguration as added during
* configureStreams(). A single libcamera Stream may be used to deliver
* one or more streams to the Android framework.
*/
unsigned int index_;
}; };
class CameraDevice : protected libcamera::Loggable class CameraDevice : protected libcamera::Loggable