Merge changes I9fb4e87e,Id018c33c,I5b6bdb3b
* changes:
fix tuner filter callback scheduler
fix filter event creation
fix mFilterEvents locking
diff --git a/tv/tuner/aidl/default/Filter.cpp b/tv/tuner/aidl/default/Filter.cpp
index 5f61c8d..54037be 100644
--- a/tv/tuner/aidl/default/Filter.cpp
+++ b/tv/tuner/aidl/default/Filter.cpp
@@ -35,7 +35,11 @@
#define WAIT_TIMEOUT 3000000000
FilterCallbackScheduler::FilterCallbackScheduler(const std::shared_ptr<IFilterCallback>& cb)
- : mCallback(cb), mDataLength(0), mTimeDelayInMs(0), mDataSizeDelayInBytes(0) {
+ : mCallback(cb),
+ mIsConditionMet(false),
+ mDataLength(0),
+ mTimeDelayInMs(0),
+ mDataSizeDelayInBytes(0) {
start();
}
@@ -44,12 +48,14 @@
}
void FilterCallbackScheduler::onFilterEvent(DemuxFilterEvent&& event) {
- std::lock_guard<std::mutex> lock(mLock);
+ std::unique_lock<std::mutex> lock(mLock);
mCallbackBuffer.push_back(std::move(event));
mDataLength += getDemuxFilterEventDataLength(event);
- if (mDataLength >= mDataSizeDelayInBytes) {
- // size limit has been reached, send out events
+ if (isDataSizeDelayConditionMetLocked()) {
+ mIsConditionMet = true;
+ // unlock, so thread is not immediately blocked when it is notified.
+ lock.unlock();
mCv.notify_all();
}
}
@@ -67,21 +73,22 @@
}
void FilterCallbackScheduler::setTimeDelayHint(int timeDelay) {
- // updating the setTimeDelay does not go into effect until the condition
- // variable times out or is notified.
- // One possibility is to notify the condition variable right away when the
- // time delay changes, but I don't see the benefit over waiting for the next
- // timeout / push, since -- in any case -- this will not change very often.
+ std::unique_lock<std::mutex> lock(mLock);
mTimeDelayInMs = timeDelay;
+ // always notify condition variable to update timeout
+ mIsConditionMet = true;
+ lock.unlock();
+ mCv.notify_all();
}
void FilterCallbackScheduler::setDataSizeDelayHint(int dataSizeDelay) {
- // similar to updating the time delay hint, when mDataSizeDelayInBytes
- // changes, this will not go into immediate effect, but will wait until the
- // next filterEvent.
- // We could technically check the current data length and notify the
- // condition variable if we wanted to, but again, this may be overkill.
+ std::unique_lock<std::mutex> lock(mLock);
mDataSizeDelayInBytes = dataSizeDelay;
+ if (isDataSizeDelayConditionMetLocked()) {
+ mIsConditionMet = true;
+ lock.unlock();
+ mCv.notify_all();
+ }
}
bool FilterCallbackScheduler::hasCallbackRegistered() const {
@@ -96,6 +103,10 @@
void FilterCallbackScheduler::stop() {
mIsRunning = false;
if (mCallbackThread.joinable()) {
+ {
+ std::lock_guard<std::mutex> lock(mLock);
+ mIsConditionMet = true;
+ }
mCv.notify_all();
mCallbackThread.join();
}
@@ -109,17 +120,15 @@
void FilterCallbackScheduler::threadLoopOnce() {
std::unique_lock<std::mutex> lock(mLock);
- // mTimeDelayInMs is an atomic value, so it should be copied into a local
- // variable before use (to make sure both the if statement and wait_for use
- // the same value).
- int timeDelayInMs = mTimeDelayInMs;
- if (timeDelayInMs > 0) {
- mCv.wait_for(lock, std::chrono::milliseconds(timeDelayInMs));
+ if (mTimeDelayInMs > 0) {
+ // Note: predicate protects from lost and spurious wakeups
+ mCv.wait_for(lock, std::chrono::milliseconds(mTimeDelayInMs),
+ [this] { return mIsConditionMet; });
} else {
- // no reason to timeout, just wait until main thread determines it's
- // okay to send data.
- mCv.wait(lock);
+ // Note: predicate protects from lost and spurious wakeups
+ mCv.wait(lock, [this] { return mIsConditionMet; });
}
+ mIsConditionMet = false;
// condition_variable wait locks mutex on timeout / notify
// Note: if stop() has been called in the meantime, do not send more filter
@@ -131,7 +140,22 @@
mCallbackBuffer.clear();
mDataLength = 0;
}
- lock.unlock();
+}
+
+// mLock needs to be held to call this function
+bool FilterCallbackScheduler::isDataSizeDelayConditionMetLocked() {
+ if (mDataSizeDelayInBytes == 0) {
+ // Data size delay is disabled.
+ if (mTimeDelayInMs == 0) {
+ // Events should only be sent immediately if time delay is disabled
+ // as well.
+ return true;
+ }
+ return false;
+ }
+
+ // Data size delay is enabled.
+ return mDataLength >= mDataSizeDelayInBytes;
}
int FilterCallbackScheduler::getDemuxFilterEventDataLength(const DemuxFilterEvent& event) {
@@ -542,7 +566,9 @@
// For the first time of filter output, implementation needs to send the filter
// Event Callback without waiting for the DATA_CONSUMED to init the process.
while (mFilterThreadRunning) {
+ std::unique_lock<std::mutex> lock(mFilterEventsLock);
if (mFilterEvents.size() == 0) {
+ lock.unlock();
if (DEBUG_FILTER) {
ALOGD("[Filter] wait for filter data output.");
}
@@ -559,6 +585,7 @@
mConfigured = false;
}
+ // lock is still being held
for (auto&& event : mFilterEvents) {
mCallbackScheduler.onFilterEvent(std::move(event));
}
@@ -741,7 +768,6 @@
}
::ndk::ScopedAStatus Filter::startPesFilterHandler() {
- std::lock_guard<std::mutex> lock(mFilterEventsLock);
if (mFilterOutput.empty()) {
return ::ndk::ScopedAStatus::ok();
}
@@ -796,7 +822,11 @@
ALOGD("[Filter] assembled pes data length %d", pesEvent.dataLength);
}
- mFilterEvents.push_back(DemuxFilterEvent::make<DemuxFilterEvent::Tag::pes>(pesEvent));
+ {
+ std::lock_guard<std::mutex> lock(mFilterEventsLock);
+ mFilterEvents.push_back(DemuxFilterEvent::make<DemuxFilterEvent::Tag::pes>(pesEvent));
+ }
+
mPesOutput.clear();
}
@@ -811,7 +841,6 @@
}
::ndk::ScopedAStatus Filter::startMediaFilterHandler() {
- std::lock_guard<std::mutex> lock(mFilterEventsLock);
if (mFilterOutput.empty()) {
return ::ndk::ScopedAStatus::ok();
}
@@ -900,7 +929,12 @@
.firstMbInSlice = 0, // random address
};
- mFilterEvents.push_back(DemuxFilterEvent::make<DemuxFilterEvent::Tag::tsRecord>(recordEvent));
+ {
+ std::lock_guard<std::mutex> lock(mFilterEventsLock);
+ mFilterEvents.push_back(
+ DemuxFilterEvent::make<DemuxFilterEvent::Tag::tsRecord>(recordEvent));
+ }
+
mRecordFilterOutput.clear();
return ::ndk::ScopedAStatus::ok();
}
@@ -918,7 +952,6 @@
bool Filter::writeSectionsAndCreateEvent(vector<int8_t>& data) {
// TODO check how many sections has been read
ALOGD("[Filter] section handler");
- std::lock_guard<std::mutex> lock(mFilterEventsLock);
if (!writeDataToFilterMQ(data)) {
return false;
}
@@ -930,7 +963,12 @@
.sectionNum = 1,
.dataLength = static_cast<int32_t>(data.size()),
};
- mFilterEvents.push_back(DemuxFilterEvent::make<DemuxFilterEvent::Tag::section>(secEvent));
+
+ {
+ std::lock_guard<std::mutex> lock(mFilterEventsLock);
+ mFilterEvents.push_back(DemuxFilterEvent::make<DemuxFilterEvent::Tag::section>(secEvent));
+ }
+
return true;
}
@@ -1028,7 +1066,11 @@
mediaEvent.pts = mPts;
mPts = 0;
}
- mFilterEvents.push_back(std::move(event));
+
+ {
+ std::lock_guard<std::mutex> lock(mFilterEventsLock);
+ mFilterEvents.push_back(std::move(event));
+ }
// Clear and log
native_handle_close(nativeHandle);
@@ -1068,7 +1110,11 @@
mediaEvent.pts = mPts;
mPts = 0;
}
- mFilterEvents.push_back(std::move(event));
+
+ {
+ std::lock_guard<std::mutex> lock(mFilterEventsLock);
+ mFilterEvents.push_back(std::move(event));
+ }
mSharedAvMemOffset += output.size();
@@ -1092,8 +1138,6 @@
void Filter::createMediaEvent(vector<DemuxFilterEvent>& events) {
AudioExtraMetaData audio;
- events.resize(1);
-
audio.adFade = 1;
audio.adPan = 2;
audio.versionTextTag = 3;
@@ -1101,17 +1145,15 @@
audio.adGainFront = 5;
audio.adGainSurround = 6;
- events[0] = DemuxFilterEvent::make<DemuxFilterEvent::Tag::media>();
- events[0].get<DemuxFilterEvent::Tag::media>().streamId = 1;
- events[0].get<DemuxFilterEvent::Tag::media>().isPtsPresent = true;
- events[0].get<DemuxFilterEvent::Tag::media>().dataLength = 3;
- events[0].get<DemuxFilterEvent::Tag::media>().offset = 4;
- events[0].get<DemuxFilterEvent::Tag::media>().isSecureMemory = true;
- events[0].get<DemuxFilterEvent::Tag::media>().mpuSequenceNumber = 6;
- events[0].get<DemuxFilterEvent::Tag::media>().isPesPrivateData = true;
- events[0]
- .get<DemuxFilterEvent::Tag::media>()
- .extraMetaData.set<DemuxFilterMediaEventExtraMetaData::Tag::audio>(audio);
+ DemuxFilterMediaEvent mediaEvent;
+ mediaEvent.streamId = 1;
+ mediaEvent.isPtsPresent = true;
+ mediaEvent.dataLength = 3;
+ mediaEvent.offset = 4;
+ mediaEvent.isSecureMemory = true;
+ mediaEvent.mpuSequenceNumber = 6;
+ mediaEvent.isPesPrivateData = true;
+ mediaEvent.extraMetaData.set<DemuxFilterMediaEventExtraMetaData::Tag::audio>(audio);
int av_fd = createAvIonFd(BUFFER_SIZE_16M);
if (av_fd == -1) {
@@ -1129,16 +1171,16 @@
uint64_t dataId = mLastUsedDataId++ /*createdUID*/;
mDataId2Avfd[dataId] = dup(av_fd);
- events[0].get<DemuxFilterEvent::Tag::media>().avDataId = static_cast<int64_t>(dataId);
- events[0].get<DemuxFilterEvent::Tag::media>().avMemory = ::android::dupToAidl(nativeHandle);
+ mediaEvent.avDataId = static_cast<int64_t>(dataId);
+ mediaEvent.avMemory = ::android::dupToAidl(nativeHandle);
+
+ events.push_back(DemuxFilterEvent::make<DemuxFilterEvent::Tag::media>(std::move(mediaEvent)));
native_handle_close(nativeHandle);
native_handle_delete(nativeHandle);
}
void Filter::createTsRecordEvent(vector<DemuxFilterEvent>& events) {
- events.resize(2);
-
DemuxPid pid;
DemuxFilterScIndexMask mask;
DemuxFilterTsRecordEvent tsRecord1;
@@ -1153,13 +1195,11 @@
tsRecord2.pts = 1;
tsRecord2.firstMbInSlice = 2; // random address
- events[0].set<DemuxFilterEvent::Tag::tsRecord>(tsRecord1);
- events[1].set<DemuxFilterEvent::Tag::tsRecord>(tsRecord2);
+ events.push_back(DemuxFilterEvent::make<DemuxFilterEvent::Tag::tsRecord>(std::move(tsRecord1)));
+ events.push_back(DemuxFilterEvent::make<DemuxFilterEvent::Tag::tsRecord>(std::move(tsRecord2)));
}
void Filter::createMmtpRecordEvent(vector<DemuxFilterEvent>& events) {
- events.resize(2);
-
DemuxFilterMmtpRecordEvent mmtpRecord1;
mmtpRecord1.scHevcIndexMask = 1;
mmtpRecord1.byteNumber = 2;
@@ -1170,36 +1210,32 @@
mmtpRecord2.firstMbInSlice = 3;
mmtpRecord2.tsIndexMask = 4;
- events[0].set<DemuxFilterEvent::Tag::mmtpRecord>(mmtpRecord1);
- events[1].set<DemuxFilterEvent::Tag::mmtpRecord>(mmtpRecord2);
+ events.push_back(
+ DemuxFilterEvent::make<DemuxFilterEvent::Tag::mmtpRecord>(std::move(mmtpRecord1)));
+ events.push_back(
+ DemuxFilterEvent::make<DemuxFilterEvent::Tag::mmtpRecord>(std::move(mmtpRecord2)));
}
void Filter::createSectionEvent(vector<DemuxFilterEvent>& events) {
- events.resize(1);
-
DemuxFilterSectionEvent section;
section.tableId = 1;
section.version = 2;
section.sectionNum = 3;
section.dataLength = 0;
- events[0].set<DemuxFilterEvent::Tag::section>(section);
+ events.push_back(DemuxFilterEvent::make<DemuxFilterEvent::Tag::section>(std::move(section)));
}
void Filter::createPesEvent(vector<DemuxFilterEvent>& events) {
- events.resize(1);
-
DemuxFilterPesEvent pes;
pes.streamId = 1;
pes.dataLength = 1;
pes.mpuSequenceNumber = 2;
- events[0].set<DemuxFilterEvent::Tag::pes>(pes);
+ events.push_back(DemuxFilterEvent::make<DemuxFilterEvent::Tag::pes>(std::move(pes)));
}
void Filter::createDownloadEvent(vector<DemuxFilterEvent>& events) {
- events.resize(1);
-
DemuxFilterDownloadEvent download;
download.itemId = 1;
download.mpuSequenceNumber = 2;
@@ -1207,41 +1243,36 @@
download.lastItemFragmentIndex = 4;
download.dataLength = 0;
- events[0].set<DemuxFilterEvent::Tag::download>(download);
+ events.push_back(DemuxFilterEvent::make<DemuxFilterEvent::Tag::download>(std::move(download)));
}
void Filter::createIpPayloadEvent(vector<DemuxFilterEvent>& events) {
- events.resize(1);
-
DemuxFilterIpPayloadEvent ipPayload;
ipPayload.dataLength = 0;
- events[0].set<DemuxFilterEvent::Tag::ipPayload>(ipPayload);
+ events.push_back(
+ DemuxFilterEvent::make<DemuxFilterEvent::Tag::ipPayload>(std::move(ipPayload)));
}
void Filter::createTemiEvent(vector<DemuxFilterEvent>& events) {
- events.resize(1);
-
DemuxFilterTemiEvent temi;
temi.pts = 1;
temi.descrTag = 2;
temi.descrData = {3};
- events[0].set<DemuxFilterEvent::Tag::temi>(temi);
+ events.push_back(DemuxFilterEvent::make<DemuxFilterEvent::Tag::temi>(std::move(temi)));
}
void Filter::createMonitorEvent(vector<DemuxFilterEvent>& events) {
- events.resize(1);
-
DemuxFilterMonitorEvent monitor;
monitor.set<DemuxFilterMonitorEvent::Tag::scramblingStatus>(ScramblingStatus::SCRAMBLED);
- events[0].set<DemuxFilterEvent::Tag::monitorEvent>(monitor);
+
+ events.push_back(
+ DemuxFilterEvent::make<DemuxFilterEvent::Tag::monitorEvent>(std::move(monitor)));
}
void Filter::createRestartEvent(vector<DemuxFilterEvent>& events) {
- events.resize(1);
-
- events[0].set<DemuxFilterEvent::Tag::startId>(1);
+ events.push_back(DemuxFilterEvent::make<DemuxFilterEvent::Tag::startId>(1));
}
} // namespace tuner
diff --git a/tv/tuner/aidl/default/Filter.h b/tv/tuner/aidl/default/Filter.h
index 7298561..8388c98 100644
--- a/tv/tuner/aidl/default/Filter.h
+++ b/tv/tuner/aidl/default/Filter.h
@@ -77,6 +77,9 @@
void threadLoop();
void threadLoopOnce();
+ // function needs to be called while holding mLock
+ bool isDataSizeDelayConditionMetLocked();
+
static int getDemuxFilterEventDataLength(const DemuxFilterEvent& event);
private:
@@ -84,16 +87,15 @@
std::thread mCallbackThread;
std::atomic<bool> mIsRunning;
- // mLock protects mCallbackBuffer, mCv, and mDataLength
+ // mLock protects mCallbackBuffer, mIsConditionMet, mCv, mDataLength,
+ // mTimeDelayInMs, and mDataSizeDelayInBytes
std::mutex mLock;
std::vector<DemuxFilterEvent> mCallbackBuffer;
+ bool mIsConditionMet;
std::condition_variable mCv;
int mDataLength;
-
- // both of these need to be atomic (not just mTimeDelayInMs) as this class
- // needs to be threadsafe.
- std::atomic<int> mTimeDelayInMs;
- std::atomic<int> mDataSizeDelayInBytes;
+ int mTimeDelayInMs;
+ int mDataSizeDelayInBytes;
};
class Filter : public BnFilter {