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

Commit 93cf9130 authored by Linus Nilsson's avatar Linus Nilsson
Browse files

Transcoder: Fixed codec leaks on cancel. 

When transcoding was cancelled, the VideoTrackTranscoder
held strong references to itself through its internal
codec handler work queue.
This commit clears the work queue and blocks new messages
from being stored on cancel.

Test: Unit test and address sanitizer.
Fixes: 168522198
Change-Id: I9c3a4469378a91a492c6872bafc6d494dadf2853
parent c31d2491

media/libmediatranscoding/transcoder/Android.bp

+16 −2
Original line number Diff line number Diff line
@@ -14,8 +14,8 @@ 
 * limitations under the License.

 */



cc_library_shared {

    name: "libmediatranscoder",

cc_defaults {

    name: "mediatranscoder_defaults",



    srcs: [

        "MediaSampleQueue.cpp",
@@ -60,3 +60,17 @@ cc_library_shared { 
        cfi: true,

    },

}



cc_library_shared {

    name: "libmediatranscoder",

    defaults: ["mediatranscoder_defaults"],

}



cc_library_shared {

    name: "libmediatranscoder_asan",

    defaults: ["mediatranscoder_defaults"],



    sanitize: {

        address: true,

    },

}

media/libmediatranscoding/transcoder/MediaTranscoder.cpp

+1 −1
Original line number Diff line number Diff line
@@ -155,7 +155,7 @@ void MediaTranscoder::onTrackFinished(const MediaTrackTranscoder* transcoder) { 
}



void MediaTranscoder::onTrackError(const MediaTrackTranscoder* transcoder, media_status_t status) {

    LOG(DEBUG) << "TrackTranscoder " << transcoder << " returned error " << status;

    LOG(ERROR) << "TrackTranscoder " << transcoder << " returned error " << status;

    sendCallback(status);

}

media/libmediatranscoding/transcoder/VideoTrackTranscoder.cpp

+17 −3
Original line number Diff line number Diff line
@@ -40,7 +40,11 @@ static constexpr float kDefaultKeyFrameIntervalSeconds = 1.0f; 
template <typename T>

void VideoTrackTranscoder::BlockingQueue<T>::push(T const& value, bool front) {

    {

        std::unique_lock<std::mutex> lock(mMutex);

        std::scoped_lock lock(mMutex);

        if (mAborted) {

            return;

        }



        if (front) {

            mQueue.push_front(value);

        } else {
@@ -52,7 +56,7 @@ void VideoTrackTranscoder::BlockingQueue<T>::push(T const& value, bool front) { 


template <typename T>

T VideoTrackTranscoder::BlockingQueue<T>::pop() {

    std::unique_lock<std::mutex> lock(mMutex);

    std::unique_lock lock(mMutex);

    while (mQueue.empty()) {

        mCondition.wait(lock);

    }
@@ -61,6 +65,14 @@ T VideoTrackTranscoder::BlockingQueue<T>::pop() { 
    return value;

}



// Note: Do not call if another thread might waiting in pop.

template <typename T>

void VideoTrackTranscoder::BlockingQueue<T>::abort() {

    std::scoped_lock lock(mMutex);

    mAborted = true;

    mQueue.clear();

}



// The CodecWrapper class is used to let AMediaCodec instances outlive the transcoder object itself

// by giving the codec a weak pointer to the transcoder. Codecs wrapped in this object are kept

// alive by the transcoder and the codec's outstanding buffers. Once the transcoder stops and all
@@ -484,12 +496,14 @@ media_status_t VideoTrackTranscoder::runTranscodeLoop() { 
        message();

    }



    mCodecMessageQueue.abort();

    AMediaCodec_stop(mDecoder);



    // Return error if transcoding was stopped before it finished.

    if (mStopRequested && !mEosFromEncoder && mStatus == AMEDIA_OK) {

        mStatus = AMEDIA_ERROR_UNKNOWN;  // TODO: Define custom error codes?

    }



    AMediaCodec_stop(mDecoder);

    return mStatus;

}

media/libmediatranscoding/transcoder/include/media/VideoTrackTranscoder.h

+2 −0
Original line number Diff line number Diff line
@@ -51,11 +51,13 @@ private: 
    public:

        void push(T const& value, bool front = false);

        T pop();

        void abort();



    private:

        std::mutex mMutex;

        std::condition_variable mCondition;

        std::deque<T> mQueue;

        bool mAborted = false;

    };

    class CodecWrapper;

media/libmediatranscoding/transcoder/tests/Android.bp

+11 −1
Original line number Diff line number Diff line
@@ -17,7 +17,7 @@ cc_defaults { 
        "libbase",

        "libcutils",

        "libmediandk",

        "libmediatranscoder",

        "libmediatranscoder_asan",

        "libutils",

    ],
@@ -26,6 +26,15 @@ cc_defaults { 
        "-Wall",

    ],



    sanitize: {

        misc_undefined: [

            "unsigned-integer-overflow",

            "signed-integer-overflow",

        ],

        cfi: true,

        address: true,

    },



    data: [":test_assets"],

    test_config_template: "AndroidTestTemplate.xml",

    test_suites: ["device-tests", "TranscoderTests"],
@@ -80,4 +89,5 @@ cc_test { 
    name: "MediaTranscoderTests",

    defaults: ["testdefaults"],

    srcs: ["MediaTranscoderTests.cpp"],

    shared_libs: ["libbinder_ndk"],

}