Re: [Tracker] PATCH: Faster PNG extractor

[Martyn Russell <martyn lanedo com>]

On 28/06/13 07:45, Philip Van Hoof wrote:
> Op 28/06/2013 8:30, Jonatan Pålsson schreef:
>
> Hi Jonatan,

Hello Jonatan, Philip,

> > The main reason for not modifying the original extractor is that I
> > want to keep it as a fallback if this new extractor fails due to an
> > unexpected file structure. Since png-faster tries to skip to the end
> > of the file by estimating the location of the metadata contained in
> > the end of the file using the file size & IDAT chunk size, I predict
> > it may fail more often than the original. Since tracker-extract
> > handles these failures gracefully, this is not a problem however.
> >
> > The best way I can see to get a similar functionality in to the
> > existing extractor would be to modify libpng to allow skipping to the
> > end of the file (right now there is a comment in the existing png
> > extractor noting that this functionality is missing from the library),
> > but since reading the PNG format is relatively simple I opted to put
> > this functionality in the extractor rather than first patching libpng
> > (I am not sure how much work this would be, either).
> >
> > What are your thoughts on keeping png-faster as a separate, optional
> > extractor module which can be enabled when extraction speed is of
> > primary concern?
>
> After reading this, my opinion is that we should keep it as separate
> modules. What do you think, Martyn?

I think that really depends on the diff. It's harder to see what the 
real changes are with an entirely new module.

Ideally, I would like to share functionality where possible. That's the 
first thing.

If it's a case of switching between fast() and slow() functions 
depending on IDAT chunks then it's likely to be quicker over thousands 
of files, to do that quick test first and then call the slow function in 
the one extractor instead of switching out to the next module where we 
have to do a bunch of new seeking from scratch. It's hard to say without 
looking at the real changes and should be tested too. I've not looked in 
detail myself yet and I plan to :)

> Patching libpng is a longer term fine solution, too. Not sure how
> willing libpng maintainers will be, but for the time being we can deal
> with the opitmization this way.

Yea, I would go with that if possible and see how receptive the 
community is to that. Usually they are if you come bearing patches :)

--
Regards,
Martyn

Founder and CEO of Lanedo GmbH.