Feat/container streaming#2
Conversation
There was a problem hiding this comment.
Code Review
This pull request migrates the AnimeStudio CLI to .NET 10, removes the desktop GUI and patcher, and introduces a low-memory streaming container storage mechanism to optimize resource usage. Feedback on the changes suggests guarding managed field cleanup in FbxExporterContext.Dispose to ensure finalizer safety, optimizing directory measurement in RunSummary by using DirectoryInfo.EnumerateFiles to avoid redundant metadata queries, and improving stream reading robustness in SevenZipHelper using Stream.ReadExactly and BinaryPrimitives.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if (Interlocked.Exchange(ref _disposeState, 1) != 0) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| _frameToNode.Clear(); | ||
| _createdMaterials.Clear(); | ||
| _createdTextures.Clear(); | ||
| _frameToNode?.Clear(); | ||
| _createdMaterials?.Clear(); | ||
| _createdTextures?.Clear(); | ||
|
|
||
| AsFbxDisposeContext(ref _pContext); | ||
| if (_pContext != IntPtr.Zero) | ||
| { | ||
| AsFbxDisposeContext(ref _pContext); | ||
| } |
There was a problem hiding this comment.
In the standard .NET Dispose pattern, accessing managed fields (such as _frameToNode, _createdMaterials, and _createdTextures) during finalization (when disposing is false) is unsafe because those managed objects may have already been finalized or are in an unstable state. These operations should be guarded with if (disposing).
if (Interlocked.Exchange(ref _disposeState, 1) != 0)
{
return;
}
if (disposing)
{
_frameToNode?.Clear();
_createdMaterials?.Clear();
_createdTextures?.Clear();
}
if (_pContext != IntPtr.Zero)
{
AsFbxDisposeContext(ref _pContext);
}| internal static FileTreeStatistics MeasureDirectory(string directoryPath) | ||
| { | ||
| long fileCount = 0; | ||
| long totalBytes = 0; | ||
| foreach (var filePath in Directory.EnumerateFiles( | ||
| directoryPath, | ||
| "*", | ||
| OutputEnumerationOptions)) | ||
| { | ||
| checked | ||
| { | ||
| fileCount++; | ||
| totalBytes += new FileInfo(filePath).Length; | ||
| } | ||
| } | ||
|
|
||
| return new FileTreeStatistics(fileCount, totalBytes); | ||
| } |
There was a problem hiding this comment.
Using Directory.EnumerateFiles followed by new FileInfo(filePath).Length for each file is inefficient because it performs an extra OS metadata query for every single file. In .NET, DirectoryInfo.EnumerateFiles returns FileInfo objects directly, which avoids manual allocations and redundant system calls.
internal static FileTreeStatistics MeasureDirectory(string directoryPath)
{
long fileCount = 0;
long totalBytes = 0;
var directoryInfo = new DirectoryInfo(directoryPath);
foreach (var fileInfo in directoryInfo.EnumerateFiles(
"*",
OutputEnumerationOptions))
{
checked
{
fileCount++;
totalBytes += fileInfo.Length;
}
}
return new FileTreeStatistics(fileCount, totalBytes);
}| public static void StreamDecompressWithHeader(Stream inStream, Stream outStream) | ||
| { | ||
| var decoder = new Decoder(); | ||
| var properties = new byte[5]; | ||
| if (inStream.Read(properties, 0, properties.Length) != properties.Length) | ||
| throw new InvalidDataException("LZMA input is too short."); | ||
|
|
||
| long outSize = 0; | ||
| for (var index = 0; index < sizeof(long); index++) | ||
| { | ||
| var value = inStream.ReadByte(); | ||
| if (value < 0) | ||
| throw new InvalidDataException("LZMA input does not contain an output length."); | ||
| outSize |= (long)(byte)value << (8 * index); | ||
| } | ||
|
|
||
| if (outSize < 0) | ||
| throw new InvalidDataException($"LZMA output length cannot be negative: {outSize}."); | ||
|
|
||
| decoder.SetDecoderProperties(properties); | ||
| var compressedSize = inStream.Length - inStream.Position; | ||
| decoder.Code(inStream, outStream, compressedSize, outSize, null); | ||
| } |
There was a problem hiding this comment.
Using Stream.Read without guaranteeing that the full buffer is read can lead to partial reads and corrupt data. In modern .NET, Stream.ReadExactly is available and should be used to ensure the full buffer is read. Additionally, we can stack-allocate the 8-byte buffer for the size and use BinaryPrimitives.ReadInt64LittleEndian to read the size efficiently without 8 separate ReadByte calls.
public static void StreamDecompressWithHeader(Stream inStream, Stream outStream)
{
var decoder = new Decoder();
var properties = new byte[5];
try
{
inStream.ReadExactly(properties);
}
catch (EndOfStreamException)
{
throw new InvalidDataException("LZMA input is too short.");
}
Span<byte> sizeBuffer = stackalloc byte[8];
try
{
inStream.ReadExactly(sizeBuffer);
}
catch (EndOfStreamException)
{
throw new InvalidDataException("LZMA input does not contain an output length.");
}
long outSize = System.Buffers.Binary.BinaryPrimitives.ReadInt64LittleEndian(sizeBuffer);
if (outSize < 0)
throw new InvalidDataException($"LZMA output length cannot be negative: {outSize}.");
decoder.SetDecoderProperties(properties);
var compressedSize = inStream.Length - inStream.Position;
decoder.Code(inStream, outStream, compressedSize, outSize, null);
}
No description provided.