Re: [Libjpeg-turbo-devel] libjpeg-turbo for Android

[Matt S. <ms...@go...>]

Sweet!  That's definitely good news.

Best,
Matt

On Thu, Jun 4, 2015 at 2:15 PM, DRC <dco...@us...>
wrote:

> Since we're talking about a new feature here, it would not bother me if
> we assumed that the new feature only works with non-suspending data
> sources (although of course the existing API still needs to work with
> suspending data sources.)  There are a lot of features in the libjpeg
> API that served a valid purpose back when CPU and memory resources were
> much more constrained but are of debatable usefulness these days, and
> IMHO, I/O suspension is one of those features.
>
>
> On 6/4/15 12:34 PM, Matt Sarett wrote:
> > I'm guessing that the implementation would need to support suspending
> > and non-suspending data sources?  We don't currently have a need for
> > supporting suspending data sources, so I just want to make sure that the
> > need exists somewhere else before working it into the implementation.
> >
> > Best,
> > Matt
> >
> > On Mon, Jun 1, 2015 at 5:03 PM, Matt Sarett <ms...@go...
> > <mailto:ms...@go...>> wrote:
> >
> >     Thanks for the info, that's actually very helpful!
> >
> >     I'll try to explain a little bit better what I'm trying to do.
> >
> >     Let's say we are trying to skip 8 rows on a baseline jpeg (and our
> >     current row is a multiple of 8, so we are not in the middle of a
> >     block.  I would simply call decode_mcu() for all of the MCU_cols and
> >     adjust the output_scanline down 8 rows.
> >
> >     When context based upsampling is enabled, things become a little bit
> >     trickier, because it's not safe to skip the final row that the
> >     caller asks to skip.  If the next call is a read, we need to have
> >     the results of the previous row in order to do the upsampling.
> >
> >     For progressive decodes, all of the entropy decoding occurs
> >     upfront.  To skip, it's sufficient just to adjust the output
> >     scanline and current MCU row.
> >
> >     I haven't made any modifications to the actual code that performs
> >     entropy decoding.  I am considering adding alternate versions that
> >     do not write to a buffer, since we don't use the coefficients anyway
> >     (if this improves performance).
> >
> >     I'm still at a relatively early stage of understanding the existing
> >     code, so these strategies may change, but hopefully they give you an
> >     idea of the approach that I am trying to take.  My hope is to take
> >     advantage of the existing entropy decoding modules to move ahead in
> >     the input stream without actually doing any additional work after
> that.
> >
> >     Let me know if you have any additional thoughts!
> >
> >     Best,
> >     Matt
> >
> >     On Mon, Jun 1, 2015 at 1:27 PM, DRC
> >     <dco...@us...
> >     <mailto:dco...@us...>> wrote:
> >
> >         The automated tests that are run with 'make test' are the only
> test
> >         suite I have, and it's much more comprehensive than the one in
> >         libjpeg.
> >            However, these tests are primarily designed to cover all of
> the
> >         various algorithms that might be SIMD-accelerated, as well as the
> >         various entropy algorithms.  The goal of 'make test' is to fail
> >         if any
> >         one of these algorithms is implemented incorrectly for a
> particular
> >         architecture.
> >
> >         In order for me to tell you which encoding settings might have
> any
> >         bearing on what you're doing, I need to have a better
> >         understanding of
> >         which parts of the code your modifications are touching.  If
> >         you're just
> >         dealing with the entropy stuff, then you would need to test all
> >         of the
> >         permutations of:
> >
> >         -- Baseline or progressive encoding
> >         -- Huffman or Arithmetic entropy coding
> >         -- Optimized or unoptimized entropy
> >
> >         libjpeg-turbo and libjpeg do not support lossless or
> >         hierarchical JPEGs.
> >            structure.txt may provide further information that is of use.
> >
> >         libjpeg[-turbo] does support non-interleaved JPEGs, but
> >         generating one
> >         isn't exactly straightforward.  Refer to the -scans argument for
> >         cjpeg
> >         in wizard.txt.
> >
> >
> >         On 6/1/15 10:29 AM, Matt Sarett wrote:
> >         > DRC,
> >         >
> >         > So I've made some good progress so far.  I have a working
> implementation
> >         > (thought I'm not sure it's the final one) for three classes of
> images:
> >         > sequential encoding, sequential encoding with h2v2 upsampling
> (where
> >         > context rows are required), and progressive encoding.
> >         >
> >         > I'm aware that there are plenty more types of jpegs in
> existence, but
> >         > I'm not sure which are supported by libjpeg-turbo and I don't
> have test
> >         > images that cover these.  Do you have a test suite that you
> might be
> >         > able to share that would help me test for additional cases?
> >         >
> >         > I'd also value any thoughts you might have on tricky/rare
> types of jpegs
> >         > I may have missed.  I think I may be missing progressive jpegs
> with
> >         > context upsampling (if these exist), hierarchical and lossless
> jpegs,
> >         > and non-interleaved jpegs etc.
> >         >
> >         > Any guidance you have would be appreciated.
> >         >
> >         > Thanks!
> >         > Matt
> >         >
> >         > On Thu, May 21, 2015 at 3:03 PM, Matt Sarett <
> ms...@go... <mailto:ms...@go...>
> >         > <mailto:ms...@go... <mailto:ms...@go...>>>
> wrote:
> >         >
> >         >     That's great to hear!
> >         >
> >         >     It sounds like we have similar goals.  I certainly hope
> that the
> >         >     patch will be as non-disruptive as it can be.  It is my
> initial
> >         >     impression that it may be possible to implement without
> modifying
> >         >     any of the existing pathways.
> >         >
> >         >     Thanks for the background on the Huffman decoder.  It was
> my plan to
> >         >     lean on the existing code, and I will pay attention to
> respecting
> >         >     the implementation that is already in place.
> >         >
> >         >     This is definitely still in the early stages, but I
> appreciate your
> >         >     offer consult and I will certainly be in touch with any
> >         >     questions/concerns.
> >         >
> >         >     Talk to you soon,
> >         >     Matt
> >         >
> >         >     On Thu, May 21, 2015 at 12:58 PM, DRC
> >         >     <dco...@us...
> >         <mailto:dco...@us...>
> >         >     <mailto:dco...@us...
> >         <mailto:dco...@us...>>> wrote:
> >         >
> >         >         I would be very receptive to that.  Regarding the
> previous Android
> >         >         patches, I did contribute a significant amount of pro
> bono labor
> >         >         integrating some things like RGB565 decoding that I
> felt were of
> >         >         broad
> >         >         benefit to the project (the RGB565 feature required
> significant
> >         >         refactoring before it could be made to work properly
> outside of an
> >         >         Android-specific context.)  There were some smaller
> things I
> >         >         rejected
> >         >         from the patch set, such as:
> >         >
> >         >         -- the Android-specific memory management code-- due
> to an
> >         >         incompatible
> >         >         license
> >         >         -- the Android.mk build system.  It's project policy
> not to
> >         >         include any
> >         >         new build systems that are more limited in scope than
> the
> >         >         existing build
> >         >         systems, and after many hours of trying, I was never
> able to
> >         >         make the
> >         >         Android.mk system properly build static and shared
> libraries.
> >         >         Thus, my
> >         >         position was that that build system should be
> maintained downstream,
> >         >         since it is really meant for building libjpeg-turbo as
> a system
> >         >         library,
> >         >         not for user or app developer consumption.
> >         >
> >         >         The major thing I rejected was the tiled decoding
> patch, and I
> >         >         didn't
> >         >         reject it per se.  I just said that it was very
> disruptive and would
> >         >         require quite a lot of effort to integrate and test
> properly, and I
> >         >         wasn't willing to do that work for free.  Because that
> patch had
> >         >         a lot
> >         >         of touch points within the libjpeg portion of the
> code, it would
> >         >         have
> >         >         required a great deal of testing, and no one at the
> time was able to
> >         >         explain to me adequately why the feature was
> necessary-- your
> >         >         message
> >         >         below is actually the explanation I've been looking
> for for four
> >         >         years.  :|
> >         >
> >         >         Because libjpeg-turbo is so stable and ubiquitous at
> this point,
> >         >         I have
> >         >         to be very conservative regarding what I check into
> the code base.
> >         >         Because I am an independent developer, that makes
> things even
> >         >         trickier,
> >         >         because often it's the case that, if I integrate or
> develop a new
> >         >         feature and that feature breaks down the road, I end
> up having
> >         >         to fix it
> >         >         for free (because, even if I was being paid to develop
> the
> >         >         feature, the
> >         >         original contract has long expired.)  The even worse
> situation
> >         >         is when I
> >         >         get a feature submission that looks clean, I integrate
> it, but
> >         >         it breaks
> >         >         down the road and I can't contact the developer.  I'm
> going
> >         >         through that
> >         >         situation right now with the MIPS SIMD code.
> >         >
> >         >         Thus, I think the ideal situation here would be for
> Google to
> >         >         develop
> >         >         this patch in a way that is as non-disruptive as
> possible and
> >         >         for us to
> >         >         establish a long-term relationship regarding its
> maintenance.  2
> >         >         years
> >         >         from now, if (for instance) some application developer
> discovers
> >         >         a bug
> >         >         in this new feature, I want to be able to e-mail
> someone at
> >         >         Google and
> >         >         say "fix this" rather than having to invest my own pro
> bono
> >         >         labor to fix
> >         >         code I didn't develop in the first place.  I want to
> be able to
> >         >         look at
> >         >         the diff and see that the new code is, to the fullest
> extent
> >         >         possible,
> >         >         used only when it is needed and that it interferes as
> little as
> >         >         possible
> >         >         with the existing code.  I like new features that can
> easily be
> >         >         walled
> >         >         off and disabled at run time if they prove to be buggy.
> >         >         Particularly
> >         >         vis-a-vis the Huffman decoder, that code has undergone
> intense
> >         >         scrutiny
> >         >         by Mozilla and others over the years, so I am loathe
> to do
> >         >         anything that
> >         >         might undo any of our hard work in stabilizing and
> securing that
> >         >         code.
> >         >
> >         >         There will likely still be some effort required on my
> part to
> >         >         properly
> >         >         integrate the code-- related to the proper setting of
> the ABI
> >         >         revision
> >         >         number in the shared libraries, etc.-- but as long as
> the patch is
> >         >         clean, the integration work should be minor, and I
> don't mind
> >         >         doing it.
> >         >
> >         >         So yes, I would definitely be interested in
> integrating this, as
> >         >         I think
> >         >         the feature would also benefit other users.  I am
> happy to
> >         >         consult with
> >         >         you regarding how best to implement it behind the
> scenes, if you
> >         >         need me
> >         >         to, but in general, if you have a good idea of what
> you want to
> >         >         do, then
> >         >         I would say just develop it and submit the patch
> through the patch
> >         >         tracker once you have something you think is stable.
> >         >
> >         >
> >         >         On 5/21/15 11:06 AM, Matt Sarett wrote:
> >         >         > Turbo Developers,
> >         >         >
> >         >         >
> >         >         > I am a member of the Skia graphics team within
> Google.  We feel that
> >         >         > making Android a user of libjpeg-turbo will have
> significant benefits
> >         >         > for jpeg decodes on Android.  In particular, we will
> benefit from the
> >         >         > fast and accurate Neon IDCT, optimized Huffman
> decodes, flexible color
> >         >         > conversions, and flexible scaling.  We are excited
> about the potential
> >         >         > impact of this project!
> >         >         >
> >         >         >
> >         >         > A feature of our current Android jpeg decoder
> (libjpeg plus patches) is
> >         >         > the ability to decode image subsets efficiently.
> The idea is that if we
> >         >         > only need to display a subset of an image, we will
> improve performance
> >         >         > by decoding only that subset (rather than decoding
> the entire image and
> >         >         > then cropping).
> >         >         >
> >         >         >
> >         >         > Despite the optimal features of libjpeg-turbo, our
> current decoder
> >         >         > outperforms libjpeg-turbo on a given set of subset
> decoding performance
> >         >         > benchmarks.  libjpeg-turbo suffers on subsets near
> the bottom of the
> >         >         > image (because it must decode all preceding rows)
> and on thin vertical
> >         >         > stripe subsets (because it must decode the each row
> in its entirety).
> >         >         >
> >         >         >
> >         >         > We are not concerned with the vertical stripe
> subsets given that this
> >         >         > decode is rare and optimizing this case is not
> particularly useful,
> >         >         > however, we are hoping to make a contribution to
> libjpeg-turbo to aid
> >         >         > with subsets at the bottom of the image.
> >         >         >
> >         >         >
> >         >         > Specifically we feel it would be beneficial to
> extend the standard API
> >         >         > with the additional function:
> >         >         >
> >         >         > GLOBAL(JDIMENSION) jpeg_skip_scanlines
> (j_decompress_ptr cinfo,
> >         >         > JDIMENSION lines);
> >         >         >
> >         >         >
> >         >         > This function would advance the input source (and
> the row counters etc.)
> >         >         > past the input number of lines without actually
> performing the work of
> >         >         > decoding these lines.  We feel that this would make
> libjpeg-turbo
> >         >         > superior to our current jpeg decoder in all
> important cases and would
> >         >         > nullify any opposition within Android to switching
> over to
> >         >         > libjpeg-turbo.  I am currently working on
> implementing this API in a
> >         >         > clean, efficient way that avoids touching the
> existing code path as much
> >         >         > as possible.
> >         >         >
> >         >         >
> >         >         > We are aware of the effort that started in 2011 to
> upstream Android's
> >         >         > subset decoding to libjpeg-turbo, and we understand
> turbo's objections
> >         >         > to those patches.  Maintaining a working version of
> our subset decode on
> >         >         > top of an evolving libjpeg has been a big headache
> for us and has
> >         >         > hampered our ability to continue to update our jpeg
> library.  It is not
> >         >         > surprising that turbo chose to avoid that headache,
> and it is our goal
> >         >         > to move to a new approach that allows us to get past
> these problems as well.
> >         >         >
> >         >         >
> >         >         > We hope that the new approach would be more
> appealing in that it would:
> >         >         >
> >         >         >   *
> >         >         >
> >         >         >     Maintain ABI compatibility
> >         >         >
> >         >         >   *
> >         >         >
> >         >         >     Minimize touch points with other libjpeg-turbo
> pathways
> >         >         >
> >         >         >   *
> >         >         >
> >         >         >     Continue to allow libjpeg-turbo to act as a
> drop-in replacement for
> >         >         >     libjpeg
> >         >         >
> >         >         >
> >         >         > Do you have any thoughts on this type of addition to
> libjpeg-turbo?
> >         >         > Would you be receptive to upstreaming this type of
> patch?  We certainly
> >         >         > recognize that you are the experts of libjpeg-turbo
> and would value your
> >         >         > input as we begin to pursue to this project.
> >         >         >
> >         >         >
> >         >         > Best,
> >         >         >
> >         >         > Matt
> >
> >
>  ------------------------------------------------------------------------------
> >         _______________________________________________
> >         Libjpeg-turbo-devel mailing list
> >         Lib...@li...
> >         <mailto:Lib...@li...>
> >         https://lists.sourceforge.net/lists/listinfo/libjpeg-turbo-devel
> >
> >
> >
> >
> >
> >
> ------------------------------------------------------------------------------
> >
> >
> >
> > _______________________________________________
> > Libjpeg-turbo-devel mailing list
> > Lib...@li...
> > https://lists.sourceforge.net/lists/listinfo/libjpeg-turbo-devel
> >
>
>
> ------------------------------------------------------------------------------
> _______________________________________________
> Libjpeg-turbo-devel mailing list
> Lib...@li...
> https://lists.sourceforge.net/lists/listinfo/libjpeg-turbo-devel
>