DebugDump does not return an error code, thus the implementation can not
return not implemented.
As a result, the test can not expect any output from the function.
Only test that the call does not crash and add a log if the function is
probably not implemented in the test.
Test: vts-tradefed run vts --module VtsHalAudioV2_0Target
Test: call/play music/record/video...
Bug: 36311550
Change-Id: I2c18958bceb1eb638491f9afce9d8e8025ccd3ec
Signed-off-by: Kevin Rocard <krocard@google.com>
Return an error if framesCount or frameSize are null to avoid a division
by zero when calculating the buffer size.
The message queues are allocated with a buffer size but if two big they will
assert not return an error.
Thus take some margin on the buffer size check.
Note that both function should be refactored as 99% identical.
Test: vts-tradefed run vts --module VtsHalAudioV2_0Target
Test: call/play music/record/video...
Bug: 36311550
Change-Id: I0576e9016ef2e567c8d4e171c6237883d9865db9
Signed-off-by: Kevin Rocard <krocard@google.com>
Those functions had lots of copy paste on errors and
the following patch will even add more error detections.
Refactor the hidl_cb call to avoid all duplication.
Note that both function should be refactored as 99% identical.
Test: vts-tradefed run vts --module VtsHalAudioV2_0Target
Test: call/play music/record/video...
Bug: 36311550
Change-Id: I40d6926b4f9f5e3aba51e878f55fb013f4ca09c1
Signed-off-by: Kevin Rocard <krocard@google.com>
Some values of AudioMode are exposed although implementation detail.
Make sure the client can not use them.
Test: vts-tradefed run vts --module VtsHalAudioV2_0Target
Test: call/play music/record/video...
Bug: 36311550
Change-Id: If513c2a06efa8a92459f0af80c63232fc63302b0
Signed-off-by: Kevin Rocard <krocard@google.com>
setHwAvSync and setParameters were implemented in the pre-hidl interface
as set_parameters.
Unfortunately set_parameters did not return an error if a key was not
implemented.
As most HIDL implementation will be a wrapper around the pre-hidl
interface, allow those functions to return OK on not implemented key.
Test: vts-tradefed run vts --module VtsHalAudioV2_0Target
Test: call/play music/record/video...
Bug: 36311550
Change-Id: Icfcaa02b7d63e03375fddc90dc5a803754c1874f
Signed-off-by: Kevin Rocard <krocard@google.com>
When sending parameters to the HAL (and some getters are implemented
with getParameters), the client expect a status consistent
with the other HIDL methods. Ie: not implemented or success and failure.
Unfortunately, the legacy get_parameter interface, which currently most
Audio HIDL implementation are a wrapper around, do not return such error
code.
Get parameters return a list of key values.
- If a requested key does not return a key value pair, consider it not
implemented
- If a requested key returns a key not followed by a correct value,
consider it a failure
- otherwise it is a success
Test: vts-tradefed run vts --module VtsHalAudioV2_0Target
Test: call/play music/record/video...
Bug: 36311550
Change-Id: Id6711e9c1974fe5a336b6de83a9b6d14f74437c9
Signed-off-by: Kevin Rocard <krocard@google.com>
In order to avoid style inconsistency as well as non functional
modification in following patches,
fix the style of all files modified for the fix of the VTS tests.
Patch generated with:
$ clang-format -i --style file -- <file list>
Bug: 36311550
Test: compile
Change-Id: I53dbcdabf959a6100e34a2ee4d0f951d525049cb
This is intended to clarify whether surprising zeroing out of HAL
control structures happens to a particular HAL module, or to any
HAL module at random.
Unfortunately, the crash itself can't be prevented as
audioflinger can't work around a HAL outage, and needs to be
restarted anyway.
Bug: 36225019
Test: verified that the logged string contains the information
Change-Id: I5843d89b4e5385b4ce269f72b5891ccb646daeba
This reverts commit e4228e7aa6.
Further analysis of the bug has shown that the problem is not related
to the use of multiple inheritance.
Bug: 36225019
Change-Id: Icdcaf84061c0ecf29f891521bd961c3ee89f1cfc
We still not sure what causes crashes in Device::get|setParam*,
but it seems that it is somehow caused by the fact that the parameters
code is in a separate class with virtual methods, from which
Device class inherits along with IDevice interface.
The workaround is to substitute multiple inheritance with
delegation in Device class. Hopefully this will either eliminate
crashes or make the underlying reasons more clear.
Some of the code got reformatted by clang-format as a presubmit
requirement.
Bug: 36225019
Test: make
Change-Id: Id785c3565bbebd5acc26ca46472961698d9c6208
Add checks for validity of the passed in native handle.
Bug: 37590777
Change-Id: If906f0086c3442819e1844cc521215d5b74af015
Test: VTS
(cherry picked from commit 7bae6a0b63)
Initialize audio patch handle to avoid
uninitialized value causing validation
failure in vendor hal audio patch.
Bug: 36994757
Test: Initiate voice call, no audio heard from other end
Change-Id: I4cf57210432430af89109f605813645ff332bfbb
prepare for write was refactor by Ib4170eb6a9f88f9352d0912083b43d600771bb8e
to use temporary variable in a rai pattern.
Nevertheless the member variable was still used too early in
prepareForRead resulting in a crash for capture.
Test: assistant, camcorder
Bug: 37492059
Change-Id: I08140834959c440798cd40700dd089adca2f3e40
Signed-off-by: Kevin Rocard <krocard@google.com>
"default" implies using "primary" legacy device module.
This doesn't change anything for binderized mode, because
the server was only registering itself as "sound_trigger.primary",
so it was the only possibility anyways.
Usage of "stub" (debugging) module is still possible
if audioflinger and corresponding services are built with
USE_LEGACY_LOCAL_AUDIO_HAL=true (added makefile check to ensure
that).
Test: "Ok Google" from screen off state on sailfish
Bug: 36570720
Change-Id: I88f1ebd4c1efd0f91e37458746c336a4beef33ae
If prepareToWrite or prepareToRead fails after EventFlag is created, it
is not destroyed. This lead to strange random crashes (double free it seems).
Use the RAII pattern to manage the EventFlag life cycle.
Test: Run test on target
Bug: 36311550
Change-Id: I53a04a62b7d12fdcc94afd8ced3e547aa6edff50
Signed-off-by: Kevin Rocard <krocard@google.com>
The audio buffer size is not provided by the client, it is computed from
the sample size and the number of sample.
No check was done as if the multiplication of these two numbers would
produce an overflow. This leaded to erroneous memory access crashing the
media server.
Test: Run on target
Bug: 36311550
Change-Id: I3436800ab6ac1b5e6a6aa4d03d6b96910eb54652
Signed-off-by: Kevin Rocard <krocard@google.com>
If the requested buffer was too big, memory allocation would fail,
resulting if a audio hal crash (uncatch exception thrown by new).
Properly hadle the failure by retuning INVALID_PARAMETERS in such case.
Bug: 36311550
Test: Run test on target
Change-Id: Ib4170eb6a9f88f9352d0912083b43d600771bb8e
Signed-off-by: Kevin Rocard <krocard@google.com>
The result status was not checked.
Test: Run test on target
Bug: 36311550
Change-Id: I197b52d0b5a1276d3e3beba105bb91639f89e060
Signed-off-by: Kevin Rocard <krocard@google.com>
Some test did not have any documentation
Test: compile & run
Bug: 36311550
Change-Id: I37c40f6f17993a275e5c40b9a835ac04acf4f8e6
Signed-off-by: Kevin Rocard <krocard@google.com>
Stream objects used to hold a pointer to underlying HAL device
object which they didn't own. Since destruction of server side
objects is asynchronous, it was possible that a Device object
gets destroyed before Stream objects, making all the HAL device
object pointer to become stale.
Fixed by adding a strong reference to Device objects into Stream
objects.
Bug: 36702804
Change-Id: I3da3611afbb91d6fd6410ac5b8af2a2eebfa6dac
Test: ran Loopback app and HAL VTS tests
(cherry picked from commit 96d3573cda)
Return are movable only, thus the fact that assertResult took a copy of
the value meant that the callee had to provide an rvalue.
This was not needed as the Result was not modified. Change it to a const
reference.
Test: run test on board
Bug: 34170075
Signed-off-by: Kevin Rocard <krocard@google.com>
For now most tests are negative.
Test: run test on board
Bug: 34170075
Change-Id: Id207ae310e0fa5a3ab4db49122249594cd2c2703
Signed-off-by: Kevin Rocard <krocard@google.com>
Add negative test for the remaining methods of IStream
and positive one for most.
Test: run test on board
Bug: 34170075
Change-Id: Ib18c6d2e4d0a14ee4517f31319ddecf5e23ac8de
Signed-off-by: Kevin Rocard <krocard@google.com>
As some part of this big test were failing, it was difficult to actually
track on the dashboard what was wrong.
Split it in small independent equivalent tests.
Test: mm & run on target
Bug: 34170075
Change-Id: I03686e55ec112f0f0c0ef2683c5dc1ae79ff8d5e
Signed-off-by: Kevin Rocard <krocard@google.com>
This eases the debuging of failing tests.
Also print hex value of the enums as well of their literal values.
Test: run the test on target
Bug: 34170075
Change-Id: Iea1d7d2269e0e5b413240aaa905daae755846e74
Signed-off-by: Kevin Rocard <krocard@google.com>
ASSERT_INVALID_ARGUMENTS was a macro that asserted that a given Result
or Return contained INVALID_ARGUMENT. The problem was that a result can
have lots of other values like INVALID_STATE or NOT_SUPPORTED.
Additionally not all test expect only one possible result.
Introduce two overload of ASSERT_RESULT()
The first one takes an expected Result value and compare it to the
obtained one.
The second take a list and expect the obtained one to be in this list.
Test: run the test on target
Bug: 34170075
Change-Id: I798729f27f723c98292610bfb43dbdb2724ec2ca
Signed-off-by: Kevin Rocard <krocard@google.com>
The gtest dashboard only shows test names, not their parameters.
This mean that parametrized tests are names 0,1,2...
Thus when one test fails, the dashboard info is not very useful.
This patch appends the audio config to the test name.
Test: run the test on target
Bug: 34170075
Change-Id: I77090a3a17e4a3e97ecb72a8a89ad2d88626d96f
Signed-off-by: Kevin Rocard <krocard@google.com>
Master volume was the first pair of getter/setter tested
and did not use the generic test*Accessor function.
Test: run the corresponding vts test
Test: vts-tradefed r vts-hal-hidl -m VtsHalAudioV2_0Target
Bug: 34170075
Change-Id: I77080a3a17e4a3e97ecb72a8a89ad2d88626d96f
Signed-off-by: Kevin Rocard <krocard@google.com>
The IDevice::debugDump method dumps debug info in a file descriptor.
Such file descriptor was previously the writing end of a pipe.
As the test is not multithreaded, if the dump was bigger than the pipe
buffer, a deadlock would occur.
With this patch, the test uses a file instead of a pipe. Thus write
will never block infinitely.
Test: run the corresponding vts test
Test: vts-tradefed r vts-hal-hidl -m VtsHalAudioV2_0Target
Bug: 34170075
Change-Id: I928cae712a1cb4411f907b3a9583014ba6486abc
Signed-off-by: Kevin Rocard <krocard@google.com>
Its base class was tested but not the primary
device getters and setters.
Test: run the corresponding vts test
Test: vts-tradefed r vts-hal-hidl -m VtsHalAudioV2_0Target
Bug: 34170075
Change-Id: Ic451761c7b8b29fd302a62aba83be6ce7be35f64
Signed-off-by: Kevin Rocard <krocard@google.com>
Test: run the corresponding vts test
Test: vts-tradefed r vts-hal-hidl -m VtsHalAudioV2_0Target
Bug: 34170075
Change-Id: I0652f1bf748de99a8a2594630daf5ea2acec8934
Signed-off-by: Kevin Rocard <krocard@google.com>
Previously failing tests were commented because I thought
our implementation had to pass all tests.
As a mater of facts, it is not the case and tests are allowed to fail.
Bugs will be raised for those failure.
This is much better way to track the fixes that FIXMEs.
Test: run the corresponding vts test
Test: vts-tradefed r vts-hal-hidl -m VtsHalAudioV2_0Target
Bug: 34170075
Change-Id: I928cae712a1cb4411f907b3a9583014ba6487abd
Signed-off-by: Kevin Rocard <krocard@google.com>
Do not close the native fd for shared memory passed by
the legacy HAL as it is owned by tinyalsa implementation.
Test: test MMAP NO IRQ playback and capture streams
Bug: 31458351
Change-Id: Ie0c2a38574e5ac5c13fdb8a5c81fd02cf7d0e47a
Test: run the vts test on target
Test: vts-tradefed r vts-hal-hidl -m VtsHalAudioV2_0Target
Bug: 34170075
Change-Id: I4bd6cb0aa5b7cc628537cd7c024542c4db8b592d
Signed-off-by: Kevin Rocard <krocard@google.com>
