From Bugzilla Helper: User-Agent: Mozilla/5.0 (Windows; U; WinNT4.0; en-US; rv:0.8.1+) Gecko/20010417 BuildID: 2001041704 Progressive Jpegs should display a low quality aproximation of the image, with gradual improvement of quality as the image downloads. But now it does not work this way, the image is displayed when the download has finished. Reproducible: Always Steps to Reproduce: Go to http://fog.ccsf.cc.ca.us/~pthiry/multimedia/monalisabig.jpg Or to http://home.hkstar.com/~hyytep/jpeg.html Or to http://www.netadvies.nl/advies/pjpgdemo.html Actual Results: The image displays when download has finished Expected Results: The image should display progressive with increasing quality For more information: http://www.faqs.org/faqs/jpeg-faq/part1/section-11.html

It also happens in Linux. Changing OS to all It works fine in Navigator 4.x

Marking NEW.

changing status to [imglib]

What is this RFE business? Progressive JPEG support is very important, especially for 56K users. Supporting this is a fundamental part of supporting JPEG images. Upgrading to normal severity.

*** Bug 91689 has been marked as a duplicate of this bug. ***

This really needs to be fixed for nsbeta1 as well. Adding appropriate keyword.

I am seeing the bug - progressive/interlaced jpegs not loading progressively in build: 2001062815 as well.

*** Bug 93281 has been marked as a duplicate of this bug. ***

It depends how you define "progressive". It's broken from the fundimental viewpoint that it won't render in a "raster" mode as with gif -- at full res while being received from top to bottom. Usually "progressive jpeg" means low-res overlayed by incresingly higer-res detail. I'd change the summary to "jpegs don't render as they are received". This is not related to progressive or not.

lohphat, I just tested this quickly, and confirmed that non-progressive JPEGs do display as they are loaded. It's only progressive JPEGs that don't, so this has everything to do with whether a jpeg is progressive or not. No need to change the title.

You're correct. After testing the following URL on NSCP 4.78 I could catch that it is a progressive jpeg. http://www.fuckedcompany.com/images/911/5th_avenue_facing_south.jpg

The attached patch has only been minimally tested and contains a whole load of debugging printf's and cruft at the moment, but does the trick. Feedback welcomed.

Hmm. Still has issues with aborting decoding.

Okay, this one acknowledges that jpeg_finish_output() may suspend, and hence seems stable. The same caveats apply though, including a pretty slow line-by-line way of notifying the image observer, plus any JPEG decoding errors having been temporarily promoted to fatal.

Okay, http://www.battletrolls.com/html/homepage/noflash.php seems to be a good (nasty) testcase. This latest (#3) pair of patches against nsJPEGDecoder.h and nsJPEGDecoder.cpp (supposedly) improve the suspendability of jpeg_finish_output() calls and adds suspendability for jpeg_start_output(). It's the stablest version yet (that is, I can't make it emit a libjpeg error and racily abort decoding). The fly in the ointment is at line 499 of the patched nsJPEGDecoder.cpp, where I have inserted a |break| to keep us in the 'more data please'/JPEG_WAIT_STARTOUT state forever; this is because if I let it pass the point where jpeg_start_output() has suspended once, it starts corrupting memory whether I retry, consume, whatever. Possibly a libjpeg bug, more likely I really need someone with superior libjpeg juju to take a look and test my sanity (it's late).

Adding keywords...this needs a review/super review

Removing 'patch' and 'review' keywords -- this is strictly a work-in-progress with known problems, and right now I'm looking for comments, not r/sr.

adam: What is the status of your patch? If you plan to finish it, you might take the assignment to get it off pavlov's list.

I hope to finish it some time, but realistically that's probably at least weeks away (I'm very busy at the moment). Meanwhile I'd rather leave it as pav's if he wants it. :)

Removing nsbeta1 nomination because this bug has been plussed.

Setting Severity=Minor since final image display isn't actually interdicted.

giving to nivedita. you can work with adam@gimp.org on this as he has already spent some time working on it.

I'm out of time, so my last patch I think is as far as I go. IIRC when I left it I was wondering if jpeglib was wanting the ability to 'rewind' a few bytes to pick up rendering an interrupted stream, and we hadn't implemented such an ability in the IO methods we were providing. It does look to me like something buffering-related, either way. Two progressive quick-win options if we don't get things working properly in our short timescales: 1) Turn off smoothing of the incomplete levels. We might be paying the smoothing cost even though we're not displaying progressively and don't see the visual benefit. 2) At least display the final pass progressively. The final pass accounts for probably the majority of the file size (but not the majority of visually-useful data, hence the benefits of progressive rendering) so at least we'll have started to show *something* good before we most of the stream down. I *think* that the last version of the WIP patch will reliably do this with a small change or two for a quick implementation, but is better being implemented from scratch for less code-impact if you're not interested in picking up the other baggage which was aimed towards full progressive rendering support.

