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

Commit 5096385d authored by Pawin Vongmasa's avatar Pawin Vongmasa
Browse files

Make a separate MediaPlayer2Base::NotifyCallback 

This avoids the collision with notify_callback_f.

Also, this CL should fix the potential use-after-free in notify().

Test: Builds properly
Bug: 70546581
Change-Id: Ia4919522d00c94591d6f14a27cb6465a8c4efaeb
parent 2eee2bf9

media/libmedia/MediaPlayer2Factory.cpp

+3 −3
Original line number Diff line number Diff line
@@ -126,8 +126,8 @@ player2_type MediaPlayer2Factory::getPlayerType(const sp<MediaPlayer2Engine>& cl 


sp<MediaPlayer2Base> MediaPlayer2Factory::createPlayer(

        player2_type playerType,

        void* cookie,

        notify_callback_f notifyFunc,

        const wp<MediaPlayer2Engine> &client,

        MediaPlayer2Base::NotifyCallback notifyFunc,

        pid_t pid) {

    sp<MediaPlayer2Base> p;

    IFactory* factory;
@@ -152,7 +152,7 @@ sp<MediaPlayer2Base> MediaPlayer2Factory::createPlayer( 


    init_result = p->initCheck();

    if (init_result == NO_ERROR) {

        p->setNotifyCallback(cookie, notifyFunc);

        p->setNotifyCallback(client, notifyFunc);

    } else {

        ALOGE("Failed to create player object of type %d, initCheck failed"

              " (res = %d)", playerType, init_result);

media/libmedia/MediaPlayer2Factory.h

+2 −2
Original line number Diff line number Diff line
@@ -65,8 +65,8 @@ class MediaPlayer2Factory { 
                                      const sp<DataSource> &source);



    static sp<MediaPlayer2Base> createPlayer(player2_type playerType,

                                             void* cookie,

                                             notify_callback_f notifyFunc,

                                             const wp<MediaPlayer2Engine> &client,

                                             MediaPlayer2Base::NotifyCallback notifyFunc,

                                             pid_t pid);



    static void registerBuiltinFactories();

media/libmedia/MediaPlayer2Manager.cpp

+8 −5
Original line number Diff line number Diff line
@@ -1258,12 +1258,13 @@ status_t MediaPlayer2Manager::Client::getRetransmitEndpoint( 
}



void MediaPlayer2Manager::Client::notify(

        void* cookie, int msg, int ext1, int ext2, const Parcel *obj)

        const wp<MediaPlayer2Engine> &listener, int msg, int ext1, int ext2, const Parcel *obj)

{

    Client* client = static_cast<Client*>(cookie);

    if (client == NULL) {

    sp<MediaPlayer2Engine> spListener = listener.promote();

    if (spListener == NULL) {

        return;

    }

    Client* client = static_cast<Client*>(spListener.get());



    sp<MediaPlayer2EngineClient> c;

    sp<Client> nextClient;
@@ -1311,7 +1312,7 @@ void MediaPlayer2Manager::Client::notify( 
    }



    if (c != NULL) {

        ALOGV("[%d] notify (%p, %d, %d, %d)", client->mConnId, cookie, msg, ext1, ext2);

        ALOGV("[%d] notify (%p, %d, %d, %d)", client->mConnId, spListener.get(), msg, ext1, ext2);

        c->notify(msg, ext1, ext2, obj);

    }

}
@@ -1405,7 +1406,9 @@ status_t MediaPlayer2Manager::Client::enableAudioDeviceCallback(bool enabled) 
#if CALLBACK_ANTAGONIZER

const int Antagonizer::interval = 10000; // 10 msecs



Antagonizer::Antagonizer(notify_callback_f cb, void* client) :

Antagonizer::Antagonizer(

        MediaPlayer2Manager::NotifyCallback cb,

        const wp<MediaPlayer2Engine> &client) :

    mExit(false), mActive(false), mClient(client), mCb(cb)

{

    createThread(callbackThread, this);

media/libmedia/MediaPlayer2Manager.h

+11 −9
Original line number Diff line number Diff line
@@ -46,7 +46,9 @@ class MediaPlayer2EngineClient; 
#if CALLBACK_ANTAGONIZER

class Antagonizer {

public:

    Antagonizer(notify_callback_f cb, void* client);

    Antagonizer(

            MediaPlayer2Base::NotifyCallback cb,

            const wp<MediaPlayer2Engine> &client);

    void start() { mActive = true; }

    void stop() { mActive = false; }

    void kill();
@@ -58,8 +60,8 @@ private: 
    Condition                        mCondition;

    bool                             mExit;

    bool                             mActive;

    void*               mClient;

    notify_callback_f   mCb;

    wp<MediaPlayer2Engine>           mClient;

    MediaPlayer2Base::NotifyCallback mCb;

};

#endif
@@ -301,7 +303,7 @@ private: 
        status_t                setDataSource_post(const sp<MediaPlayer2Base>& p,

                                                   status_t status);



        static  void            notify(void* cookie, int msg,

        static  void            notify(const wp<MediaPlayer2Engine> &listener, int msg,

                                       int ext1, int ext2, const Parcel *obj);



                pid_t           pid() const { return mPid; }

media/libmedia/include/media/MediaPlayer2Interface.h

+14 −14
Original line number Diff line number Diff line
@@ -67,14 +67,14 @@ enum player2_type { 
// duration below which we do not allow deep audio buffering

#define AUDIO_SINK_MIN_DEEP_BUFFER_DURATION_US 5000000



// callback mechanism for passing messages to MediaPlayer2 object

typedef void (*notify_callback_f)(void* cookie,

        int msg, int ext1, int ext2, const Parcel *obj);



// abstract base class - use MediaPlayer2Interface

class MediaPlayer2Base : public RefBase

{

public:

    // callback mechanism for passing messages to MediaPlayer2 object

    typedef void (*NotifyCallback)(const wp<MediaPlayer2Engine> &listener,

            int msg, int ext1, int ext2, const Parcel *obj);



    // AudioSink: abstraction layer for audio output

    class AudioSink : public RefBase {

    public:
@@ -158,7 +158,7 @@ public: 
        virtual status_t    enableAudioDeviceCallback(bool enabled);

    };



                        MediaPlayer2Base() : mCookie(0), mNotify(0) {}

                        MediaPlayer2Base() : mClient(0), mNotify(0) {}

    virtual             ~MediaPlayer2Base() {}

    virtual status_t    initCheck() = 0;

    virtual bool        hardwareOutput() = 0;
@@ -272,22 +272,22 @@ public: 
    };



    void        setNotifyCallback(

            void* cookie, notify_callback_f notifyFunc) {

            const wp<MediaPlayer2Engine> &client, NotifyCallback notifyFunc) {

        Mutex::Autolock autoLock(mNotifyLock);

        mCookie = cookie; mNotify = notifyFunc;

        mClient = client; mNotify = notifyFunc;

    }



    void        sendEvent(int msg, int ext1=0, int ext2=0,

                          const Parcel *obj=NULL) {

        notify_callback_f notifyCB;

        void* cookie;

        NotifyCallback notifyCB;

        wp<MediaPlayer2Engine> client;

        {

            Mutex::Autolock autoLock(mNotifyLock);

            notifyCB = mNotify;

            cookie = mCookie;

            client = mClient;

        }



        if (notifyCB) notifyCB(cookie, msg, ext1, ext2, obj);

        if (notifyCB) notifyCB(client, msg, ext1, ext2, obj);

    }



    virtual status_t dump(int /* fd */, const Vector<String16>& /* args */) const {
@@ -306,8 +306,8 @@ private: 
    friend class MediaPlayer2Manager;



    Mutex                  mNotifyLock;

    void*               mCookie;

    notify_callback_f   mNotify;

    wp<MediaPlayer2Engine> mClient;

    NotifyCallback         mNotify;

};



// Implement this class for media players that use the AudioFlinger software mixer