Atrac: Forward-port comments from the other branches

Core/HLE/AtracCtx.cpp

@@ -188,6 +188,8 @@ void AtracBase::UpdateContextFromPSPMem() {
 
 	// Read in any changes from the game to the context.
 	// TODO: Might be better to just always track in RAM.
+	// TODO: It's possible that there are more changes we should read. Who knows,
+	// problem games like FlatOut might poke stuff into the context?
 	bufferState_ = context_->info.state;
 	// This value is actually abused by games to store the SAS voice number.
 	loopNum_ = context_->info.loopNum;
@@ -765,7 +767,6 @@ int AtracBase::GetSecondBufferInfo(u32 *fileOffset, u32 *desiredSize) {
 
 void Atrac::GetStreamDataInfo(u32 *writePtr, u32 *writableBytes, u32 *readOffset) {
 	u32 calculatedReadOffset;
-	// TODO: Feels like this should already have been computed?
 	CalculateStreamInfo(&calculatedReadOffset);
 
 	*writePtr = first_.addr + first_.offset;
@@ -776,7 +777,7 @@ void Atrac::GetStreamDataInfo(u32 *writePtr, u32 *writableBytes, u32 *readOffset
 void Atrac::UpdateBufferState() {
 	if (bufferMaxSize_ >= track_.fileSize) {
 		if (first_.size < track_.fileSize) {
-			// The buffer is big enough, but we don't have all the data yet.
+			// The buffer is big enough in RAM, but we don't have all the data yet.
 			bufferState_ = ATRAC_STATUS_HALFWAY_BUFFER;
 		} else {
 			bufferState_ = ATRAC_STATUS_ALL_DATA_LOADED;
@@ -793,6 +794,8 @@ void Atrac::UpdateBufferState() {
 	}
 }
 
+// The game calls this after actually writing data to the buffer, as specified by the return values from GetStreamDataInfo.
+// So, we should not have to call CalculateStreamInfo again here (although, might not be a bad idea for safety).
 int Atrac::AddStreamData(u32 bytesToAdd) {
 	u32 readOffset;
 	CalculateStreamInfo(&readOffset);
@@ -963,7 +966,6 @@ u32 Atrac::DecodeData(u8 *outbuf, u32 outbufPtr, u32 *SamplesNum, u32 *finish, i
 		return SCE_ERROR_ATRAC_ALL_DATA_DECODED;
 	}
 
-	// TODO: This isn't at all right, but at least it makes the music "last" some time.
 	u32 numSamples = 0;
 
 	// It seems like the PSP aligns the sample position to 0x800...?
@@ -1081,6 +1083,7 @@ u32 Atrac::DecodeData(u8 *outbuf, u32 outbufPtr, u32 *SamplesNum, u32 *finish, i
 void AtracBase::SetLoopNum(int loopNum) {
 	// Spammed in MHU
 	loopNum_ = loopNum;
+	// Logic here looks wacky?
 	if (loopNum != 0 && track_.loopinfo.size() == 0) {
 		// Just loop the whole audio
 		// This is a rare modification of track_ after the fact.

Core/HLE/AtracCtx.h

@@ -51,6 +51,7 @@ const int PSP_ATRAC_LOOP_STREAM_DATA_IS_ON_MEMORY = -3;
 
 // This is not a PSP-native struct.
 // But, it's stored in its entirety in savestates, which makes it awkward to change it.
+// This is used for both first_ and second_, but the latter doesn't use all the fields.
 struct InputBuffer {
 	// Address of the buffer.
 	u32 addr;
@@ -79,6 +80,7 @@ struct AtracLoopInfo {
 
 class AudioDecoder;
 
+// This is (mostly) constant info, once a track has been loaded.
 struct Track {
 	// This both does and doesn't belong in Track - it's fixed for an Atrac instance. Oh well.
 	u32 codecType = 0;
@@ -86,10 +88,11 @@ struct Track {
 	// Size of the full track being streamed or played. Can be a lot larger than the in-memory buffer in the streaming modes.
 	u32 fileSize = 0;
 
-	// Not really used for much except queries, this keeps track of the bitrate of the track.
+	// Not really used for much except queries, this keeps track of the bitrate of the track (kbps).
 	u32 bitrate = 64;
 
-	// Signifies whether to use a more efficient coding mode with less stereo separation. For our purposes, just metadata.
+	// Signifies whether to use a more efficient coding mode with less stereo separation. For our purposes, just metadata,
+	// not actually used in decoding.
 	int jointStereo = 0;
 
 	// Number of audio channels in the track.
@@ -108,10 +111,11 @@ struct Track {
 	// sometimes not.
 	int firstSampleOffset = 0;
 
-	// Last sample number. Though, we made it so in Analyze, it's exclusive in the file.
+	// Last sample number. Inclusive. Though, we made it so that in Analyze, it's exclusive in the file.
 	// Does not take firstSampleOffset into account.
 	int endSample = 0;
 
+	// NOTE: The below CAN be written.
 	// Loop configuration. The PSP only supports one loop but we store them all.
 	std::vector<AtracLoopInfo> loopinfo;
 	// The actual used loop offsets. These appear to be raw offsets, not taking FirstSampleOffset2() into account.
@@ -195,6 +199,7 @@ public:
 		outputChannels_ = channels;
 	}
 
+	// Refactor?
 	int atracID_ = -1;
 
 	PSPPointer<SceAtracContext> context_{};
@@ -280,7 +285,9 @@ public:
 		return second_.size;
 	}
 
+	// Ask where in memory new data should be written.
 	void GetStreamDataInfo(u32 *writePtr, u32 *writableBytes, u32 *readOffset) override;
+	// Notify the player that the user has written some new data.
 	int AddStreamData(u32 bytesToAdd) override;
 	u32 AddStreamDataSas(u32 bufPtr, u32 bytesToAdd) override;
 	u32 ResetPlayPosition(int sample, int bytesWrittenFirstBuf, int bytesWrittenSecondBuf) override;
@@ -288,6 +295,7 @@ public:
 	int SetData(u32 buffer, u32 readSize, u32 bufferSize, int outputChannels, int successCode) override;
 	u32 SetSecondBuffer(u32 secondBuffer, u32 secondBufferSize) override;
 	u32 DecodeData(u8 *outbuf, u32 outbufPtr, u32 *SamplesNum, u32 *finish, int *remains) override;
+	// Returns how many samples the next DecodeData will write.
 	u32 GetNextSamples() override;
 	void InitLowLevel(u32 paramsAddr, bool jointStereo) override;
 

Core/HLE/sceAtrac.cpp

@@ -40,14 +40,14 @@
 
 // Notes about sceAtrac buffer management
 //
-// sceAtrac decodes from a buffer the game fills, where this buffer is one of:
+// sceAtrac decodes from a buffer the game fills, where this buffer has a status, one of:
-//   * Not yet initialized (state NO DATA = 1)
+//   * Not yet initialized (state NO_DATA = 1)
-//   * The entire size of the audio data, and filled with audio data (state ALL DATA LOADED = 2)
+//   * The entire size of the audio data, and filled with audio data (state ALL_DATA_LOADED = 2)
-//   * The entire size, but only partially filled so far (state HALFWAY BUFFER = 3)
+//   * The entire size, but only partially filled so far (state HALFWAY_BUFFER = 3)
-//   * Smaller than the audio, sliding without any loop (state STREAMED WITHOUT LOOP = 4)
+//   * Smaller than the audio, sliding without any loop (state STREAMED_WITHOUT_LOOP = 4)
-//   * Smaller than the audio, sliding with a loop at the end (state STREAMED WITH LOOP AT END = 5)
+//   * Smaller than the audio, sliding with a loop at the end (state STREAMED_WITH_LOOP_AT_END = 5)
-//   * Smaller with a second buffer to help with a loop in the middle (state STREAMED WITH SECOND BUF = 6)
+//   * Smaller with a second buffer to help with a loop in the middle (state STREAMED_WITH_SECOND_BUF = 6)
-//   * Not managed, decoding using "low level" manual looping etc. (LOW LEVEL = 8)
+//   * Not managed, decoding using "low level" manual looping etc. (LOW_LEVEL = 8)
 //   * Not managed, reserved externally - possibly by sceSas - through low level (RESERVED = 16)
 //
 // This buffer is generally filled by sceAtracAddStreamData, and where to fill it is given by
@@ -59,6 +59,7 @@
 // the buffer it will call sceAtracSetSecondBuffer.
 // The second buffer will just contain the data for the end of loop.  The "first" buffer may manage
 // only the looped portion, or some of the part after the loop (depending on second buf size.)
+// TODO: What games use this?
 //
 // Most files will be in RIFF format.  It's also possible to load in an OMA/AA3 format file, but
 // ultimately this will share the same buffer - it's just offset a bit more.