I applied the patches but I could observe progressive jpeg behaviour. The image got displayed as in sequential jpg, rather than the one which improves in quality over the subsequent scans. Am I missing some thing here.

Well, that's odd. The patch looks likes what's been in my tree for months, and I do get progressive rendering (modulo the known intermittant early-bailout bug). I'm on unix/X11. I'd be surprised if that made a difference, but... Perhaps your connection is too fast? The progressive rendering will coalesce earlier passes if there is enough buffered data coming in to skip ahead to a later, better pass.

My observations were specific to windowsNT platform, let me check it out on Linux as well.

I could observe some difference in the jpeg diplay on the Linux build. It appeared fazy to clear. But it was too fast. I think I with agree you about the connection being fast .

moving to 1.0.1

.

pav/gagan - what are the chnaces that we'd be able to get a fix for this before 06.14.2002? pls add eta, or nsbeta1- (indicating that it is not needed for MachV). thanks!

Comment on attachment 86982 [details] [diff] [review] pick up a missing i/o suspension case Looks good. r=adam@gimp.org

Taking bug.

I've been playing with a patched trunk built today and it's worked flawlessly for me. A big image to test progressive loading with is here: http://wfmh.org.pl/~thorgal/wtc-photo-small.jpg

Comment on attachment 86995 [details] [diff] [review] handle a couple more (last?) i/o suspension cases I don't like the: + /* There's been an error - return NS_OK so that + libpr0n doesn't throw away a partial image load */ If you don't return an error, you will end up with the partially loaded image in the cache. That doesn't seem like a good thing. The flag image status flag STATUS_LOAD_PARTIAL is meant to be used for this kind of thing. Perhaps an additional error should be added and returned. http://lxr.mozilla.org/seamonkey/source/modules/libpr0n/public/ImageErrors.h

That bit of code will only get triggered if the image stream is corrupt, so what's the harm in caching the image? It's a lot better than tantalizing the user by incrementally loading an image and then whipping it away if libjpeg sees something wrong.

we should add the non-fatal errors to this list so that they can return a successful error code instead of always returning success.

Comment on attachment 87257 [details] [diff] [review] as above, with pav's suggest and a cynical comment r=pavlov

Comment on attachment 87257 [details] [diff] [review] as above, with pav's suggest and a cynical comment sr=jag

Checked into trunk.

Backed out due to Tp regression (bug 153280).

*** Bug 153433 has been marked as a duplicate of this bug. ***

Crap! I hope this patch makes it back in soon. It seems to me that to render pjepgs progressively, you'd almost *have* to take a little performance hit, so I hope people aren't thinking of taking this feature out completely! I would think that even if you change it so it doesn't render to screen quite as often, there'll still be a slight perf hit.

The trick is quantifying what is an acceptable perf hit, since it depend on the network speed. See http://bugzilla.mozilla.org/show_bug.cgi?id=153280#c2

Compare the way this works in Mozilla to the way this works in NS4. In NS4 it only draws one pass using the data that's available, then does a final pass at full quality. Maybe if it was rendered like this in Mozilla the performance hit wouldn't be as big of a deal.

Comment on attachment 88723 [details] [diff] [review] as above plus fix for bug 153433 (background pjpeg broken) r=pavlov

Comment on attachment 88723 [details] [diff] [review] as above plus fix for bug 153433 (background pjpeg broken) sr=jag

Comment on attachment 90154 [details] [diff] [review] make sure we show a complete scan during incremental loading r=biesi if you move the do..while above the for(;;) as discussed on irc

Comment on attachment 90242 [details] [diff] [review] loop moved per biesi's comments sr=jag

Checked in on trunk.

Fixed

Leaving open for possible branch pickup.

Updating URL; original is 404.

Comment on attachment 90242 [details] [diff] [review] loop moved per biesi's comments branch approval granted; please change mozilla1.0.1+ to fixed1.0.1 when this is checked into branch.

Checked into branch.

Verified fixed win XP branch build 2002073108, Mac branch build 2002073112 and linux branch build 2002073006, marking verified1.0.1

looks good with recent builds, so verifying on the trunk.