RgbaInputFile: Multipart support

[ignus2]

The simplest way to add multipart support to RgbaInputFile. I needed this in a project and this was the easiest solution I found.
It might be useful, please take a look and comment if this is worthwhile for integration.

[linux-foundation-easycla]

The committers are authorized under a signed CLA.

• ✅ Balázs Oroszi (989a624)

[peterhillman]

This will cause a memory leak if any of the following lines throw an exception, since the destructor won't be called to delete the _multiPartFile.

[ignus2]

Indeed. Does the same apply to _inputFile in the other existing constructor as well? Here _inputFile is simply new'd too, that's what I used as a basis basically:

RgbaInputFile::RgbaInputFile (const char name[], int numThreads):
    _inputFile (new InputFile (name, numThreads)),
    _fromYca (0),
    _channelNamePrefix ("")
{
    RgbaChannels rgbaChannels = channels();

    if (rgbaChannels & WRITE_C)
	_fromYca = new FromYca (*_inputFile, rgbaChannels);
}

[peterhillman]

I don't think it's too much burden to switch this to an InputPart, and use the MultiPart API for all constructors.
That also means you don't need the 'friend' in the header

[peterhillman]

The disadvantage of this approach is that you need to know how many parts are in the file. You also cannot use this API to scan through the parts of a file without reopening the file each time with a different partNumber

A more complete API might provide a 'parts()' method to return the number of parts, and a 'setPart' method to change the part used for subsequent reads, as setLayerName() does.

[ignus2]

Yes, this also ties into your previous comment regarding using the MultiPart API for all constructors.
I'm not sure if I'm the best person for this task, maybe this should have been a feature request (with my code shown as a proof of concept) rather than a pull request.
What do you think/propose?

[peterhillman]

I think it's a great addition, and I like your approach. If you'd like to go ahead and fix the DCO and CLA issues, we can merge it as is. I'm happy to make extra tweaks on top of that. Otherwise, I guess we can start again?

[ignus2]

I'd say take the code and run. I could go about the DCO/CLA stuff but if it's OK with you just take the commit, cherry pick or copy or use a basis, as you prefer. From my perspective I proposed this because I need this in a project, and it's not convenient to maintain a fork.
Let me know what you think.

[lgritz]

Technically, that's against the spirit behind the DCO/CLA. The PR is your code, so "take it and run" is really just another way of saying "commit it without a DCO or CLA," right? Somebody else re-typing it doesn't change the original authorship or ownership of the code. It's the DCO/CLA that is the formality that certifies to us that it is indeed ok with the owner of the code to incorporate it into the project under the project's license terms.

[ignus2]

DCO/CLA done. Now you can legally take the code and run ;)

[lgritz]

Thanks!

[cary-ilm]

Following up on this, @peterhillman and @lgritz, you indicated it's ok as is but didn't formally approve the review. You want to give it a formal approval?

[lgritz]

I was only chiming in on whether the CLA was needed. I don't know enough about the internals of RgbaInputFile to have an opinion about the code.

[peterhillman]

I've done a little work extending this PR further in #1201.
If #1201 is approved, we should approve #1194 then merge it first and #1201 straight afterwards.