Fix Android Audio hard

android/app/CMakeLists.txt

@@ -1,6 +1,8 @@
 
 cmake_minimum_required(VERSION 3.2)
 
+set(CMAKE_CXX_STANDARD 14)
+
 add_library(chiaki-jni SHARED
         src/main/cpp/chiaki-jni.c
         src/main/cpp/video-decoder.h
@@ -8,7 +10,8 @@ add_library(chiaki-jni SHARED
         src/main/cpp/audio-decoder.h
         src/main/cpp/audio-decoder.c
         src/main/cpp/audio-output.h
-        src/main/cpp/audio-output.cpp)
+        src/main/cpp/audio-output.cpp
+        src/main/cpp/circular-fifo.hpp)
 target_link_libraries(chiaki-jni chiaki-lib)
 
 find_library(ANDROID_LIB_LOG log)

android/app/src/main/cpp/audio-decoder.c

@@ -66,14 +66,15 @@ static void *android_chiaki_audio_decoder_output_thread_func(void *user)
 		ssize_t codec_buf_index = AMediaCodec_dequeueOutputBuffer(decoder->codec, &info, -1);
 		if(codec_buf_index >= 0)
 		{
-			CHIAKI_LOGD(decoder->log, "Got decoded audio of size %d", (int)info.size);
 			if(decoder->settings_cb)
 			{
 				size_t codec_buf_size;
 				uint8_t *codec_buf = AMediaCodec_getOutputBuffer(decoder->codec, (size_t)codec_buf_index, &codec_buf_size);
-				decoder->frame_cb((int16_t *)codec_buf, codec_buf_size / 2, decoder->cb_user);
+				size_t samples_count = info.size / sizeof(int16_t);
+				//CHIAKI_LOGD(decoder->log, "Got %llu samples => %f ms of audio", (unsigned long long)samples_count, 1000.0f * (float)(samples_count / 2) / (float)decoder->audio_header.rate);
+				decoder->frame_cb((int16_t *)codec_buf, samples_count, decoder->cb_user);
 			}
-			AMediaCodec_releaseOutputBuffer(decoder->codec, (size_t)codec_buf_index, info.size != 0);
+			AMediaCodec_releaseOutputBuffer(decoder->codec, (size_t)codec_buf_index, false);
 			if(info.flags & AMEDIACODEC_BUFFER_FLAG_END_OF_STREAM)
 			{
 				CHIAKI_LOGI(decoder->log, "AMediaCodec for Audio Decoder reported EOS");

android/app/src/main/cpp/audio-output.cpp

@@ -17,12 +17,17 @@
 
 #include "audio-output.h"
 
+#include "circular-buf.hpp"
+
 #include <chiaki/log.h>
 #include <chiaki/thread.h>
 
 #include <oboe/Oboe.h>
 
-#define BUFFER_SIZE_DEFAULT 409600
+#define BUFFER_CHUNK_SIZE 512
+#define BUFFER_CHUNKS_COUNT 128
+
+using AudioBuffer = CircularBuffer<BUFFER_CHUNKS_COUNT, BUFFER_CHUNK_SIZE>;
 
 class AudioOutput;
 
@@ -43,12 +48,7 @@ struct AudioOutput
 	ChiakiLog *log;
 	oboe::ManagedStream stream;
 	AudioOutputCallback stream_callback;
-
-	ChiakiMutex buffer_mutex; // TODO: make lockless
-	uint8_t *buffer;
-	size_t buffer_size; // total size
-	size_t buffer_start;
-	size_t buffer_length; // filled size
+	AudioBuffer buf;
 
 	AudioOutput() : stream_callback(this) {}
 };
@@ -57,23 +57,6 @@ extern "C" void *android_chiaki_audio_output_new(ChiakiLog *log)
 {
 	auto r = new AudioOutput();
 	r->log = log;
-
-	if(chiaki_mutex_init(&r->buffer_mutex, false) != CHIAKI_ERR_SUCCESS)
-	{
-		delete r;
-		return nullptr;
-	}
-	r->buffer_size = BUFFER_SIZE_DEFAULT;
-	r->buffer = (uint8_t *)malloc(r->buffer_size);
-	if(!r->buffer)
-	{
-		chiaki_mutex_fini(&r->buffer_mutex);
-		delete r;
-		return nullptr;
-	}
-	r->buffer_start = 0;
-	r->buffer_length = 0;
-
 	return r;
 }
 
@@ -83,8 +66,6 @@ extern "C" void android_chiaki_audio_output_free(void *audio_output)
 		return;
 	auto ao = reinterpret_cast<AudioOutput *>(audio_output);
 	ao->stream = nullptr;
-	free(ao->buffer);
-	chiaki_mutex_fini(&ao->buffer_mutex);
 	delete ao;
 }
 
