Re: [fpc-devel] zstream, fpc and lazarus
Paul Ishenin schreef: Paul Ishenin пишет: Daniël Mantione пишет: Done, please test if Lazarus works now. Thanks, ok now. Can you merge that to fixes branch. Because now its current state is not useful with lazarus :) Can the zstream fixes be merged to the fixes_2_2 branch? It would fix bug reports 10327, 10323, 10347, 10366, 10410. Vincent ___ fpc-devel maillist - fpc-devel@lists.freepascal.org http://lists.freepascal.org/mailman/listinfo/fpc-devel
Re: [fpc-devel] zstream, fpc and lazarus
Op Sun, 2 Dec 2007, schreef Paul Ishenin: Vincent Snijders wrote: Daniël Mantione schreef: Op Sat, 1 Dec 2007, schreef Vincent Snijders: Daniël Mantione schreef: If committed a proper fix, as bonus you can now randomly seek through the compressed file, there are no limitations anymore. Unfortunately, it still crashes Lazarus: [FORMS.PP] ExceptionOccurred Sender=Ecompressionerror Exception=buffer error A buffer error occurs when the output buffer is full or the input buffer is empty. As the output buffer is set on each read operation it has to be the input buffer. Put a: writeln(Fstream.avail_in); ... at the end of the reset procedure. If it happens to be non-zero, we have found the problem. It does not happen at all. The reset method doesn't seem to be called. I think the problem is still the same. When you seek TDecompressionStream to offset 0 with soFromBegining origin then Source stream must be repositioned too. Ok, read with me. If we seek backwards (which is the case with offset 0 from beginning), we call reset: if origin=sofrombeginning then dec(offset,raw_read); if offset0 then begin inc(offset,raw_read); reset; end; Inside reset, we set the source stream position to 0: procedure Tdecompressionstream.reset; var err:smallint; begin raw_read:=0; compressed_read:=0; source.position:=0; inflateEnd(Fstream); if skipheader then err:=inflateInit2(Fstream,-MAX_WBITS) else err:=inflateInit(Fstream); if errZ_OK then raise Edecompressionerror.create(zerror(err)); end; So, this is not the problem. I can't see any seek instruction inside fpreadpng.pp, so no seeking happens at all. There must be something different going on. Daniël___ fpc-devel maillist - fpc-devel@lists.freepascal.org http://lists.freepascal.org/mailman/listinfo/fpc-devel
Re: [fpc-devel] zstream, fpc and lazarus
Op Sun, 2 Dec 2007, schreef Paul Ishenin: Daniël Mantione ?: Ok, read with me. If we seek backwards (which is the case with offset 0 from beginning), we call reset: if origin=sofrombeginning then dec(offset,raw_read); if offset0 then begin inc(offset,raw_read); reset; end; Inside reset, we set the source stream position to 0: No, Daniel. When we just create TDecompressionStream we seeks to soFromBeginning, 0. We have raw_read = 0, so offset = 0 too after dec(offset, raw_read). And next condition with offset 0 then is false = so no reset call. I fixed that locally after your commit and lazarus works. This seems extremely strange to me. Are you telling that the source stream is not at the start of the Zstream when you create the decompression stream? By the way, I'm getting some doubts about position:=0. This basically seeks the source file to its begin. But what happens if the Zstream does not start at the beginning of the file? Daniël___ fpc-devel maillist - fpc-devel@lists.freepascal.org http://lists.freepascal.org/mailman/listinfo/fpc-devel
Re: [fpc-devel] zstream, fpc and lazarus
Daniël Mantione wrote: Op Sun, 2 Dec 2007, schreef Paul Ishenin: Daniël Mantione ?: Ok, read with me. If we seek backwards (which is the case with offset 0 from beginning), we call reset: if origin=sofrombeginning then dec(offset,raw_read); if offset0 then begin inc(offset,raw_read); reset; end; Inside reset, we set the source stream position to 0: No, Daniel. When we just create TDecompressionStream we seeks to soFromBeginning, 0. We have raw_read = 0, so offset = 0 too after dec(offset, raw_read). And next condition with offset 0 then is false = so no reset call. I fixed that locally after your commit and lazarus works. This seems extremely strange to me. Are you telling that the source stream is not at the start of the Zstream when you create the decompression stream? Exactly. At least when I debugged pngreader it was at the end. By the way, I'm getting some doubts about position:=0. This basically This is according codegear help. We should folow it since many code depends on it. seeks the source file to its begin. But what happens if the Zstream does not start at the beginning of the file? My assumption is that zstream must be at the begining or one should use temporary memory stream to achieve that. Best regards, Paul Ishenin. ___ fpc-devel maillist - fpc-devel@lists.freepascal.org http://lists.freepascal.org/mailman/listinfo/fpc-devel
Re: [fpc-devel] zstream, fpc and lazarus
Op Sun, 2 Dec 2007, schreef Paul Ishenin: This seems extremely strange to me. Are you telling that the source stream is not at the start of the Zstream when you create the decompression stream? Exactly. At least when I debugged pngreader it was at the end. How did it get at the end? By the way, I'm getting some doubts about position:=0. This basically This is according codegear help. We should folow it since many code depends on it. Please copy/paste that help. I can't believe that one would demand that a zstream starts from the start of a file; I know no file format which works this way, not even gzip. seeks the source file to its begin. But what happens if the Zstream does not start at the beginning of the file? My assumption is that zstream must be at the begining or one should use temporary memory stream to achieve that. How does it work in Lazarus? A PNG file does not start with a zstream, it is in the middle of the file. If you seek to the start of a PNG file, you will read PNG chunks rather than a Zstream. Daniël___ fpc-devel maillist - fpc-devel@lists.freepascal.org http://lists.freepascal.org/mailman/listinfo/fpc-devel
Re: [fpc-devel] zstream, fpc and lazarus
Daniël Mantione wrote: Please copy/paste that help. I can't believe that one would demand that a zstream starts from the start of a file; I know no file format which works this way, not even gzip. delphi help: You can call the Seek method with an Offset of zero, and the Origin set to soFromBeginning. This has the effect of resetting the input stream, effectively rewinding the stream back to the beginning. How does it work in Lazarus? A PNG file does not start with a zstream, it is in the middle of the file. If you seek to the start of a PNG file, you will read PNG chunks rather than a Zstream. paste from fcl-image\src\fpreadpng.pp: ZData := TMemoryStream.Create; try EndOfFile := false; while not EndOfFile do begin ReadChunk; HandleChunk; end; Decompress := TDecompressionStream.Create (ZData); try Decompress.position := 0; DoDecompress; finally Decompress.Free; end; So it uses memory stream. It writes data to ZData memory stream, then creates TDecompressionStream. Do you see ZData.Position = 0 here? I dont. Code assume that TDecompressionStream.Create will do that. But TDecompressionStream skips that = we have crash. Best regards, Paul Ishenin. ___ fpc-devel maillist - fpc-devel@lists.freepascal.org http://lists.freepascal.org/mailman/listinfo/fpc-devel
Re: [fpc-devel] zstream, fpc and lazarus
Op Sun, 2 Dec 2007, schreef Paul Ishenin: Daniël Mantione wrote: Please copy/paste that help. I can't believe that one would demand that a zstream starts from the start of a file; I know no file format which works this way, not even gzip. delphi help: You can call the Seek method with an Offset of zero, and the Origin set to soFromBeginning. This has the effect of resetting the input stream, effectively rewinding the stream back to the beginning. How does it work in Lazarus? A PNG file does not start with a zstream, it is in the middle of the file. If you seek to the start of a PNG file, you will read PNG chunks rather than a Zstream. paste from fcl-image\src\fpreadpng.pp: ZData := TMemoryStream.Create; try EndOfFile := false; while not EndOfFile do begin ReadChunk; HandleChunk; end; Decompress := TDecompressionStream.Create (ZData); try Decompress.position := 0; DoDecompress; finally Decompress.Free; end; So it uses memory stream. It writes data to ZData memory stream, then creates TDecompressionStream. Do you see ZData.Position = 0 here? I dont. Code assume that TDecompressionStream.Create will do that. But TDecompressionStream skips that = we have crash. Okay, that explains it. The memory stream is the only solution to seek in a sane way. Now we could switch to Delphi behaviour, but to be honest, I don't want to enforce the memory stream to everyone who wants to use a compressed stream; it should be usable on plain files. I'm going to interpret the Delphi docs liberally; the beginning is the beginning of the zstream, not the beginning of the file. If you want to reset the source stream, you will have to reset the source stream :) Ok, it is a Delphi incompatibility, but really a corner case, and doing so has many advantages. Daniël___ fpc-devel maillist - fpc-devel@lists.freepascal.org http://lists.freepascal.org/mailman/listinfo/fpc-devel
Re: [fpc-devel] zstream, fpc and lazarus
Daniël Mantione wrote: Okay, that explains it. The memory stream is the only solution to seek in a sane way. Now we could switch to Delphi behaviour, but to be honest, I don't want to enforce the memory stream to everyone who wants to use a compressed stream; it should be usable on plain files. I'm going to interpret the Delphi docs liberally; the beginning is the beginning of the zstream, not the beginning of the file. If you want to reset the source stream, you will have to reset the source stream :) Ok, it is a Delphi incompatibility, but really a corner case, and doing so has many advantages. I believe that fpreadpng.pp must be fixed, too. At least it should reset memory stream to zero position itself and don't assume that zstream will do it instead. At maximum everything can be done without memory stream at all - PNG format is designed to be sequential. As for zstream behaviour, I fully agree: it should remember source stream position at creation time and seek to it at reset. This way, compatibility existing with Delphi code won't be broken, because its authors supply streams in which compression data starts from the beginning. Furthermore (and I issued a corresponding patch for the previous version), at destruction decompression stream should adjust source position to the end of actually decompressed data. This way makes it possible to process streams that have compressed data followed by some other data. Regards, Sergei ___ fpc-devel maillist - fpc-devel@lists.freepascal.org http://lists.freepascal.org/mailman/listinfo/fpc-devel
Re: [fpc-devel] zstream, fpc and lazarus
Daniël Mantione wrote: I agree, but I would like to be carefull about one thing: zstreams read from pipes or sockets. If someone closes a socket zstream early, a seek to end would result in an exception. Is there a way we can detect a stream to be seekable? Probably no, and it's (sometimes) a badly needed feature. However, two solutions come to my mind: 1) Ignore that particular exception. Not very good, because excessive data will still be read from the source stream. 2) Add a property, something like MaxSourceSize, and limit decompression stream to read no more than that many bytes from the source. Of course, this size must be known beforehand, but usually sequential streams are arranged in a way that it is known. Or, maybe zlib data header already contains this size? Sergei ___ fpc-devel maillist - fpc-devel@lists.freepascal.org http://lists.freepascal.org/mailman/listinfo/fpc-devel
Re: [fpc-devel] zstream, fpc and lazarus
On Sun, 2 Dec 2007, Daniël Mantione wrote: Op Sun, 2 Dec 2007, schreef Sergei Gorelkin: Daniël Mantione wrote: Okay, that explains it. The memory stream is the only solution to seek in a sane way. Now we could switch to Delphi behaviour, but to be honest, I don't want to enforce the memory stream to everyone who wants to use a compressed stream; it should be usable on plain files. I'm going to interpret the Delphi docs liberally; the beginning is the beginning of the zstream, not the beginning of the file. If you want to reset the source stream, you will have to reset the source stream :) Ok, it is a Delphi incompatibility, but really a corner case, and doing so has many advantages. I believe that fpreadpng.pp must be fixed, too. At least it should reset memory stream to zero position itself and don't assume that zstream will do it instead. At maximum everything can be done without memory stream at all - PNG format is designed to be sequential. As for zstream behaviour, I fully agree: it should remember source stream position at creation time and seek to it at reset. This way, compatibility existing with Delphi code won't be broken, because its authors supply streams in which compression data starts from the beginning. We actually don't need remember the start position, I simply to a backwards seek, which does the trick. Furthermore (and I issued a corresponding patch for the previous version), at destruction decompression stream should adjust source position to the end of actually decompressed data. This way makes it possible to process streams that have compressed data followed by some other data. I agree, but I would like to be carefull about one thing: zstreams read from pipes or sockets. If someone closes a socket zstream early, a seek to end would result in an exception. Is there a way we can detect a stream to be seekable? Not currently. This is in the pipeline (sic). This is one of the things I wanted to address when re-creating zstream. Originally I planned to do this myself, remember ? ;-) The idea is to create TStreamCapability = (scRead,scWrite,scSeekForward,scSeekBackward,scSize); Class Function TStream.StreamCapabilities : TStreamCapabilities; And to check this. It can be used to optimize certain operations. Now, a non-seekable stream without size is assumed in FPC, making some operations slow. Delphi assumes a stream which is seekable, with size, making some operations impossible, like pipes or sockets. I submitted a bugreport about this way back in 2000. Michael.___ fpc-devel maillist - fpc-devel@lists.freepascal.org http://lists.freepascal.org/mailman/listinfo/fpc-devel
Re: [fpc-devel] zstream, fpc and lazarus
Daniël Mantione пишет: Done, please test if Lazarus works now. Thanks, ok now. Best regards, Paul Ishenin. ___ fpc-devel maillist - fpc-devel@lists.freepascal.org http://lists.freepascal.org/mailman/listinfo/fpc-devel
Re: [fpc-devel] zstream, fpc and lazarus
Op Sun, 2 Dec 2007, schreef Sergei Gorelkin: Daniël Mantione wrote: I agree, but I would like to be carefull about one thing: zstreams read from pipes or sockets. If someone closes a socket zstream early, a seek to end would result in an exception. Is there a way we can detect a stream to be seekable? Probably no, and it's (sometimes) a badly needed feature. However, two solutions come to my mind: 1) Ignore that particular exception. Not very good, because excessive data will still be read from the source stream. 2) Add a property, something like MaxSourceSize, and limit decompression stream to read no more than that many bytes from the source. Of course, this size must be known beforehand, but usually sequential streams are arranged in a way that it is known. Or, maybe zlib data header already contains this size? It doesn't, otherwise it could not be sequentially written. As you cannot unread some bytes, this makes the bytes remaining in the buffer problematic. Daniël___ fpc-devel maillist - fpc-devel@lists.freepascal.org http://lists.freepascal.org/mailman/listinfo/fpc-devel
Re: [fpc-devel] zstream, fpc and lazarus
Paul Ishenin пишет: Daniël Mantione пишет: Done, please test if Lazarus works now. Thanks, ok now. Can you merge that to fixes branch. Because now its current state is not useful with lazarus :) Best regards, Paul Ishenin. ___ fpc-devel maillist - fpc-devel@lists.freepascal.org http://lists.freepascal.org/mailman/listinfo/fpc-devel
Re: [fpc-devel] zstream, fpc and lazarus
Op Fri, 30 Nov 2007, schreef Vincent Snijders:
Daniël Mantione schreef:
Op Mon, 26 Nov 2007, schreef Paul Ishenin:
Daniël Mantione wrote:
This patch is not correct because there might still be unread
data in the
zlib data buffers; this patch makes no attempt to flush them.
Daniël
This patch only do repositioning of Source stream to apropriate
position and
this solves crash reason of png reader (which lazarus uses a lot).
All other
logic is out of my current interests.
Yes, but instead of refusing restarts, it would support them in a buggy
way. If you support something, you should support it correctly.
I believe you know things better and do
all other fixes (since you are implementer).
I'll look at it.
In the meantime, can this patch applied with {$IFDEF
ZSTREAM_BUGGY_QUICKFIX}...{$ENDIF}
If committed a proper fix, as bonus you can now randomly seek through the
compressed file, there are no limitations anymore.
Daniël___
fpc-devel maillist - fpc-devel@lists.freepascal.org
http://lists.freepascal.org/mailman/listinfo/fpc-devel
Re: [fpc-devel] zstream, fpc and lazarus
On Sat, 1 Dec 2007, Daniël Mantione wrote:
Op Fri, 30 Nov 2007, schreef Vincent Snijders:
Daniël Mantione schreef:
Op Mon, 26 Nov 2007, schreef Paul Ishenin:
Daniël Mantione wrote:
This patch is not correct because there might still be unread
data in the
zlib data buffers; this patch makes no attempt to flush them.
Daniël
This patch only do repositioning of Source stream to apropriate
position and
this solves crash reason of png reader (which lazarus uses a lot).
All other
logic is out of my current interests.
Yes, but instead of refusing restarts, it would support them in a buggy
way. If you support something, you should support it correctly.
I believe you know things better and do
all other fixes (since you are implementer).
I'll look at it.
In the meantime, can this patch applied with {$IFDEF
ZSTREAM_BUGGY_QUICKFIX}...{$ENDIF}
If committed a proper fix, as bonus you can now randomly seek through the
compressed file, there are no limitations anymore.
Provided the source stream supports seeking, obviously...
Michael.___
fpc-devel maillist - fpc-devel@lists.freepascal.org
http://lists.freepascal.org/mailman/listinfo/fpc-devel
Re: [fpc-devel] zstream, fpc and lazarus
Daniël Mantione schreef:
Op Fri, 30 Nov 2007, schreef Vincent Snijders:
I'll look at it.
In the meantime, can this patch applied with {$IFDEF
ZSTREAM_BUGGY_QUICKFIX}...{$ENDIF}
If committed a proper fix, as bonus you can now randomly seek through the
compressed file, there are no limitations anymore.
Thanks.
Vincent
___
fpc-devel maillist - fpc-devel@lists.freepascal.org
http://lists.freepascal.org/mailman/listinfo/fpc-devel
Re: [fpc-devel] zstream, fpc and lazarus
Daniël Mantione schreef: If committed a proper fix, as bonus you can now randomly seek through the compressed file, there are no limitations anymore. Unfortunately, it still crashes Lazarus: [FORMS.PP] ExceptionOccurred Sender=Ecompressionerror Exception=buffer error Stack trace: $0050802E TDECOMPRESSIONSTREAM__READ, line 299 of src/zstream.pp $00501F9B DECODE, line 755 of src/fpreadpng.pp $00501D74 TFPREADERPNG__DODECOMPRESS, line 771 of src/fpreadpng.pp $004FF53D TFPCUSTOMIMAGEREADER__IMAGEREAD, line 230 of src/fphandler.inc $004FE391 TFPCUSTOMIMAGE__LOADFROMSTREAM, line 37 of src/fpimage.inc $004AD081 TFPIMAGEBITMAP__READSTREAM, line 1926 of graphics.pp $004A4394 TBITMAP__LOADFROMSTREAM, line 459 of ./include/bitmap.inc $004A07F0 LOADBITMAPFROMLAZARUSRESOURCE, line 1509 of graphics.pp $00428C56 CREATEBUTTON, line 1433 of main.pp $004287BB TMAINIDE__SETUPSPEEDBUTTONS, line 1462 of main.pp $00427A27 TMAINIDE__CREATE, line 1119 of main.pp $00402943 main, line 93 of lazarus.pp ___ fpc-devel maillist - fpc-devel@lists.freepascal.org http://lists.freepascal.org/mailman/listinfo/fpc-devel
Re: [fpc-devel] zstream, fpc and lazarus
Op Sat, 1 Dec 2007, schreef Vincent Snijders: Daniël Mantione schreef: If committed a proper fix, as bonus you can now randomly seek through the compressed file, there are no limitations anymore. Unfortunately, it still crashes Lazarus: [FORMS.PP] ExceptionOccurred Sender=Ecompressionerror Exception=buffer error A buffer error occurs when the output buffer is full or the input buffer is empty. As the output buffer is set on each read operation it has to be the input buffer. Put a: writeln(Fstream.avail_in); ... at the end of the reset procedure. If it happens to be non-zero, we have found the problem. Daniël___ fpc-devel maillist - fpc-devel@lists.freepascal.org http://lists.freepascal.org/mailman/listinfo/fpc-devel
Re: [fpc-devel] zstream, fpc and lazarus
Daniël Mantione schreef: Op Sat, 1 Dec 2007, schreef Vincent Snijders: Daniël Mantione schreef: If committed a proper fix, as bonus you can now randomly seek through the compressed file, there are no limitations anymore. Unfortunately, it still crashes Lazarus: [FORMS.PP] ExceptionOccurred Sender=Ecompressionerror Exception=buffer error A buffer error occurs when the output buffer is full or the input buffer is empty. As the output buffer is set on each read operation it has to be the input buffer. Put a: writeln(Fstream.avail_in); ... at the end of the reset procedure. If it happens to be non-zero, we have found the problem. It does not happen at all. The reset method doesn't seem to be called. Vincent ___ fpc-devel maillist - fpc-devel@lists.freepascal.org http://lists.freepascal.org/mailman/listinfo/fpc-devel
Re: [fpc-devel] zstream, fpc and lazarus
Vincent Snijders wrote: Daniël Mantione schreef: Op Sat, 1 Dec 2007, schreef Vincent Snijders: Daniël Mantione schreef: If committed a proper fix, as bonus you can now randomly seek through the compressed file, there are no limitations anymore. Unfortunately, it still crashes Lazarus: [FORMS.PP] ExceptionOccurred Sender=Ecompressionerror Exception=buffer error A buffer error occurs when the output buffer is full or the input buffer is empty. As the output buffer is set on each read operation it has to be the input buffer. Put a: writeln(Fstream.avail_in); ... at the end of the reset procedure. If it happens to be non-zero, we have found the problem. It does not happen at all. The reset method doesn't seem to be called. I think the problem is still the same. When you seek TDecompressionStream to offset 0 with soFromBegining origin then Source stream must be repositioned too. Best regards, Paul Ishenin. ___ fpc-devel maillist - fpc-devel@lists.freepascal.org http://lists.freepascal.org/mailman/listinfo/fpc-devel
Re: [fpc-devel] zstream, fpc and lazarus
Daniël Mantione schreef:
Op Mon, 26 Nov 2007, schreef Paul Ishenin:
Daniël Mantione wrote:
This patch is not correct because there might still be unread data in the
zlib data buffers; this patch makes no attempt to flush them.
Daniël
This patch only do repositioning of Source stream to apropriate position and
this solves crash reason of png reader (which lazarus uses a lot). All other
logic is out of my current interests.
Yes, but instead of refusing restarts, it would support them in a buggy
way. If you support something, you should support it correctly.
I believe you know things better and do
all other fixes (since you are implementer).
I'll look at it.
In the meantime, can this patch applied with {$IFDEF
ZSTREAM_BUGGY_QUICKFIX}...{$ENDIF}
Vincent
___
fpc-devel maillist - fpc-devel@lists.freepascal.org
http://lists.freepascal.org/mailman/listinfo/fpc-devel
Re: [fpc-devel] zstream, fpc and lazarus
Daniël Mantione wrote: This patch is not correct because there might still be unread data in the zlib data buffers; this patch makes no attempt to flush them. Daniël This patch only do repositioning of Source stream to apropriate position and this solves crash reason of png reader (which lazarus uses a lot). All other logic is out of my current interests. I believe you know things better and do all other fixes (since you are implementer). Best regards, Paul Ishenin. ___ fpc-devel maillist - fpc-devel@lists.freepascal.org http://lists.freepascal.org/mailman/listinfo/fpc-devel
Re: [fpc-devel] zstream, fpc and lazarus
Op Mon, 26 Nov 2007, schreef Paul Ishenin: Daniël Mantione wrote: This patch is not correct because there might still be unread data in the zlib data buffers; this patch makes no attempt to flush them. Daniël This patch only do repositioning of Source stream to apropriate position and this solves crash reason of png reader (which lazarus uses a lot). All other logic is out of my current interests. Yes, but instead of refusing restarts, it would support them in a buggy way. If you support something, you should support it correctly. I believe you know things better and do all other fixes (since you are implementer). I'll look at it. Daniël___ fpc-devel maillist - fpc-devel@lists.freepascal.org http://lists.freepascal.org/mailman/listinfo/fpc-devel
Re: [fpc-devel] zstream, fpc and lazarus
Op Mon, 26 Nov 2007, schreef Paul Ishenin: Hello, FPC developers' list. Lazarus crashes with latest fpc (trunk). I found that reason in zstream. TDecompressionStream must reset position of Source stream if Seek is called with Offset = 0 and Origin = soFromBeginning (according to delphi help). Attached patch fixes that little problem (hope without adding another). This patch is not correct because there might still be unread data in the zlib data buffers; this patch makes no attempt to flush them. Daniël___ fpc-devel maillist - fpc-devel@lists.freepascal.org http://lists.freepascal.org/mailman/listinfo/fpc-devel
