Re: [Tigervnc-devel] [Tigervnc-commits] SF.net SVN: tigervnc:[4841] trunk/common/rfb
I've stared at the code yet again and can't see where the corruption was occurring prior to 4841. I understand and agree with you setting the various method arguments to const. That's not the issue. The issue is that I don't understand why some of the code in tightEncode.h was moved around-- particularly why encodeJpegRect16() and encodeJpegRect32() were replaced with encodeJpegRect(). Yes, I see that you're now re-calling ig-getRawPixelsR() in the body of encodeJpeg(), but why? On 2/3/12 9:00 AM, Pierre Ossman wrote: On Tue, 31 Jan 2012 05:03:06 -0600 DRC dcomman...@users.sourceforge.net wrote: I have not seen this, either in high-level or low-level tests. Please explain how to reproduce it. The easiest way to provoke it is with a composited desktop and a solid blue background. I've been using the Gnome 3 fallback on Fedora 16. -- Virtualization Cloud Management Using Capacity Planning Cloud computing makes use of virtualization - but cloud computing also focuses on allowing computing to be delivered as a service. http://www.accelacomm.com/jaw/sfnl/114/51521223/ ___ Tigervnc-devel mailing list Tigervnc-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tigervnc-devel
Re: [Tigervnc-devel] [Tigervnc-commits] SF.net SVN: tigervnc:[4841] trunk/common/rfb
On 2/9/12 6:34 AM, Pierre Ossman wrote:
On Sun, 05 Feb 2012 15:32:52 -0600
DRC dcomman...@users.sourceforge.net wrote:
Perhaps I'm still missing something, because after examining the patch
again, I don't see where the original buffer corruption was occurring or
how your patch fixes it. Part of the difficulty in analyzing this patch
is that you seem to have taken the opportunity to move a lot of things
around to conform to your own personal preferences. I would ask you to
please reformulate the patch with as few changes as possible and to
clarify where the buffer corruption was occurring.
The specific bug I was reproducing was caused by the force solid code
path, which kept pixels pointing at the frame buffer. To be able to
audit the rest of the code for similar misuse, I needed to clear it up
so it was easier to follow. Just the addition of const pointers required
substantial changes. As part of this the JPEG code was changed to make
it painfully obvious it behaves very differently from the other parts
of the tight encoder.
You mean this code?
if (ig-willTransform()) {
ig-translatePixels(pixels, solidColor, 1);
pixels = (PIXEL_T *)solidColor;
}
I don't follow. As you can see, it changes the value of the pixels
pointer so that it points at the local solidColor variable and no
longer at the framebuffer, so the framebuffer is not modified. I can't
see any other place in the code where the framebuffer was being modified.
--
Virtualization Cloud Management Using Capacity Planning
Cloud computing makes use of virtualization - but cloud computing
also focuses on allowing computing to be delivered as a service.
http://www.accelacomm.com/jaw/sfnl/114/51521223/
___
Tigervnc-devel mailing list
Tigervnc-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tigervnc-devel
Re: [Tigervnc-devel] [Tigervnc-commits] SF.net SVN: tigervnc:[4841] trunk/common/rfb
I think it would be interesting to see if there is a relation between this and the issue discussed in bug 3414789... If I can make time next week I'll to look into it. -brian On Fri, Feb 3, 2012 at 10:00 AM, Pierre Ossman oss...@cendio.se wrote: On Tue, 31 Jan 2012 05:03:06 -0600 DRC dcomman...@users.sourceforge.net wrote: I have not seen this, either in high-level or low-level tests. Please explain how to reproduce it. The easiest way to provoke it is with a composited desktop and a solid blue background. I've been using the Gnome 3 fallback on Fedora 16. -- Pierre OssmanOpenSource-based Thin Client Technology System Developer Telephone: +46-13-21 46 00 Cendio ABWeb: http://www.cendio.com A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing? -- Try before you buy = See our experts in action! The most comprehensive online learning library for Microsoft developers is just $99.99! Visual Studio, SharePoint, SQL - plus HTML5, CSS3, MVC3, Metro Style Apps, more. Free future releases when you subscribe now! http://p.sf.net/sfu/learndevnow-dev2 ___ Tigervnc-devel mailing list Tigervnc-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tigervnc-devel -- Try before you buy = See our experts in action! The most comprehensive online learning library for Microsoft developers is just $99.99! Visual Studio, SharePoint, SQL - plus HTML5, CSS3, MVC3, Metro Style Apps, more. Free future releases when you subscribe now! http://p.sf.net/sfu/learndevnow-dev2___ Tigervnc-devel mailing list Tigervnc-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tigervnc-devel
Re: [Tigervnc-devel] [Tigervnc-commits] SF.net SVN: tigervnc:[4841] trunk/common/rfb
On Mon, 30 Jan 2012 15:55:11 -0600 DRC dcomman...@users.sourceforge.net wrote: How does it not work so well? You can't just commit potentially disruptive patches after the code has been stabilized without first opening them up for discussion. I expect a more detailed answer than your commit log below. The tight encoder _writes_ to the pixel buffer given to it, corrupting the screen contents. -- Pierre OssmanOpenSource-based Thin Client Technology System Developer Telephone: +46-13-21 46 00 Cendio ABWeb: http://www.cendio.com A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing? signature.asc Description: PGP signature -- Keep Your Developer Skills Current with LearnDevNow! The most comprehensive online learning library for Microsoft developers is just $99.99! Visual Studio, SharePoint, SQL - plus HTML5, CSS3, MVC3, Metro Style Apps, more. Free future releases when you subscribe now! http://p.sf.net/sfu/learndevnow-d2d___ Tigervnc-devel mailing list Tigervnc-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tigervnc-devel
Re: [Tigervnc-devel] [Tigervnc-commits] SF.net SVN: tigervnc:[4841] trunk/common/rfb
On 1/31/12 2:57 AM, Pierre Ossman wrote: On Mon, 30 Jan 2012 15:55:11 -0600 DRC dcomman...@users.sourceforge.net wrote: How does it not work so well? You can't just commit potentially disruptive patches after the code has been stabilized without first opening them up for discussion. I expect a more detailed answer than your commit log below. The tight encoder _writes_ to the pixel buffer given to it, corrupting the screen contents. I have not seen this, either in high-level or low-level tests. Please explain how to reproduce it. -- Keep Your Developer Skills Current with LearnDevNow! The most comprehensive online learning library for Microsoft developers is just $99.99! Visual Studio, SharePoint, SQL - plus HTML5, CSS3, MVC3, Metro Style Apps, more. Free future releases when you subscribe now! http://p.sf.net/sfu/learndevnow-d2d ___ Tigervnc-devel mailing list Tigervnc-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tigervnc-devel
Re: [Tigervnc-devel] [Tigervnc-commits] SF.net SVN: tigervnc:[4841] trunk/common/rfb
How does it not work so well? You can't just commit potentially
disruptive patches after the code has been stabilized without first
opening them up for discussion.
I expect a more detailed answer than your commit log below.
On 1/30/12 7:58 AM, ossm...@users.sourceforge.net wrote:
Revision: 4841
http://tigervnc.svn.sourceforge.net/tigervnc/?rev=4841view=rev
Author: ossman_
Date: 2012-01-30 13:58:44 + (Mon, 30 Jan 2012)
Log Message:
---
The Tight encoder uses the pixel buffer as a scratch pad, which doesn't
work so well with the new optimisation to feed it the raw frame buffer.
Reorganise and clean up the code to handle this, and make the raw frame
buffer pointer const so that we might avoid such bugs in the future.
Modified Paths:
--
trunk/common/rfb/TightEncoder.cxx
trunk/common/rfb/TightEncoder.h
trunk/common/rfb/TransImageGetter.cxx
trunk/common/rfb/TransImageGetter.h
trunk/common/rfb/tightEncode.h
Modified: trunk/common/rfb/TightEncoder.cxx
===
--- trunk/common/rfb/TightEncoder.cxx 2012-01-30 13:53:11 UTC (rev 4840)
+++ trunk/common/rfb/TightEncoder.cxx 2012-01-30 13:58:44 UTC (rev 4841)
@@ -418,3 +418,19 @@
os-writeBytes(mos.data(), mos.length());
writer-endRect();
}
+
+void TightEncoder::encodeJpegRect(const Rect r, rdr::OutStream *os)
+{
+ const rdr::U8 *buf;
+ int stride;
+
+ buf = ig-getRawPixelsR(r, stride);
+
+ jc.clear();
+ jc.compress(buf, stride * serverpf.bpp / 8, r, serverpf,
+ jpegQuality, jpegSubsampling);
+
+ os-writeU8(0x09 4);
+ os-writeCompactLength(jc.length());
+ os-writeBytes(jc.data(), jc.length());
+}
Modified: trunk/common/rfb/TightEncoder.h
===
--- trunk/common/rfb/TightEncoder.h 2012-01-30 13:53:11 UTC (rev 4840)
+++ trunk/common/rfb/TightEncoder.h 2012-01-30 13:58:44 UTC (rev 4841)
@@ -100,9 +100,9 @@
int paletteInsert(rdr::U32 rgb, int numPixels, int bpp);
void paletteReset(void);
-void fastFillPalette8(rdr::U8 *data, int stride, const Rect r);
-void fastFillPalette16(rdr::U16 *data, int stride, const Rect r);
-void fastFillPalette32(rdr::U32 *data, int stride, const Rect r);
+void fastFillPalette8(const rdr::U8 *buffer, int stride, const Rect r);
+void fastFillPalette16(const rdr::U8 *buffer, int stride, const Rect r);
+void fastFillPalette32(const rdr::U8 *buffer, int stride, const Rect r);
void fillPalette8(rdr::U8 *data, int count);
void fillPalette16(rdr::U16 *data, int count);
@@ -135,10 +135,7 @@
void encodeIndexedRect16(rdr::U16 *buf, const Rect r, rdr::OutStream
*os);
void encodeIndexedRect32(rdr::U32 *buf, const Rect r, rdr::OutStream
*os);
-void encodeJpegRect16(rdr::U16 *buf, int stride, const Rect r,
- rdr::OutStream *os);
-void encodeJpegRect32(rdr::U32 *buf, int stride, const Rect r,
- rdr::OutStream *os);
+void encodeJpegRect(const Rect r, rdr::OutStream *os);
SMsgWriter* writer;
rdr::MemOutStream mos;
Modified: trunk/common/rfb/TransImageGetter.cxx
===
--- trunk/common/rfb/TransImageGetter.cxx 2012-01-30 13:53:11 UTC (rev
4840)
+++ trunk/common/rfb/TransImageGetter.cxx 2012-01-30 13:58:44 UTC (rev
4841)
@@ -56,12 +56,12 @@
PixelTransformer::setColourMapEntries(firstCol, nCols);
}
-rdr::U8 *TransImageGetter::getRawPixelsRW(const Rect r, int *stride)
+const rdr::U8 *TransImageGetter::getRawPixelsR(const Rect r, int *stride)
{
if (!offset.equals(Point(0, 0)))
-return pb-getPixelsRW(r.translate(offset.negate()), stride);
+return pb-getPixelsR(r.translate(offset.negate()), stride);
else
-return pb-getPixelsRW(r, stride);
+return pb-getPixelsR(r, stride);
}
void TransImageGetter::getImage(void* outPtr, const Rect r, int outStride)
Modified: trunk/common/rfb/TransImageGetter.h
===
--- trunk/common/rfb/TransImageGetter.h 2012-01-30 13:53:11 UTC (rev
4840)
+++ trunk/common/rfb/TransImageGetter.h 2012-01-30 13:58:44 UTC (rev
4841)
@@ -72,11 +72,11 @@
// padding will be outStride-r.width() pixels).
void getImage(void* outPtr, const Rect r, int outStride=0);
-// getRawPixelsRW() gets the given rectangle of data directly from the
+// getRawPixelsR() gets the given rectangle of data directly from the
// underlying PixelBuffer, bypassing the translation logic. Only use
// this when doing something that's independent of the client's pixel
// format.
-rdr::U8 *getRawPixelsRW(const Rect r, int *stride);
+const rdr::U8 *getRawPixelsR(const Rect r, int *stride);
//