@@ -116,29 +97,11 @@ extern "C" void android_chiaki_audio_output_settings(uint32_t channels, uint32_t
 extern "C" void android_chiaki_audio_output_frame(int16_t *buf, size_t samples_count, void *audio_output)
 {
 	auto ao = reinterpret_cast<AudioOutput *>(audio_output);
-	ChiakiMutexLock lock(&ao->buffer_mutex);
 
 	size_t buf_size = samples_count * sizeof(int16_t);
-	if(ao->buffer_size - ao->buffer_length < buf_size)
-	{
+	size_t pushed = ao->buf.Push(reinterpret_cast<uint8_t *>(buf), buf_size);
+	if(pushed < buf_size)
 		CHIAKI_LOGW(ao->log, "Audio Output Buffer Overflow!");
-		buf_size = ao->buffer_size - ao->buffer_length;
-	}
-
-	if(!buf_size)
-		return;
-
-	size_t end = (ao->buffer_start + ao->buffer_length) % ao->buffer_size;
-	ao->buffer_length += buf_size;
-	if(end + buf_size > ao->buffer_size)
-	{
-		size_t part = ao->buffer_size - end;
-		memcpy(ao->buffer + end, buf, part);
-		buf_size -= part;
-		buf = (int16_t *)(((uint8_t *)buf) + part);
-		end = 0;
-	}
-	memcpy(ao->buffer + end, buf, buf_size);
 }
 
 oboe::DataCallbackResult AudioOutputCallback::onAudioReady(oboe::AudioStream *stream, void *audio_data, int32_t num_frames)
@@ -151,33 +114,16 @@ oboe::DataCallbackResult AudioOutputCallback::onAudioReady(oboe::AudioStream *st
 
 	int32_t bytes_per_frame = stream->getBytesPerFrame();
 	size_t buf_size_requested = static_cast<size_t>(bytes_per_frame * num_frames);
+	auto buf = reinterpret_cast<uint8_t *>(audio_data);
 
-	size_t buf_size_delivered = buf_size_requested;
-	uint8_t *buf = (uint8_t *)audio_data;
-
-	{
-		ChiakiMutexLock lock(&audio_output->buffer_mutex);
-		if(audio_output->buffer_length < buf_size_delivered)
-		{
-			CHIAKI_LOGW(audio_output->log, "Audio Output Buffer Underflow!");
-			buf_size_delivered = audio_output->buffer_length;
-		}
-
-		if(audio_output->buffer_start + buf_size_delivered > audio_output->buffer_size)
-		{
-			size_t part = audio_output->buffer_size - audio_output->buffer_start;
-			memcpy(buf, audio_output->buffer + audio_output->buffer_start, part);
-			memcpy(buf + part, audio_output->buffer, buf_size_delivered - part);
-		}
-		else
-			memcpy(buf, audio_output->buffer + audio_output->buffer_start, buf_size_delivered);
-
-		audio_output->buffer_start = (audio_output->buffer_start + buf_size_delivered) % audio_output->buffer_size;
-		audio_output->buffer_length -= buf_size_delivered;
-	}
+	size_t buf_size_delivered = audio_output->buf.Pop(buf, buf_size_requested);
+	//CHIAKI_LOGW(audio_output->log, "Delivered %llu", (unsigned long long)buf_size_delivered);
 
 	if(buf_size_delivered < buf_size_requested)
+	{
+		CHIAKI_LOGW(audio_output->log, "Audio Output Buffer Underflow!");
 		memset(buf + buf_size_delivered, 0, buf_size_requested - buf_size_delivered);
+	}
 
 	return oboe::DataCallbackResult::Continue;
 }

android/app/src/main/cpp/circular-buf.hpp

@@ -0,0 +1,153 @@
+/*
+ * This file is part of Chiaki.
+ *
+ * Chiaki is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * Chiaki is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with Chiaki.  If not, see <https://www.gnu.org/licenses/>.
+ */
+
+#ifndef CHIAKI_JNI_CIRCULARBUF_HPP
+#define CHIAKI_JNI_CIRCULARBUF_HPP
+
+#include "circular-fifo.hpp"
+
+#include <string.h>
+#include <assert.h>
+
+#include <android/log.h>
+
+template<size_t ChunksCount, size_t ChunkSize>
+class CircularBuffer
+{
+	static_assert(ChunksCount > 0);
+	static_assert(ChunkSize > 0);
+
+	private:
+		using Queue = memory_relaxed_aquire_release::CircularFifo<uint8_t *, ChunksCount>;
+		Queue full_queue;
+		Queue free_queue;
+		uint8_t *buffer;
+
+		uint8_t *push_chunk;
+		size_t push_chunk_size; // written bytes from the start of the chunk
+
+		uint8_t *pop_chunk;
+		size_t pop_chunk_size; // remaining bytes until the end of the chunk
+
+		void FlushChunks()
+		{
+			for(size_t i=1; i<ChunksCount; i++)
+				free_queue.push(buffer + i * ChunkSize);
+
+			push_chunk = buffer;
+			push_chunk_size = 0;
+
+			pop_chunk = nullptr;
+			pop_chunk_size = 0;
+		}
+
+	public:
+		CircularBuffer() : buffer(new uint8_t[ChunksCount * ChunkSize])
+		{
+			FlushChunks();
+		}
+
+		~CircularBuffer()
+		{
+			delete buffer;
+		}
+
+		/**
+		 * Reset the entire Buffer.
+		 * WARNING: Not thread-safe at all! Call only when no producer and consumer is running.
+		 */
+		void Flush()
+		{
+			full_queue = Queue();
+			free_queue = Queue();
+			FlushChunks();
+		}
+
+		/**
+		 * @return bytes that were pushed
+		 */
+		size_t Push(uint8_t *buf, size_t buf_size)
+		{
+			size_t pushed = 0;
+			while(pushed < buf_size)
+			{
+				if(!push_chunk)
+				{
+					if(!free_queue.pop(push_chunk))
+						push_chunk = nullptr;
+					if(!push_chunk)
+						return pushed;
+					push_chunk_size = 0;
+				}
+
+				size_t to_push = buf_size - pushed;
+				size_t remaining_space = ChunkSize - push_chunk_size;
+				if(to_push > remaining_space)
+					to_push = remaining_space;
+				memcpy(push_chunk + push_chunk_size, buf + pushed, to_push);
+				pushed += to_push;
+				push_chunk_size += to_push;
+
+				if(push_chunk_size == ChunkSize)
+				{
+					bool success = full_queue.push(push_chunk);
+					assert(success); // We should have made the queues big enough
+					push_chunk = nullptr;
+				}
+			}
+
+			return pushed;
+		}
+
+		/**
+		 * @return bytes that were popped
+		 */
+		size_t Pop(uint8_t *buf, size_t buf_size)
+		{
+			size_t popped = 0;
+			while(popped < buf_size)
+			{
+				if(!pop_chunk)
+				{
+					if(!full_queue.pop(pop_chunk))
+						pop_chunk = nullptr;
+					if(!pop_chunk)
+						return popped;
+					pop_chunk_size = ChunkSize;
+				}
+
+				size_t to_pop = buf_size - popped;
+				if(to_pop > pop_chunk_size)
+					to_pop = pop_chunk_size;
+				memcpy(buf + popped, pop_chunk + (ChunkSize - pop_chunk_size), to_pop);
+				popped += to_pop;
+				pop_chunk_size -= to_pop;
+
+				if(pop_chunk_size == 0)
+				{
+					bool success = free_queue.push(pop_chunk);
+					assert(success);// We should have made the queues big enough
+					pop_chunk = nullptr;
+				}
+			}
+
+			return popped;
+		}
+};
+
+
+#endif //CHIAKI_JNI_CIRCULARBUF_HPP

android/app/src/main/cpp/circular-fifo.hpp

@@ -0,0 +1,108 @@
+/* 
+* Not any company's property but Public-Domain
+* Do with source-code as you will. No requirement to keep this
+* header if need to use it/change it/ or do whatever with it
+*
+* Note that there is No guarantee that this code will work 
+* and I take no responsibility for this code and any problems you
+* might get if using it. 
+*
+* Code & platform dependent issues with it was originally 
+* published at http://www.kjellkod.cc/threadsafecircularqueue
+* 2012-16-19  @author Kjell Hedström, hedstrom@kjellkod.cc */
+
+// should be mentioned the thinking of what goes where
+// it is a "controversy" whether what is tail and what is head
+// http://en.wikipedia.org/wiki/FIFO#Head_or_tail_first
+
+#ifndef CIRCULARFIFO_AQUIRE_RELEASE_H_
+#define CIRCULARFIFO_AQUIRE_RELEASE_H_
+
+#include <atomic>
+#include <cstddef>
+namespace memory_relaxed_aquire_release {
+template<typename Element, size_t Size> 
+class CircularFifo{
+public:
+  enum { Capacity = Size+1 };
+
+  CircularFifo() : _tail(0), _head(0){}   
+  virtual ~CircularFifo() {}
+
+  bool push(const Element& item); // pushByMOve?
+  bool pop(Element& item);
+
+  bool wasEmpty() const;
+  bool wasFull() const;
+  bool isLockFree() const;
+
+private:
+  size_t increment(size_t idx) const; 
+
+  std::atomic <size_t>  _tail;  // tail(input) index
+  Element    _array[Capacity];
+  std::atomic<size_t>   _head; // head(output) index
+};
+
+template<typename Element, size_t Size>
+bool CircularFifo<Element, Size>::push(const Element& item)
+{	
+  const auto current_tail = _tail.load(std::memory_order_relaxed); 
+  const auto next_tail = increment(current_tail); 
+  if(next_tail != _head.load(std::memory_order_acquire))                           
+  {	
+    _array[current_tail] = item;
+    _tail.store(next_tail, std::memory_order_release); 
+    return true;
+  }
+  
+  return false; // full queue
+
+}
+
+
+// Pop by Consumer can only update the head (load with relaxed, store with release)
+//     the tail must be accessed with at least aquire
+template<typename Element, size_t Size>
+bool CircularFifo<Element, Size>::pop(Element& item)
+{
+  const auto current_head = _head.load(std::memory_order_relaxed);  
+  if(current_head == _tail.load(std::memory_order_acquire)) 
+    return false; // empty queue
+
+  item = _array[current_head]; 
+  _head.store(increment(current_head), std::memory_order_release); 
+  return true;
+}
+
+template<typename Element, size_t Size>
+bool CircularFifo<Element, Size>::wasEmpty() const
+{
+  // snapshot with acceptance of that this comparison operation is not atomic
+  return (_head.load() == _tail.load()); 
+}
+
+
+// snapshot with acceptance that this comparison is not atomic
+template<typename Element, size_t Size>
+bool CircularFifo<Element, Size>::wasFull() const
+{
+  const auto next_tail = increment(_tail.load()); // aquire, we dont know who call
+  return (next_tail == _head.load());
+}
+
+
+template<typename Element, size_t Size>
+bool CircularFifo<Element, Size>::isLockFree() const
+{
+  return (_tail.is_lock_free() && _head.is_lock_free());
+}
+
+template<typename Element, size_t Size>
+size_t CircularFifo<Element, Size>::increment(size_t idx) const
+{
+  return (idx + 1) % Capacity;
+}
+
+} // memory_relaxed_aquire_release
+#endif /* CIRCULARFIFO_AQUIRE_RELEASE_H_ */