Merge "Change raw pointer mNamedReaders to reference" (5ede4eef) · Commits · e / os / android_frameworks_av

media/libnbaio/NBLog.cpp

+15 −3

Original line number	Diff line number	Diff line
		@@ -1000,6 +1000,8 @@ bool NBLog::Reader::isIMemory(const sp<IMemory>& iMemory) const
		return iMemory != 0 && mIMemory != 0 && iMemory->pointer() == mIMemory->pointer();
		}

		// ---------------------------------------------------------------------------

		void NBLog::appendTimestamp(String8 body, const void data) {
		int64_t ts;
		memcpy(&ts, data, sizeof(ts));
		@@ -1246,6 +1248,8 @@ NBLog::Merger::Merger(const void *shared, size_t size):
		{}

		void NBLog::Merger::addReader(const NBLog::NamedReader &reader) {
		// FIXME This is called by binder thread in MediaLogService::registerWriter
		// but the access to shared variable mNamedReaders is not yet protected by a lock.
		mNamedReaders.push_back(reader);
		}

		@@ -1269,6 +1273,8 @@ bool operator>(const struct MergeItem &i1, const struct MergeItem &i2) {

		// Merge registered readers, sorted by timestamp
		void NBLog::Merger::merge() {
		// FIXME This is called by merge thread
		// but the access to shared variable mNamedReaders is not yet protected by a lock.
		int nLogs = mNamedReaders.size();
		std::vector<std::unique_ptr<NBLog::Reader::Snapshot>> snapshots(nLogs);
		std::vector<NBLog::EntryIterator> offsets(nLogs);
		@@ -1303,19 +1309,25 @@ void NBLog::Merger::merge() {
		}
		}

		const std::vector<NBLog::NamedReader> *NBLog::Merger::getNamedReaders() const {
		return &mNamedReaders;
		const std::vector<NBLog::NamedReader>& NBLog::Merger::getNamedReaders() const {
		// FIXME This is returning a reference to a shared variable that needs a lock
		return mNamedReaders;
		}

		// ---------------------------------------------------------------------------

		NBLog::MergeReader::MergeReader(const void *shared, size_t size, Merger &merger)
		: Reader(shared, size), mNamedReaders(merger.getNamedReaders()) {}

		void NBLog::MergeReader::handleAuthor(const NBLog::AbstractEntry &entry, String8 *body) {
		int author = entry.author();
		const char* name = (*mNamedReaders)[author].name();
		// FIXME Needs a lock
		const char* name = mNamedReaders[author].name();
		body->appendFormat("%s: ", name);
		}

		// ---------------------------------------------------------------------------

		NBLog::MergeThread::MergeThread(NBLog::Merger &merger)
		: mMerger(merger),
		mTimeoutUs(0) {}

media/libnbaio/include/NBLog.h

+11 −2

Original line number	Diff line number	Diff line
		@@ -38,6 +38,8 @@ public:

		typedef uint64_t log_hash_t;

		// FIXME Everything needed for client (writer API and registration) should be isolated
		// from the rest of the implementation.
		class Writer;
		class Reader;

		@@ -320,6 +322,7 @@ public:

		virtual ~Writer();

		// FIXME needs comments, and some should be private
		virtual void log(const char *string);
		virtual void logf(const char *fmt, ...) __attribute__ ((format (printf, 2, 3)));
		virtual void logvf(const char *fmt, va_list ap);
		@@ -504,11 +507,15 @@ public:
		void addReader(const NamedReader &reader);
		// TODO add removeReader
		void merge();
		const std::vector<NamedReader> *getNamedReaders() const;
		// FIXME This is returning a reference to a shared variable that needs a lock
		const std::vector<NamedReader>& getNamedReaders() const;
		private:
		// vector of the readers the merger is supposed to merge from.
		// every reader reads from a writer's buffer
		// FIXME Needs to be protected by a lock
		std::vector<NamedReader> mNamedReaders;

		// TODO Need comments on all of these
		uint8_t *mBuffer;
		Shared * const mShared;
		std::unique_ptr<audio_utils_fifo> mFifo;
		@@ -519,7 +526,9 @@ class MergeReader : public Reader {
		public:
		MergeReader(const void *shared, size_t size, Merger &merger);
		private:
		const std::vector<NamedReader> *mNamedReaders;
		// FIXME Needs to be protected by a lock,
		// because even though our use of it is read-only there may be asynchronous updates
		const std::vector<NamedReader>& mNamedReaders;
		// handle author entry by looking up the author's name and appending it to the body
		// returns number of bytes read from fmtEntry
		void handleAuthor(const AbstractEntry &fmtEntry, String8 *body);

services/medialog/MediaLogService.h

+2 −0

Original line number	Diff line number	Diff line
		@@ -56,6 +56,8 @@ private:
		Mutex mLock;

		Vector<NBLog::NamedReader> mNamedReaders; // protected by mLock

		// FIXME Need comments on all of these, especially about locking
		NBLog::Shared *mMergerShared;
		NBLog::Merger mMerger;
		NBLog::MergeReader mMergeReader;