[Merge] lp:~fboucault/qtubuntu-camera/hdr_scene_mode into lp:qtubuntu-camera
Jim Hodapp
jim.hodapp at canonical.com
Mon Jul 14 14:26:16 UTC 2014
Review: Needs Fixing
The main code looks great, I'd like to see some tests for the logic of setting the exposure mode and making sure the appropriate signals are emitted when expected.
Diff comments:
> === added file 'src/aalcameraexposurecontrol.cpp'
> --- src/aalcameraexposurecontrol.cpp 1970-01-01 00:00:00 +0000
> +++ src/aalcameraexposurecontrol.cpp 2014-07-11 18:11:38 +0000
> @@ -0,0 +1,141 @@
> +/*
> + * Copyright (C) 2014 Canonical, Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU Lesser General Public License as published by
> + * the Free Software Foundation; version 3.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "aalcameraexposurecontrol.h"
> +#include "aalcameracontrol.h"
> +#include "aalcameraservice.h"
> +
> +#include <hybris/camera/camera_compatibility_layer.h>
> +#include <hybris/camera/camera_compatibility_layer_capabilities.h>
> +
> +// Definition of this enum value is duplicated in camera-app
> +static const QCameraExposure::ExposureMode ExposureHdr = static_cast<QCameraExposure::ExposureMode>(QCameraExposure::ExposureModeVendor + 1);
> +
> +AalCameraExposureControl::AalCameraExposureControl(AalCameraService *service, QObject *parent)
> + : QCameraExposureControl(parent),
> + m_service(service),
> + m_requestedExposureMode(QCameraExposure::ExposureAuto),
> + m_actualExposureMode(QCameraExposure::ExposureAuto)
> +{
> + m_androidToQtExposureModes[SCENE_MODE_AUTO] = QCameraExposure::ExposureAuto;
> + m_androidToQtExposureModes[SCENE_MODE_ACTION] = QCameraExposure::ExposureSports;
> + m_androidToQtExposureModes[SCENE_MODE_NIGHT] = QCameraExposure::ExposureNight;
> + m_androidToQtExposureModes[SCENE_MODE_PARTY] = QCameraExposure::ExposureAuto; // FIXME: no correspondance
Is this no correspondence in your hybris implementation, or the Android code has no equivalent to hook up to?
> + m_androidToQtExposureModes[SCENE_MODE_SUNSET] = QCameraExposure::ExposureAuto; // FIXME: no correspondance
> + m_androidToQtExposureModes[SCENE_MODE_HDR] = ExposureHdr;
> +}
> +
> +void AalCameraExposureControl::init(CameraControl *control, CameraControlListener *listener)
> +{
> + Q_UNUSED(listener);
> +
> + m_requestedExposureMode = QCameraExposure::ExposureAuto;
> + m_actualExposureMode = QCameraExposure::ExposureAuto;
> +
> + m_supportedExposureModes.clear();
> + android_camera_enumerate_supported_scene_modes(control, &AalCameraExposureControl::supportedSceneModesCallback, this);
> +
> + Q_EMIT requestedValueChanged(QCameraExposureControl::ExposureMode);
> + Q_EMIT actualValueChanged(QCameraExposureControl::ExposureMode);
> + Q_EMIT parameterRangeChanged(QCameraExposureControl::ExposureMode);
> +}
> +
> +void AalCameraExposureControl::supportedSceneModesCallback(void *context, SceneMode sceneMode)
> +{
> + AalCameraExposureControl *self = (AalCameraExposureControl*)context;
> + self->m_supportedExposureModes << QVariant::fromValue(self->m_androidToQtExposureModes[sceneMode]);
> +}
> +
> +bool AalCameraExposureControl::setValue(ExposureParameter parameter, const QVariant& value)
> +{
> + if (!value.isValid()) {
> + return false;
> + }
> +
> + if (parameter == QCameraExposureControl::ExposureMode) {
> + m_requestedExposureMode = value.value<QCameraExposure::ExposureMode>();
> + Q_EMIT requestedValueChanged(QCameraExposureControl::ExposureMode);
> +
> + if (m_supportedExposureModes.contains(value)) {
> + SceneMode sceneMode = m_androidToQtExposureModes.key(m_requestedExposureMode);
> + android_camera_set_scene_mode(m_service->androidControl(), sceneMode);
> + m_actualExposureMode = m_requestedExposureMode;
> + Q_EMIT actualValueChanged(QCameraExposureControl::ExposureMode);
> + return true;
> + }
> + }
> +
> + return false;
> +}
> +
> +QVariant AalCameraExposureControl::requestedValue(ExposureParameter parameter) const
> +{
> + if (parameter == QCameraExposureControl::ExposureMode) {
> + return QVariant::fromValue(m_requestedExposureMode);
> + }
> +
> + return QVariant();
> +}
> +
> +QVariant AalCameraExposureControl::actualValue(ExposureParameter parameter) const
> +{
> + if (parameter == QCameraExposureControl::ExposureMode) {
> + return QVariant::fromValue(m_actualExposureMode);
> + }
> +
> + return QVariant();
> +}
> +
> +bool AalCameraExposureControl::isParameterSupported(ExposureParameter parameter) const
> +{
> + switch (parameter) {
> + case QCameraExposureControl::ISO:
> + return false;
> + case QCameraExposureControl::Aperture:
> + return false;
> + case QCameraExposureControl::ShutterSpeed:
> + return false;
> + case QCameraExposureControl::ExposureCompensation:
> + return false;
> + case QCameraExposureControl::FlashPower:
> + return false;
> + case QCameraExposureControl::FlashCompensation:
> + return false;
> + case QCameraExposureControl::TorchPower:
> + return false;
> + case QCameraExposureControl::SpotMeteringPoint:
> + return false;
> + case QCameraExposureControl::ExposureMode:
> + return true;
> + case QCameraExposureControl::MeteringMode:
> + return false;
> + default:
> + return false;
> + }
> +}
> +
> +QVariantList AalCameraExposureControl::supportedParameterRange(ExposureParameter parameter, bool *continuous) const
> +{
> + if (continuous != NULL) {
> + *continuous = false;
> + }
> +
> + if (parameter == QCameraExposureControl::ExposureMode) {
> + return m_supportedExposureModes;
> + }
> +
> + return QVariantList();
> +}
>
> === added file 'src/aalcameraexposurecontrol.h'
> --- src/aalcameraexposurecontrol.h 1970-01-01 00:00:00 +0000
> +++ src/aalcameraexposurecontrol.h 2014-07-11 18:11:38 +0000
> @@ -0,0 +1,52 @@
> +/*
> + * Copyright (C) 2014 Canonical, Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU Lesser General Public License as published by
> + * the Free Software Foundation; version 3.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef AALCAMERAEXPOSURECONTROL_H
> +#define AALCAMERAEXPOSURECONTROL_H
> +
> +#include <QtCore/QMap>
> +#include <QCameraExposureControl>
> +
> +#include <hybris/camera/camera_compatibility_layer_capabilities.h>
> +
> +class AalCameraService;
> +class CameraControl;
> +class CameraControlListener;
> +
> +class AalCameraExposureControl : public QCameraExposureControl
> +{
> + Q_OBJECT
> +public:
> + explicit AalCameraExposureControl(AalCameraService *service, QObject *parent = 0);
> +
> + void init(CameraControl *control, CameraControlListener *listener);
> + bool setValue(ExposureParameter parameter, const QVariant& value);
> + QVariant requestedValue(ExposureParameter parameter) const;
> + QVariant actualValue(ExposureParameter parameter) const;
> + bool isParameterSupported(ExposureParameter parameter) const;
> + QVariantList supportedParameterRange(ExposureParameter parameter, bool *continuous) const;
> +
> + static void supportedSceneModesCallback(void *context, SceneMode sceneMode);
> +
> +private:
> + QMap<SceneMode, QCameraExposure::ExposureMode> m_androidToQtExposureModes;
> + AalCameraService *m_service;
> + QVariantList m_supportedExposureModes;
> + QCameraExposure::ExposureMode m_requestedExposureMode;
> + QCameraExposure::ExposureMode m_actualExposureMode;
> +};
> +
> +#endif // AALCAMERAEXPOSURECONTROL_H
>
> === modified file 'src/aalcameraservice.cpp'
> --- src/aalcameraservice.cpp 2013-08-01 10:41:04 +0000
> +++ src/aalcameraservice.cpp 2014-07-11 18:11:38 +0000
> @@ -27,6 +27,7 @@
> #include "aalvideoencodersettingscontrol.h"
> #include "aalvideorenderercontrol.h"
> #include "aalviewfindersettingscontrol.h"
> +#include "aalcameraexposurecontrol.h"
> #include <storagemanager.h>
>
> #include <hybris/camera/camera_compatibility_layer.h>
> @@ -56,6 +57,7 @@
> m_videoEncoderControl = new AalVideoEncoderSettingsControl(this);
> m_videoOutput = new AalVideoRendererControl(this);
> m_viewfinderControl = new AalViewfinderSettingsControl(this);
> + m_exposureControl = new AalCameraExposureControl(this);
> }
>
> AalCameraService::~AalCameraService()
> @@ -74,6 +76,7 @@
> delete m_videoEncoderControl;
> delete m_videoOutput;
> delete m_viewfinderControl;
> + delete m_exposureControl;
> if (m_oldAndroidControl)
> android_camera_delete(m_oldAndroidControl);
> if (m_androidControl)
> @@ -119,6 +122,9 @@
> if (qstrcmp(name, QCameraViewfinderSettingsControl_iid) == 0)
> return m_viewfinderControl;
>
> + if (qstrcmp(name, QCameraExposureControl_iid) == 0)
> + return m_exposureControl;
> +
> return 0;
> }
>
> @@ -285,4 +291,5 @@
> m_viewfinderControl->setAspectRatio(m_videoEncoderControl->getAspectRatio());
> m_viewfinderControl->init(camControl, listener);
> m_videoOutput->init(camControl, listener);
> + m_exposureControl->init(camControl, listener);
> }
>
> === modified file 'src/aalcameraservice.h'
> --- src/aalcameraservice.h 2013-08-01 10:41:04 +0000
> +++ src/aalcameraservice.h 2014-07-11 18:11:38 +0000
> @@ -31,6 +31,7 @@
> class AalVideoEncoderSettingsControl;
> class AalVideoRendererControl;
> class AalViewfinderSettingsControl;
> +class AalCameraExposureControl;
> class QCameraControl;
>
> struct CameraControl;
> @@ -60,6 +61,7 @@
> AalVideoEncoderSettingsControl *videoEncoderControl() const { return m_videoEncoderControl; }
> AalVideoRendererControl *videoOutputControl() const { return m_videoOutput; }
> AalViewfinderSettingsControl *viewfinderControl() const { return m_viewfinderControl; }
> + AalCameraExposureControl *exposureControl() const { return m_exposureControl; }
>
> CameraControl *androidControl();
>
> @@ -99,6 +101,7 @@
> AalVideoEncoderSettingsControl *m_videoEncoderControl;
> AalVideoRendererControl *m_videoOutput;
> AalViewfinderSettingsControl *m_viewfinderControl;
> + AalCameraExposureControl *m_exposureControl;
>
> CameraControl *m_androidControl;
> CameraControlListener *m_androidListener;
>
> === modified file 'src/src.pro'
> --- src/src.pro 2014-01-16 14:20:08 +0000
> +++ src/src.pro 2014-07-11 18:11:38 +0000
> @@ -33,6 +33,7 @@
> aalvideoencodersettingscontrol.h \
> aalvideorenderercontrol.h \
> aalviewfindersettingscontrol.h \
> + aalcameraexposurecontrol.h \
> storagemanager.h
>
> SOURCES += \
> @@ -50,4 +51,5 @@
> aalvideoencodersettingscontrol.cpp \
> aalvideorenderercontrol.cpp \
> aalviewfindersettingscontrol.cpp \
> + aalcameraexposurecontrol.cpp \
> storagemanager.cpp
>
--
https://code.launchpad.net/~fboucault/qtubuntu-camera/hdr_scene_mode/+merge/225993
Your team Ubuntu Phablet Team is subscribed to branch lp:qtubuntu-camera.
More information about the Ubuntu-reviews
mailing list