Skip to content
This repository was archived by the owner on Jul 22, 2018. It is now read-only.

Rename 'BE' to "Swap",#27

Open
klauspost wants to merge 1 commit intodevelopfrom
develop_rename_be
Open

Rename 'BE' to "Swap",#27
klauspost wants to merge 1 commit intodevelopfrom
develop_rename_be

Conversation

@klauspost
Copy link
Owner

Since it is really behaving as a swapper, and does only reads "Big Endian" on little endian platforms (albeit that is the wast majority).

Pedro, is this ok for you, if I merge this?

…does only reads Big Endian on little endian platforms (albeit that is the wast majority).
@pedrocr
Copy link
Contributor

pedrocr commented Jun 29, 2014

That's actually not the case. The files really are big endian or little endian. On little-endian architectures the bits are swapped on big endian they are not. It's the nature of the C shift operator that makes the operation architecture agnostic.

@klauspost
Copy link
Owner Author

Yes - however, it is only "TiffIFDBE" and "TiffEntryBE". These classes actually swap endianness.

I am not referring to the function names in RawDecoder, which are correct, and will remain as they are.

@klauspost
Copy link
Owner Author

Whether or not a TiffEntry is "TiffEntry" or "TiffEntryBE" doesn't actually say if the data is LE or BE. On a BE system it will be the other way around.

So I would like to rename them to avoid that confusion.

@pedrocr
Copy link
Contributor

pedrocr commented Jun 29, 2014

Ah, if that's the case then DcrDecoder is wrong. I used TiffIFDBE to mean "parse a big endian Tiff". I see that's wrong now as can be seen in TiffParser. It will break on BE architectures.

I'd argue it would be cleaner to change it the other way around though, to make TiffIFDBE mean "parse big endian" and TiffIFD "parse little endian" and have both of them swap bytes when they're in the opposite architecture.

But that's a more intrusive change. If you want to keep the current code changing them to TiffIFD and TiffIFDSwap makes sense. TiffIFDBE and TiffIFDLE could be implemented on top of those two by checking the host endianness.

@pedrocr
Copy link
Contributor

pedrocr commented Aug 9, 2014

I was looking at this while implementing a CIFF parser for CRW files. It would make a lot more sense to push the big/little stuff into stuff like FileMap and ByteStream and have TiffIFD and TiffEntry handle both types of files.

@klauspost
Copy link
Owner Author

Yes, after some back&forth, I agree that would be both simpler and more reliable, and not something that should have any big performance impact.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants