Re: [Tracker] PATCH: Faster PNG extractor

[Philip Van Hoof <philip codeminded be>]

Op 27/06/2013 19:06, Martyn Russell schreef:
> That's quite some difference!
>
> Thanks for posting some numbers. Important! :)
>
> My first thought is, why did you create a new extractor instead of 
> improve the original one?

Hi Martyn,

I think they might have opted for a different extractor because of this 
reason:

"If PNG-faster fails to read the PNG file (which should happen if, for 
instance the IDAT chunks are variable sized), the extractor will fail 
and the regular PNG extractor can re-try the extraction."

To be honest I kinda like the .rule file behaviour solution for this, 
although letting it fall back within one module also sounds good to me. 
Just that in terms of code maintainability and how the .rule system is 
designed, letting a 'faster' extractor fail based on file content's 
possibilities (in case the failure is trapped by a more generic 
extractor in the fall back) should in my opinion be fine as a practice. 
For example the faster video extractors also come upfront the slower 
GStreamer ones using the .rule system, for a similar reason.

> The patch link you gave is good, but I would love to see a diff from 
> our actual extractor right now to see how easily we could merge the 
> changes into that one.

I think it should be open for debate first wether or not we want the 
specialized "fixed-sized IDAT chunk" based PNG extractor to be a 
separate extractor module and use the .rule system for prioritizing and 
allowing extractor modules to fall back to lower priority modules, or 
wether we want it to be in one PNG extractor.

Kind regards,

Philip