PNGEncoder2 icon indicating copy to clipboard operation
PNGEncoder2 copied to clipboard

Inconsistent state after an unexpected error

Open SmilyOrg opened this issue 14 years ago • 9 comments

If the encoder errors out from any reason (e.g. out of memory), it doesn't get reset at the end, so any subsequent encodes error out with "Only one PNG can be encoded at once". A solution would be the ability to (optionally?) reset the encoder instead of throwing an error.

SmilyOrg avatar Feb 19 '12 15:02 SmilyOrg

Hmm, that's unfortunate. I'll try adding a try-catch around the encoding and see if the speed is impacted at all (probably not). Bugfix ETA: next 12 hours.

cameron314 avatar Feb 19 '12 17:02 cameron314

Ah, there's no rush :)

SmilyOrg avatar Feb 19 '12 17:02 SmilyOrg

Good thing there's no rush, 'cause this is trickier to fix than I thought. There's a bug in the compiler with try-catch in inlined functions. So... I'm not sure what to do for now. Are you using the class from haXe or AS3 (with SWC)?

If you're using AS3, you can manually reset the flag (just make sure there really isn't another image being encoded at the same time):

_PNGEncoder2.PNGEncoder2Impl.encoding = false;

New ETA for proper fix: Several weeks ;-)

cameron314 avatar Feb 19 '12 22:02 cameron314

I'm using it from AS3 with the SWC. Would it be possible to implement an option that would "override" the previous encode when you call a new one, instead of canceling the new one with an error? That is, so it would stop everything and reset the encoder state if you didn't wait for the previous one to finish or if the previous one errored out.

SmilyOrg avatar Feb 19 '12 22:02 SmilyOrg

Would a cancel() method work? It's a lot easier to implement. There really is no internal (static) state beyond the "currently encoding" flag. All the state is in the object instance, which would behave quite weirdly after an error occurred anyway (e.g. trying to continue on the next frame with corrupted data).

cameron314 avatar Feb 19 '12 22:02 cameron314

Sure, but you could just call that at the beginning of encode(), no?

SmilyOrg avatar Feb 19 '12 22:02 SmilyOrg

Nope, since that would be on the encoder object, which I don't have access to in the static function. (I could keep a static reference to the current encoder, but that's a bit yucky).

cameron314 avatar Feb 19 '12 22:02 cameron314

Ah, but don't you already have a static variable that handles the "one encode at a time" thing? Surely that's in the same field of yuckiness? :)

SmilyOrg avatar Feb 19 '12 22:02 SmilyOrg

So it is! :-) But I realized that wouldn't work anyway, since you'd probably get many more exceptions between the time the error occurred and the time it's cancelled, on the next encode.

The real solution is to handle errors properly -- the "encoding" flag isn't the only global state that's corrupted when an exception occurs: the domain memory isn't properly reset to what it was before the encoding, which shouldn't ever happen.

If all you need is a quick bandaid fix, try just setting _PNGEncoder2.PNGEncoder2Impl.encoding = false; directly before encoding. Once the compiler bug is fixed I'll add proper exception handling.

cameron314 avatar Feb 19 '12 23:02 cameron314