ping!

friendly ping! I would like to continue work on this. Is the new name better? If so, I will rebase follow-up patches

LGTM

I have thought about this and have come to the conclusion this is not really future proof.

The Property::FrontCover is IMHO fine, i.e. the changes to propertyinfo.cpp and properties.h could go in as is.

The request of additional properties should be done differently. How about the following:

1. divide properties into (currently) two sets, "Simple Metadata" (all the current ones but frontcover) and "Complex Metadata" (currently frontcover).
2. allow to specify extra properties for SimpleExtractionResult

SimpleExtractionResult result(fileName, mimeType);
result.setExtraProperties({Property::FrontCover});
plugin.extract(&result);

You can then also do:

SimpleExtractionResult result(fileName, mimeType, ExtractionResult::ExtractNothing);
result.setExtraProperties({Property::FrontCover, Property::BandLogo});
plugin.extract(&result);

In D25517#587776, @bruns wrote:

I have thought about this and have come to the conclusion this is not really future proof.

The Property::FrontCover is IMHO fine, i.e. the changes to propertyinfo.cpp and properties.h could go in as is.

The request of additional properties should be done differently. How about the following:

1. divide properties into (currently) two sets, "Simple Metadata" (all the current ones but frontcover) and "Complex Metadata" (currently frontcover).
2. allow to specify extra properties for SimpleExtractionResult

Not strictly against your proposal, but how much do we gain? What is your concern about future proof?
From my Elisa application perspective, it would be very convenient to use the proposed ExtractionResult::ExtractEverythingIncludingImageData (or renamed ExtractionResult::ExtractEverything for KF6) and the ExtractionResult::ExtractImageData flags.
For the current proposal the API does not need to be changed.

SimpleExtractionResult result(fileName, mimeType);
result.setExtraProperties({Property::FrontCover});
plugin.extract(&result);

You can then also do:

SimpleExtractionResult result(fileName, mimeType, ExtractionResult::ExtractNothing);
result.setExtraProperties({Property::FrontCover, Property::BandLogo});
plugin.extract(&result);

This is also possible with the current proposal (when the extractors honor the ExtractionResult::ExtractNothing/ExtractMetaData flags properly, which I think currently none does) by just setting the ExtractionResult::ExtractImageData.
It is just that one does not have fine-grained control over each individual "complex" property, but IMHO we do not need that.
I also fear that it may not be clear to distinguish between "simple" and "complex" properties and that users of the API would expect that every property works for the setExtraProperties, which would require an immense amount of work for all extractors.

In D25517#587957, @astippich wrote:

I also fear that it may not be clear to distinguish between "simple" and "complex" properties and that users of the API would expect that every property works for the setExtraProperties, which would require an immense amount of work for all extractors.

I agree.

The problem is the extractor serves two different use cases:

• retrieval for searching/caching properties
• on-demand extraction

Baloo falls into the first category. It is only interested in properties which can be queried/compared in a
meaningful way. Even storing the properties as a cache for e.g. Dolphin is a little bit of a stretch.

Elisa falls into the second category. It does not store the extracted data persistently, but is fine
extracting e.g. the front cover on demand.

Currently the only large/blob property is the front cover. But what about BackCover, Leaflet pages,
..., Video Thumbnails? Returning all of these even when not required is wasteful. If you want a
front cover, say so.

For KF6, ExtractEverything should just be removed. It is not especially useful, not even for unit tests.
Request the data you want and are able to handle, and do so explicitly.

In D25517#588029, @bruns wrote:

The problem is the extractor serves two different use cases:

• retrieval for searching/caching properties
• on-demand extraction
  
  Baloo falls into the first category. It is only interested in properties which can be queried/compared in a meaningful way. Even storing the properties as a cache for e.g. Dolphin is a little bit of a stretch.
  
  Elisa falls into the second category. It does not store the extracted data persistently, but is fine extracting e.g. the front cover on demand.
  
  Currently the only large/blob property is the front cover. But what about BackCover, Leaflet pages, ..., Video Thumbnails? Returning all of these even when not required is wasteful. If you want a front cover, say so.

Yeah, in that case it would be a little bit wasteful. But again, I think if we implement this fine-grained control, it should be done for all properties.
And this is out-of-scope for this patch.

For KF6, ExtractEverything should just be removed. It is not especially useful, not even for unit tests.
Request the data you want and are able to handle, and do so explicitly.

I have adjusted this as requested.

friendly ping for the series

Where are we with this? @bruns are you accepting the proposed solution or do you reject it completely?
I think it is at least way better than the current solution and maintains current API.
A larger refactoring could be done for KF6

Ping! At least a reply would be nice

ping @bruns
this has now been open and unanswered for month!
Same for the dependent patches

Since I got one accept and no other response, I will merge on the weekend

Due to the recent circumstances I will probably not implement the support in Elisa, so I won't land this unless @bruns explicitly is okay with it (I still think this is an improvement over the status quo).