Automated merge with ssh://bitbucket.org/lindenlab/viewer-vlc

1 files changed, 44 insertions, 6 deletions

diff --git a/indra/llkdu/llimagej2ckdu.cpp b/indra/llkdu/llimagej2ckdu.cpp
index 9e1dfc7213..0ed6ffefa9 100644
--- a/indra/llkdu/llimagej2ckdu.cpp
+++ b/indra/llkdu/llimagej2ckdu.cpp
@@ -31,6 +31,7 @@
 #include "llpointer.h"
 #include "llmath.h"
 #include "llkdumem.h"
+#include "stringize.h"
 
 #include "kdu_block_coding.h"
 
@@ -81,6 +82,15 @@ std::string report_kdu_exception(kdu_exception mb)
 }
 } // anonymous namespace
 
+// stream kdu_dims to std::ostream
+// Turns out this must NOT be in the anonymous namespace!
+inline
+std::ostream& operator<<(std::ostream& out, const kdu_dims& dims)
+{
+	return out << "(" << dims.pos.x << "," << dims.pos.y << "),"
+				  "[" << dims.size.x << "x" << dims.size.y << "]";
+}
+
 class kdc_flow_control {
 	
 public:
@@ -212,6 +222,12 @@ struct LLKDUMessageError : public LLKDUMessage
 		// terminating handler→flush call."
 		// So throwing an exception here isn't arbitrary: we MUST throw an
 		// exception if we want to recover from a KDU error.
+		// Because this confused me: the above quote specifically refers to
+		// the kdu_error class, which is constructed internally within KDU at
+		// the point where a fatal error is discovered and reported. It is NOT
+		// talking about the kdu_message subclass passed to
+		// kdu_customize_errors(). Destroying this static object at program
+		// shutdown will NOT engage the behavior described above.
 		if (end_of_message) 
 		{
 			LLTHROW(KDUError("LLKDUMessageError::flush()"));
@@ -243,6 +259,10 @@ LLImageJ2CKDU::~LLImageJ2CKDU()
 // Stuff for new simple decode
 void transfer_bytes(kdu_byte *dest, kdu_line_buf &src, int gap, int precision);
 
+// This is called by the real (private) initDecode() (keep_codestream true)
+// and getMetadata() methods (keep_codestream false). As far as nat can tell,
+// mode is always MODE_FAST. It was called by findDiscardLevelsBoundaries()
+// as well, when that still existed, with keep_codestream true and MODE_FAST.
 void LLImageJ2CKDU::setupCodeStream(LLImageJ2C &base, bool keep_codestream, ECodeStreamMode mode)
 {
 	S32 data_size = base.getDataSize();
@@ -253,6 +273,12 @@ void LLImageJ2CKDU::setupCodeStream(LLImageJ2C &base, bool keep_codestream, ECod
 	//
 	mCodeStreamp.reset();
 
+	// It's not clear to nat under what circumstances we would reuse a
+	// pre-existing LLKDUMemSource instance. As of 2016-08-05, it consists of
+	// two U32s and a pointer, so it's not as if it would be a huge overhead
+	// to allocate a new one every time.
+	// Also -- why is base.getData() tested specifically here? If that returns
+	// NULL, shouldn't we bail out of the whole method?
 	if (!mInputp && base.getData())
 	{
 		// The compressed data has been loaded
@@ -309,13 +335,19 @@ void LLImageJ2CKDU::setupCodeStream(LLImageJ2C &base, bool keep_codestream, ECod
 
 	S32 components = mCodeStreamp->get_num_components();
 
-	if (components >= 3)
-	{ // Check that components have consistent dimensions (for PPM file)
-		kdu_dims dims1; mCodeStreamp->get_dims(1,dims1);
-		kdu_dims dims2; mCodeStreamp->get_dims(2,dims2);
-		if ((dims1 != dims) || (dims2 != dims))
+	// Check that components have consistent dimensions (for PPM file)
+	for (int idx = 1; idx < components; ++idx)
+	{
+		kdu_dims other_dims;
+		mCodeStreamp->get_dims(idx, other_dims);
+		if (other_dims != dims)
 		{
-			LL_ERRS() << "Components don't have matching dimensions!" << LL_ENDL;
+			// This method is only called from methods that catch KDUError.
+			// We want to fail the image load, not crash the viewer.
+			throw KDUError(STRINGIZE("Component " << idx << " dimensions "
+									 << other_dims
+									 << " do not match component 0 dimensions "
+									 << dims << "!"));
 		}
 	}
 
@@ -342,6 +374,9 @@ void LLImageJ2CKDU::cleanupCodeStream()
 	mTileIndicesp.reset();
 }
 
+// This is the protected virtual method called by LLImageJ2C::initDecode().
+// However, as far as nat can tell, LLImageJ2C::initDecode() is called only by
+// llimage_libtest.cpp's load_image() function. No detectable production use.
 bool LLImageJ2CKDU::initDecode(LLImageJ2C &base, LLImageRaw &raw_image, int discard_level, int* region)
 {
 	return initDecode(base,raw_image,0.0f,MODE_FAST,0,4,discard_level,region);
@@ -374,6 +409,9 @@ bool LLImageJ2CKDU::initEncode(LLImageJ2C &base, LLImageRaw &raw_image, int bloc
 	return true;
 }
 
+// This is the real (private) initDecode() called both by the protected
+// initDecode() method and by decodeImpl(). As far as nat can tell, only the
+// decodeImpl() usage matters for production.
 bool LLImageJ2CKDU::initDecode(LLImageJ2C &base, LLImageRaw &raw_image, F32 decode_time, ECodeStreamMode mode, S32 first_channel, S32 max_channel_count, int discard_level, int* region)
 {
 	base.resetLastError();