Port GifControlDirectory additions from Java #181
Conversation
|
|
||
| if (blockSizeBytes > 3) | ||
| reader.Skip(blockSizeBytes - 3); | ||
| directory.Set(GifControlDirectory.TagTransparentColorIndex, reader.GetByte()); |
There was a problem hiding this comment.
This used to sometimes skip three bytes, but now always consumes only one. Is this correct?
There was a problem hiding this comment.
I don't know for sure without further investigation. This was ported directly from the Java version approved through drewnoakes/metadata-extractor#241.
Maybe @Nadahar has some info on it?
There was a problem hiding this comment.
I'm sorry, I checked in my memory, but I'm unable to revive any information about this. From what I can see from the PR, additional parsing was added. It doesn't seem unreasonable that less will be skipped when more is parsed.. does this behavior cause a problem?
There was a problem hiding this comment.
additional parsing was added
It's just unpacking a single byte that was previously skipped.
@kwhopper if you're happy that your testing produced better results with this change then I'm happy.
There was a problem hiding this comment.
It appears to produce the same results as the Java implementation against the image repository. To me, that's the best we can reasonably expect when porting.
There was a problem hiding this comment.
Thank you for confirming. Merged!
Could you open issue for these two please so they're not lost? |
Todo: backport GifControlDescriptor "{value}" addition to Java
Todo: backport to Java GifReader -- directory.Set(GifControlDirectory.TagUserInputFlag, (packedFields & 2) == 2);