Just return an `std::unique_ptr` constructed from an empty
initializer instead of doing a `reset()` on the existing
`config` variable and returning that. This is simpler.
Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
inputBufferReady ready signal in the simple pipeline is handled in the
pipeline handler thread. outputBufferReady and ispStatsReady signals
should be handled there too.
Rather than relying on the user of the SoftwareIsp instance, let
SoftwareIsp inherits Object. SoftwareIsp serves as a signal proxy, the
signals above are emitted from signal handlers. This means that if
SoftwareIsp inherits Object then the slots are invoked in SoftwareIsp
thread. Which is the camera manager thread because the SoftwareIsp
instance is created there.
Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
The rkisp1 and rpi pipeline handlers duplicate code to handle the
LIBCAMERA_RKISP1_TUNING_FILE and LIBCAMERA_RPI_TUNING_FILE environment
variables that override tuning file selection. Move the common code to
IPAProxy::configurationFile() to avoid the duplication, and make the
feature available to all pipeline handlers with the same behaviour.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com>
Port the UVC pipeline handler to use the new ExposureTimeMode control
when processing Camera controls in place of the AeEnable control.
The V4L2_CID_EXPOSURE_AUTO control allows 4 possible values, which
map to ExposureTimeModeAuto and ExposureTimeModeManual.
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Install the example configuration file of the virtual pipeline
handler as it serves documentation purposes, and to make the
virtual pipeline handler easily usable in CI.
Nonetheless, the file is installed with the ".example" suffix
so that it will not be used by default, to avoid cluttering
the camera lists of users whose distributions decide to
enable the virtual pipeline handler.
The file is installed in the proper location for convenience:
(1) is is easier to use it in the CI;
(2) users need not browse documentation to determine where
they should place the file.
Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Harvey Yang <chenghaoyang@chromium.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
The virtual pipeline handler prints an error message when its
configuration file can't be opened. Not providing a configuration file
is the default method to disable virtual cameras, so this error is
confusing for users. Replace it with a debug message when the
configuration file is not found, and keep an error message when it
exists but can't be opened.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Tested-by: Julien Vuillaumier <julien.vuillaumier@nxp.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
After the initial generation, when a frame is requested,
the test pattern generator rotates the image left by 1 column.
The current approach has two shortcomings:
(1) it allocates a temporary buffer to hold one column;
(2) it swaps two columns at a time.
The test patterns are simple ARGB images, in row-major order,
so doing (2) works against memory prefetching. This can be
addressed by doing the rotation one row at a time as that way
the image is addressed in a purely linear fashion. Doing so also
eliminates the need for a dynamically allocated temporary buffer,
as the required buffer now only needs to hold one sample,
which is 4 bytes in this case.
In an optimized build, this results in about a 2x increase in the
number of frames per second as reported by `cam`. In an unoptimized,
ASAN and UBSAN intrumented build, the difference is even bigger,
which is useful for running lc-compliance in CI in a reasonable time.
Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Tested-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
`PixelFormatInfo::planes.size()` always returns 3 since `planes` is
an array, but that is not the number of planes of the pixel format.
Use the `numPlanes()` getter instead.
Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Plumb the Pipeline-IPA loop in.
Load the IPA module at camera creation time and create the loop between
the pipeline and the IPA.
When a new Request is queued the IPA is asked to prepare the parameters
buffer, once ready it notifies the pipeline which queues the parameters
to the ISP along with a buffer for statistics and frames,
Once statistics are ready they get passed to the IPA which upates its
settings for the next frame.
Driveby fix an error message in the Pipeline Handler's ::freeBuffers()
function which reported a problem with the wrong video device in an
error path.
Acked-by: Nayden Kanchev <nayden.kanchev@arm.com>
Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
Acked-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Acquire the mali-c55 3a stats and parameters video devices during
::match() and plumb them in.
For this commit we simply allocate and release buffers for the
statistics and parameters. Statistics buffers are queue and dequeued
from the stats video device but their contents are for now untouched.
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Acked-by: Nayden Kanchev <nayden.kanchev@arm.com>
Co-developed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
Now that we have camera sensor control application delay values in
the CameraSensorProperties class, remove the duplicated definitions
in the RPi IPA's CameraSensorHelpers and update the pipeline handler
to use the values from CameraSensorProperties.
Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
Reviewed-by: Naushir Patuck <naush@raspberrypi.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
In RkISPPath::validate() the sensor sizes are correctly filtered to the
ones supported by the ISP. But later in RkISPPipeline::configure() the
configured stream size is passed to sensor->getFormat() and the
CameraSensor class chooses the best sensor format for the requested
stream size. This can result in a sensor format that is too big for the
ISP. Fix that by supplying the maximum resolution supported by the ISP
to setFormat().
Fixes: 761545407c ("pipeline: rkisp1: Filter out sensor sizes not supported by the pipeline")
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
When the dewarper is used, config->validate() needs to take the
restrictions of the dewarper into account. Add the corresponding checks.
As the useDewarper_ variable is now accessed earlier in
PipelineHandlerRkISP1::configure(), ensure it gets set early enough.
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
For the validate() implementation, the RkISP1CameraConfiguration needs
access to dewarper related members of the PipelineHandlerRkISP1 object.
Allow this access by adding const versions of the pipe accessors and
making RkISP1CameraConfiguration a friend of PipelineHandlerRkISP1.
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
In configure() and in the future in generateConfiguration() the
calculated stream sizes and crop rectangles depend on the dewarper being
available or not. It is therefore not possible to disable the dewarper
later if configuration fails. Error out in that case.
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Refactor validation code to prepare for extensions in the upcoming
patches. Code duplication is reduced by moving parts of the validation
logic into a lambda function. This patch does not include any functional
changes.
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
ScalerCrop is specified as being in sensor coordinates. The current
dewarper implementation on the imx8mp handles ScalerCrop in dewarper
coordinates. This leads to unexpected results and an unusable ScalerCrop
control in camshark. Fix that by transforming back and forth between
sensor coordinates and dewarper coordinates.
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Query the crop bounds on the dewarper instead of the stream in case the
camera was not yet configured when updateControls() gets called. This
provides sane defaults for the controls.
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
The sensorInfo (specifically the crop rectangle of the selected sensor
mode) is collected to be passed to the IPA later. In an upcoming patch
that data will also be needed for correct ScalerCrop handling. Move the
collection of the sensorInfo before the dewarper configuration step and
refactor the code a bit.
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
One Rectangle instance is used to calculate the inputCrop and the
outputCrop of the ISP in the rkisp1 pipeline. Split that into two
distinct variables, because both values will be needed in the upcoming
patches. This patch does not contain any functional changes.
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
In the current code, the input stage of the image resizer is used to
apply a crop to keep the aspect ratio in cases where the requested
output aspect ratio differs from the one of the selected sensor mode. On
the imx8mp the resizer hardware is not capable of cropping (for
reference see also rkisp1-resizer.c:rkisp1_rsz_set_sink_crop() in the
linux kernel v6.10).
Therefore apply the necessary cropping on the output of the ISP (on the
image stabilization block). The cropping code on the image resizer
doesn't need modifications as the requested crop gets ignored by the
kernel.
While at it, remove a todo comment that is no longer needed.
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
This patch introduces support for applying runtime controls to software
ISP. It enables the contrast control as the first control that can be
used.
Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Implement support for the ScalerCrop control that allows to apply a
digital zoom to the captured streams.
Initialize the camera controls at camera registration time and update
them at configure time as the sensor's analogue crop size might change
depending on the desired Camera configuration.
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
The Mali C55 ISP has a resizing pipeline that allows to crop and scale
images.
So far the mali-c55 pipeline has only supported cropping without using
the scaling functionalities.
Now that the kernel has gained support for the scaling operations, make
the libcamera pipeline use it by combining it with a first cropping step
to align the input and output images FOV ratio, and then scale to the
desired output size.
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
The mali-c55 driver now expects links to video devices to be enabled
in order for those devices to be streamed from / to. Enable the media
link between the resizers and their associated video device to fulfil
the requirement.
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
At present we configure raw streams by looping through the pixel
formats we support and finding one with an associated media bus
format code that the sensor can produce. In the new representation
of raw data from the kernel driver this will not work - the sensor
could produce 8, 10, 12, 14 or 16 bit data and the ISP will force
it to RAW16, which is the only actually supported output.
To fix the issue move to simply finding a pixel format with a bayer
order that matches that of the media bus format produced by the
sensor. If the sensor can produce multiple formats with the same
bayer order use the one with the largest bitdepth.
Finally, remove the claim to support RAW formats of less than 16 bits.
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
The latest version of the Mali C55 driver has changed the format
accepted by the ISP sink pad to be the 20-bit wide.
The CSI-2 receiver handles the format expansion internally by
propagating the sensor produced format from its sink to the 20-bit
expanded version on its source pad.
Instead of re-applying the sensor format to the CSI-2 receiver source
pad (which is now an invalid operation) read from there the format
propagated by the driver internally and further propagate it to the ISP
subdevice.
Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
Complete the RAW StreamConfiguration by populating the frame stride and
the frame size.
Set the minimum required alignment to 4 bytes as the Mali C55 ISP output
expands RAW output to 16 bits and a RAW Bayer macro-pixel requires
two samples to be complete.
Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
The PipelineHandler::completeBuffer() base class function returns
a boolean to indicate if there still are pending buffers in the
Request.
Simplify the bufferReady() function in the Mali-C55 pipeline handler
using the completeBuffer() return value.
Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
The Mali C55 ISP cannot upscale. The maximum allowed output size
is the sensor's resolution.
For RAW streams this is already handled in adjustRawSizes(), while
for processed streams the maximum allowed resolution was wrongly
set to the ISP maximum output size (8192x8192).
Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
A copy is made in the range-based for loop, and thus all modifications
done in the for loop body are lost, and not actually applied to
the object in the container.
Fix that by taking a reference in the range-based for loop.
Fixes: 4217c9f1aa ("libcamera: camera: Zero streams before validate()")
Fixes: 613d540267 ("pipeline: raspberrypi: Fix handling of colour spaces")
Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Naushir Patuck <naush@raspberrypi.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
This patch introduces the configuration file for Virtual Pipeline
Handler. The config file is written in yaml, and the format is
documented in `README.md`.
The config file will define the camera with IDs, supported formats and
image sources, etc. In the default config file, only Test Patterns are
used. Developers can use real images loading if desired.
Signed-off-by: Konami Shu <konamiz@google.com>
Co-developed-by: Harvey Yang <chenghaoyang@chromium.org>
Signed-off-by: Harvey Yang <chenghaoyang@chromium.org>
Co-developed-by: Yunke Cao <yunkec@chromium.org>
Signed-off-by: Yunke Cao <yunkec@chromium.org>
Co-developed-by: Tomasz Figa <tfiga@chromium.org>
Signed-off-by: Tomasz Figa <tfiga@chromium.org>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Add a test pattern generator class hierarchy for the Virtual
pipeline handler.
Implement two types of test patterns: color bars and diagonal lines
generator and use them in the Virtual pipeline handler.
A shifting mechanism is enabled. For each frame, the image is shifted to
the left by 1 pixel. It drops FPS though.
Add a dependency for libyuv to the build system to generate images
in NV12 format from the test pattern.
Signed-off-by: Konami Shu <konamiz@google.com>
Co-developed-by: Harvey Yang <chenghaoyang@chromium.org>
Signed-off-by: Harvey Yang <chenghaoyang@chromium.org>
Co-developed-by: Yunke Cao <yunkec@chromium.org>
Signed-off-by: Yunke Cao <yunkec@chromium.org>
Co-developed-by: Tomasz Figa <tfiga@chromium.org>
Signed-off-by: Tomasz Figa <tfiga@chromium.org>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
The names used by the IPA interface and the names used for buffer
completions handlers in libcamera clash in the use of the term "buffer".
For example video device buffer completion handler is called
"bufferReady" and the IPA event to ask the IPA to compute parameters are
called "fillParamsBuffers". This makes it hard to recognize which
function handles video device completion signals and which ones handle
the IPA interface events.
Rationalize the naming scheme in the IPA interface function and events
and the signal handlers in the pipelines, according to the
following table. Remove the name "buffer" from the IPA interface events
and events handler and reserve it for the buffer completion handlers.
Rename the IPA interface events and function to use the 'params' and
'stats' names as well.
IPA Interface:
- fillParamsBuffer -> computeParams [FUNCTION]
- processStatsBuffer -> processStats [FUNCTION]
- paramFilled -> paramsComputed [EVENT]
Pipeline handler:
- bufferReady -> videoBufferReady [BUFFER HANDLER]
- paramReady -> paramBufferReady [BUFFER HANDLER]
- statReady -> statBufferReady [BUFFER HANDLER]
- paramFilled -> paramsComputed [IPA EVENT HANDLER]
Cosmetic change only, no functional changes intended.
Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Reviewed-by: Milan Zamazal <mzamazal@redhat.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
PipelineHandler::stop() calls stopDevice() method to perform pipeline
specific cleanup and then completes waiting requests. If any queued
requests remain, an assertion error is raised.
Software ISP stores request buffers in
SimpleCameraData::conversionQueue_ and queues them as V4L2 signals
bufferReady. stopDevice() cleanup forgets to clean up the buffers and
their requests from conversionQueue_, possibly resulting in the
assertion error. This patch fixes the omission.
The problem wasn't very visible when
SimplePipelineHandler::kNumInternalBuffers (the number of buffers
allocated in V4L2) was equal to the number of buffers exported from
software ISP. But when the number of the exported buffers was increased
by one in commit abe2ec64f9, the assertion
error started pop up in some environments. Increasing the number of the
buffers much more, e.g. to 9, makes the problem very reproducible.
Each pipeline uses its own mechanism to track the requests to clean up
and it can't be excluded that similar omissions are present in other
places. But there is no obvious way to make a common cleanup for all
the pipelines (except for doing it instead of raising the assertion
error, which is probably undesirable, in order not to hide incomplete
pipeline specific cleanups).
Bug: https://bugs.libcamera.org/show_bug.cgi?id=234
Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Tested-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Tested-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Simple pipeline retrieves the requests to complete from the
conversionQueue_. This patch stores the requests in conversionQueue_
explicitly. This explicit tracking is supposed to be preferred to
implicit retrieval and it simplifies the completion code a bit here and
in the followup patch that adds request cleanup on stop.
The change as implemented assumes that all the buffers in each of the
conversionQueue_ elements point to the same request, the one specified.
Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
Tested-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Tested-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
The "useDewarper_" class variable is set to true during configure
only if the configuration is !isRaw.
The main path buffer pool creation can thus be moved out of the
if (!isRaw) block at allocateBuffers() time.
Cosmetic change that will make it easier to create a buffer pool
for the main and self paths unconditionally in future.
Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>