Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit e4228e7a authored by Mikhail Naganov's avatar Mikhail Naganov
Browse files

audiohal: Get rid of multiple inheritance in IDevice implementation 

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
parent 3fec5a64

audio/2.0/default/Device.cpp

+47 −10
Original line number Diff line number Diff line
@@ -26,6 +26,7 @@ 
#include "Conversions.h"

#include "Device.h"

#include "HidlUtils.h"

#include "ParametersUtil.h"

#include "StreamIn.h"

#include "StreamOut.h"
@@ -35,10 +36,29 @@ namespace audio { 
namespace V2_0 {

namespace implementation {



Device::Device(audio_hw_device_t* device)

        : mDevice(device) {

namespace {



class ParametersUtilImpl : public ParametersUtil {

   public:

    ParametersUtilImpl(audio_hw_device_t* device) : mDevice{device} {}



    char* halGetParameters(const char* keys) override {

        return mDevice->get_parameters(mDevice, keys);

    }



    int halSetParameters(const char* keysAndValues) override {

        return mDevice->set_parameters(mDevice, keysAndValues);

    }



   private:

    audio_hw_device_t* mDevice;

};



}  // namespace



Device::Device(audio_hw_device_t* device)

    : mDevice{device}, mParameters{new ParametersUtilImpl(mDevice)} {}



Device::~Device() {

    int status = audio_hw_device_close(mDevice);

    ALOGW_IF(status, "Error closing audio hw device %p: %s", mDevice, strerror(-status));
@@ -67,12 +87,28 @@ void Device::closeOutputStream(audio_stream_out_t* stream) { 
    mDevice->close_output_stream(mDevice, stream);

}



char* Device::halGetParameters(const char* keys) {

    return mDevice->get_parameters(mDevice, keys);

Result Device::getParam(const char* name, bool* value) {

    return mParameters->getParam(name, value);

}



int Device::halSetParameters(const char* keysAndValues) {

    return mDevice->set_parameters(mDevice, keysAndValues);

Result Device::getParam(const char* name, int* value) {

    return mParameters->getParam(name, value);

}



Result Device::getParam(const char* name, String8* value) {

    return mParameters->getParam(name, value);

}



Result Device::setParam(const char* name, bool value) {

    return mParameters->setParam(name, value);

}



Result Device::setParam(const char* name, int value) {

    return mParameters->setParam(name, value);

}



Result Device::setParam(const char* name, const char* value) {

    return mParameters->setParam(name, value);

}



// Methods from ::android::hardware::audio::V2_0::IDevice follow.
@@ -274,21 +310,22 @@ Return<Result> Device::setAudioPortConfig(const AudioPortConfig& config) { 


Return<AudioHwSync> Device::getHwAvSync()  {

    int halHwAvSync;

    Result retval = getParam(AudioParameter::keyHwAvSync, &halHwAvSync);

    Result retval =

        mParameters->getParam(AudioParameter::keyHwAvSync, &halHwAvSync);

    return retval == Result::OK ? halHwAvSync : AUDIO_HW_SYNC_INVALID;

}



Return<Result> Device::setScreenState(bool turnedOn)  {

    return setParam(AudioParameter::keyScreenState, turnedOn);

    return mParameters->setParam(AudioParameter::keyScreenState, turnedOn);

}



Return<void> Device::getParameters(const hidl_vec<hidl_string>& keys, getParameters_cb _hidl_cb)  {

    getParametersImpl(keys, _hidl_cb);

    mParameters->getParametersImpl(keys, _hidl_cb);

    return Void();

}



Return<Result> Device::setParameters(const hidl_vec<ParameterValue>& parameters)  {

    return setParametersImpl(parameters);

    return mParameters->setParametersImpl(parameters);

}



Return<void> Device::debugDump(const hidl_handle& fd)  {

audio/2.0/default/Device.h

+12 −9
Original line number Diff line number Diff line
@@ -27,14 +27,14 @@ 


#include <hidl/MQDescriptor.h>



#include "ParametersUtil.h"



namespace android {

namespace hardware {

namespace audio {

namespace V2_0 {

namespace implementation {



class ParametersUtil;



using ::android::hardware::audio::common::V2_0::AudioConfig;

using ::android::hardware::audio::common::V2_0::AudioHwSync;

using ::android::hardware::audio::common::V2_0::AudioInputFlag;
@@ -55,7 +55,7 @@ using ::android::hardware::hidl_vec; 
using ::android::hardware::hidl_string;

using ::android::sp;



struct Device : public IDevice, public ParametersUtil {

struct Device : public IDevice {

    explicit Device(audio_hw_device_t* device);



    // Methods from ::android::hardware::audio::V2_0::IDevice follow.
@@ -101,16 +101,19 @@ struct Device : public IDevice, public ParametersUtil { 
    void closeInputStream(audio_stream_in_t* stream);

    void closeOutputStream(audio_stream_out_t* stream);

    audio_hw_device_t* device() const { return mDevice; }

    Result getParam(const char* name, bool* value);

    Result getParam(const char* name, int* value);

    Result getParam(const char* name, String8* value);

    Result setParam(const char* name, bool value);

    Result setParam(const char* name, int value);

    Result setParam(const char* name, const char* value);



   private:

    audio_hw_device_t *mDevice;

    std::unique_ptr<ParametersUtil> mParameters;



    virtual ~Device();



    // Methods from ParametersUtil.

    char* halGetParameters(const char* keys) override;

    int halSetParameters(const char* keysAndValues) override;



    uint32_t version() const { return mDevice->common.version; }

};

audio/2.0/default/ParametersUtil.h

+15 −14
Original line number Diff line number Diff line
@@ -37,12 +37,15 @@ using ::android::hardware::hidl_vec; 


class ParametersUtil {

  public:

   virtual ~ParametersUtil() = default;

   Result getParam(const char* name, bool* value);

   Result getParam(const char* name, int* value);

   Result getParam(const char* name, String8* value);

   void getParametersImpl(

       const hidl_vec<hidl_string>& keys,

            std::function<void(Result retval, const hidl_vec<ParameterValue>& parameters)> cb);

       std::function<void(Result retval,

                          const hidl_vec<ParameterValue>& parameters)>

           cb);

   std::unique_ptr<AudioParameter> getParams(const AudioParameter& keys);

   Result setParam(const char* name, bool value);

   Result setParam(const char* name, int value);
@@ -51,8 +54,6 @@ class ParametersUtil { 
   Result setParams(const AudioParameter& param);



  protected:

    virtual ~ParametersUtil() {}



    virtual char* halGetParameters(const char* keys) = 0;

    virtual int halSetParameters(const char* keysAndValues) = 0;

};