ScummVM IRC Log - 01 Jul 2017

[Back to Index]

[00:07] <-- girafe left irc: Quit: Leaving
[00:10] <-- Farmboy0 left irc: Remote host closed the connection
[00:51] <-- Joefish left irc: Ping timeout: 276 seconds
[00:53] <snover> https://ubidestates.hibid.com/catalog/103245/radioshack-auction--1/ radioshacks liquidation auction. lots of &stuff!
[00:53] --> omer_mor joined #scummvm.
[00:55] <-- omer_mor_ left irc: Ping timeout: 260 seconds
[00:57] <snover> random boxes of tandy software and what seems to be a lot of TRS-80s. heres a box with police quest: https://ubidestates.hibid.com/lot/32247992/tandy-computer-software---police-quest-/
[00:58] <snover> a box full of cuecats&
[01:13] --> DominusExult joined #scummvm.
[01:15] <-- Dominus left irc: Ping timeout: 260 seconds
[01:15] Nick change: DominusExult -> Dominus
[01:16] <-- K4T left irc: Read error: Connection reset by peer
[01:33] --> GitHub140 joined #scummvm.
[01:33] <GitHub140> [scummvm] dreammaster pushed 3 new commits to master: https://git.io/vQR2M
[01:33] <GitHub140> scummvm/master a897ad9 Paul Gilbert: VIDEO: Add method to VideoDecoder to erase a track
[01:33] <GitHub140> scummvm/master 2838776 Paul Gilbert: VIDEO: Refactor AVIDecoder for better handling of transparency track...
[01:33] <GitHub140> scummvm/master e4ba3fc Paul Gilbert: TITANIC: Update AVISurface to use refactored AVIDecoder
[01:33] GitHub140 (GitHub140@192.30.252.40) left #scummvm.
[02:08] --> omer_mor_ joined #scummvm.
[02:10] --> exmensa joined #scummvm.
[02:11] <-- omer_mor left irc: Ping timeout: 268 seconds
[02:59] --> GitHub52 joined #scummvm.
[02:59] <GitHub52> [scummvm] dreammaster pushed 1 new commit to master: https://git.io/vQRwD
[02:59] <GitHub52> scummvm/master 28c461e Paul Gilbert: TITANIC: Don't set movie framerate until after movie has started
[02:59] GitHub52 (GitHub52@192.30.252.41) left #scummvm.
[03:00] <-- Cheeseness left irc: Quit: Leaving.
[03:01] --> Cheeseness joined #scummvm.
[03:02] <dreammaster> Sigh. Still can't quite get the Service elevator floor indicator to play in reverse. It may well be the only video in the game played explicitly backwards. Certainly the only one that manually sets the video framerate
[03:03] <dreammaster> I've at least got a quick and dirty avi decoder loop the engine run method that loads the video and plays it backwards correctly, so I know my refactoring of AVIDecoder works. Just not when it goes through the whole movie & avisurface classes
[03:04] <snover> thats frustrating. i was worried about reverse playback in SCI, since the original engine seemed to have some flag to do this, but so far ive gotten lucky and have not encountered a game that tries to do this.
[03:04] <dreammaster> Interestingly enough, seems AVIDecoder doesn't correctly play back a video in reverse even when the decoder setReverse method is called. You have to explicitly set a negative frame rate before it does. Though that seems to be a preexisting bug, and unrelated to the remaining issue I'm having
[03:05] <dreammaster> Lucky you.. not used, so let's forget about it :)
[03:06] <snover> well, in lieu of a game making my life difficult by trying to play video in reverse, i have instead decided to make my own life difficult by shoving linear interpolation upscaling into the engine :P
[03:06] <dreammaster> I guess I'll try some further experiments tomorrow morning when I'm fresh. If I can get this sorted out, I'll be a lot happier about preparing a testing announcement.
[03:06] <snover> does any other engine try to play AVIs in reverse?
[03:07] <dreammaster> Speaking of which, if someone with admin rights to the bug tracker could add in a 'Titanic' engine and 'Starship Titanic' game entry in preparation for the testing announcement, I'd appreciate it.
[03:08] <dreammaster> Huh.. doesn't look like it. titanic\support\avi_surface.cpp is the only caller to setReverse in the entire engine/ hierarchy
[03:08] <snover> look harder; theres a call in sci/graphics/video32.cpp :)
[03:08] <dreammaster> I guess that's why the setReverse issue with AVIDecoer has never been noticed :)
[03:09] <dreammaster> Oh, right, yes that too
[03:09] <dreammaster> Unless one of the other engines is setting a negative frame rate. Internally that will set the reverse flag as well
[03:09] <dreammaster> At least, I think so
[03:15] <dreammaster> Well, that's enough for me for today. I'll leave it to tomorrow
[03:16] <-- dreammaster left irc:
[03:16] <bgK> Mohawk/Myst is a heavy user of variable rate plaback including negative rates. IIRC backwards playback needs to be implemented at the video decoder level
[03:17] <bgK> the QuickTime decoder has some code to decrement the current frame when playing backwards
[03:17] <bgK> the AVI decoder does not seem to have it
[04:15] <-- CuriosTiger left irc: Remote host closed the connection
[04:16] --> CuriosTiger joined #scummvm.
[04:17] <-- Lightkey left irc: Ping timeout: 240 seconds
[04:19] <P2E> fydo, logix - could not get answers, sorry
[04:25] --> omer_mor joined #scummvm.
[04:28] <-- omer_mor_ left irc: Ping timeout: 248 seconds
[04:30] --> Lightkey joined #scummvm.
[04:42] --> ST joined #scummvm.
[04:42] #scummvm: mode change '+o ST' by ChanServ!ChanServ@services.
[04:43] <-- ST left irc: Client Quit
[04:45] <-- ST1 left irc: Ping timeout: 240 seconds
[04:48] <-- ced117 left irc: Ping timeout: 260 seconds
[04:49] --> ced117 joined #scummvm.
[04:49] <-- ced117 left irc: Changing host
[04:49] --> ced117 joined #scummvm.
[04:55] --> ST joined #scummvm.
[04:55] #scummvm: mode change '+o ST' by ChanServ!ChanServ@services.
[05:18] --> omer_mor_ joined #scummvm.
[05:20] <-- omer_mor left irc: Ping timeout: 268 seconds
[05:32] --> omer_mor joined #scummvm.
[05:34] <-- omer_mor_ left irc: Ping timeout: 248 seconds
[05:48] --> ny00123 joined #scummvm.
[05:50] --> Mia joined #scummvm.
[06:09] <-- Cheeseness left irc: Remote host closed the connection
[06:10] --> Cheeseness joined #scummvm.
[06:15] <Simei> Morning
[06:18] --> Henke37 joined #scummvm.
[06:38] --> evil-t0by joined #scummvm.
[06:38] #scummvm: mode change '+o evil-t0by' by ChanServ!ChanServ@services.
[06:51] --> omer_mor_ joined #scummvm.
[06:53] <-- omer_mor left irc: Ping timeout: 276 seconds
[06:54] <Simei> https://www.irccloud.com/pastebin/wnqGxIJp/
[06:55] <Simei> I've encountered a problem when trying to add signature to in front of stream
[06:56] <Simei> stream->read(sourceBufferPtr, streamSize); in the line 15 in this snippet, I set a breakpoint there, it always sets "\0" to sourceBufferPtr, but I have no idea why.
[06:57] <Simei> Does anyone have an idea?
[06:57] <Simei> evil-t0by _sev?
[07:10] <fuzzie_> you mean, after that line, sourceBufferPtr doesn't contain the right content?
[07:12] <fuzzie_> your snippet looks ok at a glance. but I am moving house today, so no time to debug :(
[07:18] <Simei> Yes. it still stays empty after the read
[07:18] <Simei> https://usercontent.irccloud-cdn.com/file/1VPXZVSE/debug_view.png
[07:31] --> Begasus joined #scummvm.
[07:33] --> m_kiewitz joined #scummvm.
[07:33] #scummvm: mode change '+o m_kiewitz' by ChanServ!ChanServ@services.
[07:34] <-- LittleToonCat left irc: Remote host closed the connection
[07:39] --> Begas_VBox joined #scummvm.
[07:42] <bgK> Simei: are all the other bytes in sourceBufferPtr zero?
[07:43] <bgK> if I understand correctly, sourceBufferPtr should contain a headerless PNG image
[07:43] <bgK> which means the first 4 bytes are the size of the first chunk stored in big endian order
[07:44] <bgK> having zero as the first byte seems normal

[07:49] <-- Begas_VBox left irc: Quit: Vision[0.9.8]: i've been blurred!
[07:51] --> Begas_VBox joined #scummvm.
[07:54] --> Joefish joined #scummvm.
[07:54] #scummvm: mode change '+v Joefish' by ChanServ!ChanServ@services.
[07:54] --> waltervn joined #scummvm.
[07:54] #scummvm: mode change '+o waltervn' by ChanServ!ChanServ@services.
[07:54] <waltervn> morning
[07:55] <Simei> bgK: yes, only the first 4 bytes are zeros
[07:56] <Simei> I get "[00][00][00][00]: invalid chunk type!" as error when trying to load it
[07:57] <Simei> I need to modify it to sth else?
[08:01] <bgK> Simei: it's curious all 4 first bytes are zero. What are the 4 next bytes?
[08:02] <bgK> Those should be the PNG chunk type.. which the PNG decoder seems to read as 0
[08:06] <Simei> bgK: I was wrong, it's in fact "\0 \0 \0 \r I H D R \0 \0 \002 \0 \0 ..."
[08:06] <Simei> first three bytes are zeros
[08:06] <bgK> Simei: that seems to be sane PNG data
[08:10] <Simei> bgK: the error occurs here : https://github.com/yinsimei/scummvm/blob/master/image/png.cpp#L125
[08:10] --> Harekiet2 joined #scummvm.
[08:11] <-- Harekiet left irc: Ping timeout: 255 seconds
[08:12] <evil-t0by> Simei: 5 minutes
[08:12] <evil-t0by> Make that 10
[08:13] <evil-t0by> logix: who uses briefs
[08:13] <bgK> Simei: perhaps you can replace your MemoryWriteStream with a Common::DumpFile to see if it produces a valid PNG file
[08:13] <evil-t0by> snover: w o w .
[08:14] <Simei> bgK : Ok, I will try, Thanks!
[08:14] <Simei> evil-t0by, morning
[08:17] <evil-t0by> Hum, looks ok at a glance
[08:17] <evil-t0by> Also, why am I evil
[08:18] Nick change: evil-t0by -> t0by
[08:18] <t0by> Simei: I've just come back from jogging, I'll take a quick shower and help you out here. If you can, please commit your working copy somewhere and tell me which data files you are using to that I can reproduce
[08:18] <-- Harekiet2 left irc: Read error: Connection reset by peer
[08:20] --> Harekiet joined #scummvm.
[08:20] <Simei> ok, thanks, t0by
[08:20] <Simei> t0by, bgK: it gives a valid png file by using Common::DumpFile
[08:21] <Simei> https://usercontent.irccloud-cdn.com/file/X1YdOhAB/image.png
[08:26] <-- Harekiet left irc: Ping timeout: 240 seconds
[08:27] <Simei> bgK, t0by: Ah, no, it doesn't work anymore now. Wired, only work for the first time. Let me check.
[08:28] <m_kiewitz> anyone owns a localized police quest 3 and/or amiga version of it?
[08:30] <-- Begasus left irc: Ping timeout: 255 seconds
[08:33] Action: t0by squints
[08:33] <t0by> that does not sound promising
[08:33] <t0by> Simei, please push and provide datafiles, I'd like to see this with my own eyes
[08:34] <Simei> Ok, a second
[08:36] <Simei> t0by, https://github.com/yinsimei/scummvm/tree/newWIP1-1
[08:36] <t0by> great, and the game is?
[08:36] <Simei> https://github.com/yinsimei/sludge-test-games
[08:36] <Simei> robins-rescue.slg
[08:37] <t0by> Oh, Robin's Rescue.
[08:37] <t0by> (I don't think that one should be in that repo, btw...)
[08:37] <t0by> Let me see
[08:38] <Simei> added now
[08:38] <Simei> it was ignored in fact
[08:38] <Simei> t0by
[08:39] <Simei> still a few seconds to upload it
[08:41] <t0by> Simei, done?
[08:41] <Simei> yes
[08:41] <t0by> Oh
[08:42] <t0by> I can compile it myself, thanks
[08:42] <t0by> Actually, as a rule I would like to discourage you from committing artifacts, it's bad hygiene
[08:43] --> Harekiet joined #scummvm.
[08:43] <Simei> it's in sludge/imgloader.cpp class
[08:43] <t0by> Anyway, let's see if I can reproduce
[08:43] --> Begasus joined #scummvm.
[08:43] <Simei> but, reproduce using sludge may give different md5key
[08:44] <Simei> anyway, I have fall back detection now
[08:44] <Simei> :)
[08:44] <t0by> I wouldn't assume we are chasing a specific compiler bug here
[08:44] <t0by> It could be the case, we'll see
[08:44] <t0by> Let me see...
[08:44] <Simei> the current code can output a valid png file
[08:44] <t0by> Give me a second for the rotten thing to compile
[08:45] <Simei> but it won't work once when the lines concerning writestream uncommented
[08:45] <t0by> Oh, ok
[08:46] <-- Harekiet left irc: Remote host closed the connection
[08:46] <t0by> o_O
[08:46] <t0by> Are you telling me that file->write(sourceBufferPtr, streamSize); produces a *valid* PNG file?
[08:46] <t0by> i.e. with the header and all?
[08:46] <t0by> Yay black magic.
[08:47] <t0by> Well of course.
[08:47] <Simei> line 69, I add the signature
[08:47] <t0by> ^
[08:47] <t0by> By that line of reasoning, I would expect writeStream to be valid as well...
[08:47] <t0by> lemme see
[08:47] <Simei> no
[08:48] <Simei> uncommenting the writestream lines will even make the DumpFile disfunction
[08:48] <Simei> Quite strange
[08:50] <t0by> Hrr
[08:51] <t0by> Sounds like valgrind time
[08:54] <t0by> Hum
[08:54] <t0by> Are you super sure about byte *sourceBufferPtr = (byte *)malloc(streamSize)?
[08:55] <t0by> Uh
[08:55] <t0by> stream->read(sourceBufferPtr, streamSize);
[08:55] <t0by> Am I going completely nuts or you're reading from uninitialized memory?
[08:55] <t0by> (The former is usually the case)
[08:55] <Simei> No, I just copy everything left into it
[08:56] <t0by> Ah right right
[08:56] <t0by> that's the semantics of Stream::read
[08:58] <t0by> o-or is it
[09:00] <t0by> Simei, please look at the implementation of MemoryReadStream::read(void *dataPtr, uint32 dataSize)
[09:00] <t0by> Look at the use of memcpy therein
[09:00] <Simei> Ok
[09:01] <t0by> Q: What's the semantics of memcpy? If so, does read(dataPtr) read *from* or *into* dataPtr? If so, do you ever write *into* copyPtr?
[09:01] --> Strangerke joined #scummvm.
[09:01] #scummvm: mode change '+o Strangerke' by ChanServ!ChanServ@services.
[09:01] <t0by> [All actual questions]
[09:01] <Strangerke> hi guys
[09:01] <t0by> Hi guy
[09:01] <t0by> [I'm trying to figure out what's happening here as well]
[09:02] <Simei> Let me see.
[09:02] <t0by> So, writeStream->write(signature, 8) writes *into* the internal buffer...
[09:03] <Simei> the _ptr is the pointer of copyPtr that I set into in the constructor
[09:04] <t0by> stream->read(sourceBufferPtr, streamSize) reads *into* sourceBufferPtr
[09:04] <t0by> writeStream->write(sourceBufferPtr, streamSize) should write *into* copyPtr
[09:05] <t0by> Which is all good and well, but try to run valgrind on it and be surprised
[09:09] <t0by> Well, at the very least it appears to get past if (stream.readUint32BE() != MKTAG(0x89, 'P', 'N', 'G')) {
[09:09] <Simei> I set break point in eclipse to see the content of copyPtr, it's good
[09:10] <Simei> https://github.com/yinsimei/scummvm/blob/master/image/png.cpp#L125 problem is this line
[09:10] <t0by> Yep
[09:10] <t0by> Simei, you've tried valgrinding it?
[09:11] <Simei> I only use valgrind --leak-check=yes scummvm
[09:11] <t0by> Right
[09:11] <t0by> Oh
[09:11] <Simei> haven't tried more complicated things yet.
[09:11] <t0by> What if the stream reports incorrect length?
[09:12] <t0by> Nope
[09:12] <t0by> Looks okay
[09:13] <t0by> uint32 copySize = streamSize + 8, we then write signature plus streamsize bytes on it...
[09:13] <Simei> I think the key problem is why the output file gives also a [00][00][00][00]: invalid chunk type when writestream uncommented
[09:13] <Simei> they should be independent, no ?
[09:15] <t0by> Let me see, I've commented the file thing first thing
[09:19] <t0by> ==31449== Syscall param write(buf) points to uninitialised byte(s)
[09:20] <t0by> intradesting
[09:21] <Joefish> hey Strangerke
[09:21] <t0by> Simei, no, even with the writestream stuff commented out I get the same results
[09:21] <t0by> You're probably just getting lucky by random chance
[09:21] <Simei> t0by, do you delete the file first ?
[09:22] <Simei> I don't think it's random chance
[09:22] <Joefish> not sure if you read the discussion yesterday on our channel but there seem to be problems with ProtrackerStream. I uploaded a video for comparison on my blog
[09:22] <t0by> reading uninitialized memory is as random chance as it can be, though :)
[09:23] <t0by> Let me see...
[09:23] <t0by> I'm feeling very stupid right now
[09:23] <t0by> Everything *looks* fine...
[09:24] <Simei> Joefish, what was the problem?
[09:25] <Simei> Ah, it's about ProtrackerStream, ok
[09:26] <t0by> Ah wait
[09:27] <t0by> No, I was about to say we need to seek back to 0, but that's nonsense, that's two different Streams
[09:29] <t0by> Aaaaargh!
[09:32] --> WooShell joined #scummvm.
[09:33] <WooShell> good meowning =^.^=
[09:38] --> GitHub185 joined #scummvm.
[09:38] <GitHub185> [scummvm] m-kiewitz pushed 1 new commit to master: https://git.io/vQR9e
[09:38] <GitHub185> scummvm/master a978e2a Martin Kiewitz: SCI: Add script patch for pq3, points for giving locket (bug #9862)...
[09:38] GitHub185 (GitHub185@192.30.252.45) left #scummvm.
[09:38] <m_kiewitz> i hope this commit is correct, first time I'm commiting from my new notebook
[09:41] --> GitHub75 joined #scummvm.
[09:41] <GitHub75> [scummvm] m-kiewitz pushed 1 new commit to master: https://git.io/vQR9m
[09:41] <GitHub75> scummvm/master 4fd313a Martin Kiewitz: SCI: Fix typo in pq3 script patch
[09:41] GitHub75 (GitHub75@192.30.252.40) left #scummvm.
[09:44] Action: t0by ?!?!
[09:45] <t0by> Simei, you know what's interesting?
[09:45] <Joefish> Simei: The problem is that playing the game's music from scummvm sounds quite off like missing reverb and other effects while dumping the stream input directly to protracker it sounds fine. So it seems there some problems with the protracker implementation that we need to look into eventually
[09:49] <m_kiewitz> it seems google hasn't realized that our bug tracker is not hosted on sourceforge anymore :/
[09:50] <Simei> Joefish, ok, i see
[09:50] <Simei> t0by, what's that ?
[09:51] <t0by> Look at the comment inside uint32 MemoryReadStream::read
[09:51] <t0by> could be (sigh) the cause for uninitialized memory?
[09:52] <t0by> Oh, another thing
[09:52] <t0by> I see the stream being passed to loadPNGImage is... bigDataFile
[09:53] <t0by> That sounds scary
[09:53] <t0by> I wonder if the PNG decoder knows when to stop
[09:54] <Simei> yes, it knows
[09:54] <Simei> that's what happens with all other pngs with signatures
[09:54] --> GitHub192 joined #scummvm.
[09:54] <GitHub192> [scummvm] m-kiewitz pushed 1 new commit to master: https://git.io/vQR9M
[09:54] <GitHub192> scummvm/master 5d1b527 Martin Kiewitz: NEWS: Add Police Quest 3 script patch
[09:54] GitHub192 (GitHub192@192.30.252.40) left #scummvm.
[09:54] <t0by> It does, you say.
[09:55] <t0by> Have looked at the size of your png_tmp file? :)
[09:55] <t0by> a 14 MB 512x512 PNG file?
[09:55] <Simei> That's how i write into it
[09:55] <Simei> the rest of the data
[09:56] <t0by> Yes, I mean... are we super sure the png decoder knows when it's time to stop?
[09:56] <Simei> but normally in the png decoder
[09:56] <Simei> it stops earilier
[09:56] <t0by> (And do we want to copy huge data files in memory? But that's another issue)
[09:56] <t0by> mh
[09:56] <Simei> but we don't have length of the png
[09:57] <wjp> what are you looking at?
[09:58] <t0by> In general, this make me like the idea of a "decorated" SeekableReadStream more
[09:58] <Simei> wjp, we are trying to add signature for headless png file
[09:59] <t0by> i.e. one that will give you the header if pos <= 8, otherwise read from the actual stream - 8
[09:59] <Simei> https://github.com/yinsimei/scummvm/blob/newWIP1-1/engines/sludge/imgloader.cpp#L78
[09:59] <t0by> But I can't understand where this is going wrong, however suboptimal it can be
[10:00] <t0by> I have the irrational feeling that's something with seek/rewinding, but I double-checked and it should be an issue nowhere
[10:00] <Simei> wjp, the DumpFile works but WritesStream doesn't
[10:01] <wjp> how does it fail?
[10:02] <wjp> using a MemoryWriteStream here doesn't really gain you anything though
[10:03] <wjp> I'd just do memcpy(copyPtr, signature, 8); readStream->read(copyPtr + 8, streamSize);
[10:03] <t0by> wjp, I've tried memcpy'ing to the bare byte*, no difference
[10:03] <t0by> let me sprunge
[10:04] <wjp> have you checked the return value from the readStream->read ?
[10:05] <wjp> um
[10:05] <wjp> stream->read
[10:05] <t0by> Damn my eyes
[10:06] <t0by> That's what 3 months of Scala does to your brain
[10:06] <t0by> I guess something in the corner of my mind silently assumed that it would raise an exception in case something went wrong
[10:08] <t0by> Bingo
[10:08] <t0by> wjp, good eye
[10:08] <t0by> http://sprunge.us/hhfY
[10:09] <t0by> > <t0by> Look at the comment inside uint32 MemoryReadStream::read
[10:09] <wjp> talking to yourself? ;-)
[10:09] <t0by> Actually I did think about it. I just forgot about it quickly.
[10:09] <wjp> so ImgLoader::loadImage is interesting
[10:10] <wjp> it seems to indicate you don't want to read the whole stream anyway
[10:10] <t0by> Is it?
[10:10] <t0by> Correct, we need to start at the right offset, ie start_ptr
[10:10] <wjp> well
[10:10] <wjp> that's just the current pos
[10:11] <wjp> Simei: still here?
[10:11] <Simei> Yes
[10:11] <t0by> (Under the precondition that current pos is indeed the right one, but I would be liberal in assuming that)
[10:12] <wjp> how is this code failing? (Which symptoms)
[10:12] <Simei> [00][00][00][00]: invalid chunk type!
[10:13] <Simei> it passes signature check and gives this
[10:13] <t0by> Simei, by the way this might be a completely useless exercise. If we can't know where the PNG ends, we'd best resort to the decorator approach, as copying the *whole* data file in memory every time you have to read a sprite is... eewwwwwwwwww.
[10:14] <t0by> Sorry about not thinking about that, I would have warned you off instead of encouraging it.
[10:15] <wjp> the frustrating thing is that PNGDecoder::loadStream doesn't even _use_ the seeking
[10:15] <t0by> wjp, valgrind shows that from png_read_info(pngPtr, infoPtr) on it reads from uninitialized memory.
[10:15] <t0by> Which is... funny.
[10:15] <wjp> (it just takes a SeekableReadStream since that's the ImageDecoder interface)
[10:15] <t0by> in png.cpp+125
[10:16] <Simei> https://github.com/yinsimei/scummvm/blob/master/image/png.cpp#L125
[10:16] <Simei> it's not L125 anymore in the most recent code
[10:17] <t0by> Oh
[10:17] <t0by> Sounds like a good time to rebase against master...
[10:19] <t0by> They added pngFlushStream and WriteToStream, doesn't seem super relevant anyway
[10:19] <t0by> (Thanks snover)
[10:20] <t0by> D'OH
[10:20] Last message repeated 1 time(s).
[10:20] <t0by> Simei, D'OH
[10:20] <Simei> ?
[10:20] <t0by> stream-size() is the size of the WHOLE stream
[10:21] <t0by> but we are already midway throughy it
[10:21] <Simei> I changed it
[10:21] <Simei> but it still fails
[10:21] <wjp> unlikely to be relevant
[10:21] <wjp> (except for efficiency)
[10:21] <t0by> Well, that's why stream->read() "fails"
[10:22] <t0by> we are asking it to read more bytes than are left
[10:22] <wjp> sure
[10:22] <t0by> tbh my money is on some libpng weirdness
[10:23] <t0by> size() - pos() should fix that
[10:23] <Simei> I get rid of the WriteStream, the DumpFile doesn't work neither :/
[10:23] <wjp> t0by: the libpng parts don't know the size of the stream and can't detect if it reached the end, so shouldn't be it
[10:24] <Simei> t0by, I did that, didn't make any change size() - pos()
[10:24] <wjp> have you verified there's actually a valid png here?
[10:24] <Simei> Yes
[10:24] <t0by> s/libpng weirdness/libpng weirdness *or* invalid png/
[10:25] <wjp> because you're kind of staring at something that can't really go wrong instead of the bigger picture
[10:25] <t0by> Simei, have you tried writing a 3-line C program to see if you can read the file you've dumped?
[10:25] <Simei> In the commit with WriteStream and DumpFile, it can produce a valid one
[10:25] <t0by> Simei, how do you know it's "valid", if I may?
[10:25] <wjp> Simei: which commit exactly?
[10:25] <Simei> https://usercontent.irccloud-cdn.com/file/k30TL6p4/image.png
[10:26] <Simei> https://github.com/yinsimei/scummvm/blob/newWIP1-1/engines/sludge/imgloader.cpp
[10:26] <t0by> you have no idea how much invalid and broken crap eog can display :)
[10:26] <Simei> just this one
[10:26] <t0by> (Does it use libpng? Let me see)
[10:26] <Simei> Non, it out put the stream
[10:26] <t0by> (It does)
[10:27] <t0by> (eog that is)
[10:28] <t0by> http://sprunge.us/LFXW
[10:28] <t0by> No, I think the memory bits are solid.
[10:30] <t0by> Of course are we really sure that we are not in the *middle* of a valid PNG, for example a multi-frame one, right?
[10:30] <t0by> (I have no idea, but could that be the case?)
[10:30] <t0by> Is it at all possible that libpng reads a control character that makes it seek *back* to read some header?
[10:31] <t0by> Let me see how animated PNGs are structured
[10:31] <t0by> https://en.wikipedia.org/wiki/APNG#Technical_details
[10:32] <t0by> https://en.wikipedia.org/wiki/File:Apng_assembling.svg
[10:33] <t0by> The "lack of a signature" makes me suspect we're in the middle of an animated PNG...
[10:34] <wjp> Simei: sludger.cpp line 549 should be delete[] dataFolder instead of delete dataFolder
[10:34] <Simei> Ok
[10:34] <wjp> t0by: our libpng interface doesn't even _allow_ seeking
[10:34] <t0by> wjp, not *our*, libpng itself.
[10:34] <t0by> Here's the hypotesis I have in my head
[10:35] <t0by> Look at https://en.wikipedia.org/wiki/File:Apng_assembling.svg
[10:35] <Simei> wjp, t0by: I add a few more break point and it seems that the first png in the sprite bank is well readed but the second failed
[10:35] <t0by> Simei: are you super positive that's not an animated PNG?
[10:35] <Simei> There may be an issue about the input stream
[10:35] <t0by> i.e. https://en.wikipedia.org/wiki/File:Apng_assembling.svg
[10:35] <Simei> Yes
[10:36] <Simei> That sprite bank
[10:36] <t0by> How much positive?
[10:36] <Simei> It's a .duc file
[10:36] <wjp> Simei: the problem is probably that the stream pointer gets left in the wrong position after this function
[10:36] <Simei> that means a series of sprites
[10:37] <Simei> True, so the problem is probably outside
[10:37] <t0by> Rats
[10:37] <wjp> Simei: if you check, you'll see that the first image is actually loaded fine, but the next call breaks
[10:37] <t0by> That's a side effect of reading the whole giant file, I guess
[10:38] <wjp> I'd consider adding an option to scummvm's png decoder of skipping the signature
[10:38] <t0by> wjp: thanks, that's very reasonable
[10:39] <wjp> the other options will be ... hackish
[10:39] <Simei> Ok, that's what I did in the last commit
[10:39] <t0by> At this point, I concur.
[10:39] <t0by> Too much of a hassle for reading a png.
[10:39] <Simei> So i revert and keep that one ?
[10:39] <Simei> and push ?
[10:40] <t0by> Just add a bool skipSignature parameter to loadStream
[10:40] <wjp> no
[10:40] <wjp> don't do that
[10:40] <t0by> Why not?
[10:40] <wjp> loadStream is from the ImageDecoder api
[10:40] <t0by> ...a template?
[10:40] <t0by> Arf
[10:40] <t0by> Right
[10:41] <Simei> I did it in a very dirty way by copying pasting the code
[10:41] <Simei> https://github.com/yinsimei/scummvm/blob/newWIP1-1/image/png.cpp
[10:41] <t0by> I'm not sure which one is less ugly between a flag and a method to set it and an extra method for this soule purpose
[10:42] <Simei> So, what should I do ?
[10:42] <wjp> Simei: first test if it works :-)
[10:42] <Simei> yes, it works
[10:42] <wjp> excellent
[10:42] <Simei> based on that, I tried with the streams
[10:43] <Simei> https://github.com/yinsimei/scummvm/blob/newWIP1-1/image/png.cpp#L235
[10:44] <Simei> The function is in this line precisely
[10:44] <wjp> I'd _probably_ use a png.setSkipSignature() function to set a _skipSignature flag in Image::PNGDecoder
[10:46] <wjp> (that at least somewhat matches how the other ImageDecoders handle semi-related things)
[10:46] <t0by> Of course this means you should remember to unset it when you don't need it...
[10:47] <wjp> ?
[10:47] <Simei> it's fine, in my code, i didn't use pointer
[10:47] <t0by> (Or make it one-off)
[10:47] <t0by> wjp, well, if you leave it set and try to read a PNG *with* the signature, it might not work as expected.
[10:47] <wjp> the PNGDecoder instance is local to this function
[10:48] <t0by> Obviously
[10:48] <wjp> but in general, sure
[10:48] <t0by> Were it a global/singleton or something, it would be unacceptable.
[10:48] <Simei> so I add the flag and the one-off ?
[10:48] <t0by> Mh, how about setting it as an option in the constructor?
[10:49] <t0by> PNGDecoder::PNGDecoder(bool skipSignature=false) ?
[10:50] <wjp> IMO it's way too special-purpose to justify that
[10:51] <wjp> I'd just make it a flag. Maybe make it setSkipSignature(bool) so you can turn it off again as t0by suggested
[10:51] <wjp> (This is for example also how IFFDecoder handles _numRelevantPlanes or JPEGDecoder with _colorSpace)
[10:52] <Simei> Ok, I see.
[10:53] <t0by> https://www.youtube.com/watch?v=Xl12Sp1KiEk
[10:55] Nick change: Begas_VBox -> Begas_bbl
[10:58] <t0by> wjp: thanks for assistance
[10:59] <t0by> In retrospect, it was trivially obvious that reading the whole huge file into a buffer would probably leave its pos in a weird place.
[10:59] <t0by> I could probably have kept not spotting it for two more hours.
[11:00] <wjp> you're welcome
[11:01] <t0by> (Too much FP is beginning to make me forgetful about context)
[11:02] <Simei> Thanks a lot. t0by, wjp. it's fine now :)
[11:02] <Simei> Still some other bugs to fix for the game though
[11:26] <wjp> Simei: changes to image/png.* look good. It would be better to put changes to files outside of engines/sludge in a separate commit though, since those commits will need extra review later
[11:26] <wjp> (with an "IMAGE: " prefix to the commit mesage)
[11:27] <Simei> Ah, true. I'll separate it.
[11:29] <-- WooShell left irc: Ping timeout: 255 seconds
[11:32] <wjp> could you also add a comment to setSkipSignature in png.h explaining its purpose?
[11:34] --> Harekiet joined #scummvm.
[11:35] <Simei> Ok, I'll do that
[11:36] <wjp> thanks
[11:37] <-- Henke37 left irc: Quit: ERR_SHUTDOWN
[11:43] --> Strangerke_ joined #scummvm.
[11:45] <-- Strangerke left irc: Ping timeout: 240 seconds
[11:45] Nick change: Strangerke_ -> Strangerke
[11:52] --> Farmboy0 joined #scummvm.
[11:52] <-- Farmboy0 left irc: Changing host
[11:52] --> Farmboy0 joined #scummvm.
[12:23] --> WooShell joined #scummvm.
[12:33] --> omer_mor joined #scummvm.
[12:35] <-- omer_mor_ left irc: Ping timeout: 248 seconds
[12:37] <-- t0by left irc: Quit: Leaving
[12:55] <-- Begasus left irc: Ping timeout: 255 seconds
[13:06] --> frankyboy_ joined #scummvm.
[13:07] --> Begasus joined #scummvm.
[13:16] --> omer_mor_ joined #scummvm.
[13:18] <-- omer_mor left irc: Ping timeout: 240 seconds
[13:22] --> girafe joined #scummvm.
[13:50] <-- girafe left irc: Read error: Connection reset by peer
[13:51] --> dreammaster joined #scummvm.
[13:51] #scummvm: mode change '+o dreammaster' by ChanServ!ChanServ@services.
[13:52] <dreammaster> Morning
[13:53] <dreammaster> bgK: The AVI decoder has the reverse code in handleNextPacket. When in reverse mode, it calls seekIntern to seek to each prior frame in turn
[13:55] <dreammaster> The problem I'm still having seems to be an issue of timing. I'm going to have to delve into the video decoder, likely getTimeToNextFrame, and see if I can figure out what's being incorrectly set
[14:02] --> girafe joined #scummvm.
[14:04] <-- girafe left irc: Read error: Connection reset by peer
[14:08] <bgK> oh, I'm on a branch where you have not yet written that code
[14:08] Action: bgK sympathizes with dealing with VideoDecoder
[14:10] <dreammaster> Ah. No worries :)
[14:32] <dreammaster> Ironically, the transparency video track for the service elevator indictaor only has a single frame which is used across all the frames of the main video track. So not much seeking needed :)
[14:36] --> ignalina joined #scummvm.
[14:40] --> GitHub13 joined #scummvm.
[14:40] <GitHub13> [scummvm-web] rootfather pushed 1 new commit to master: https://git.io/vQRpF
[14:40] <GitHub13> scummvm-web/master c4ab61d rootfather: WEB: Translate Hi-Res Adventures news to German
[14:40] GitHub13 (GitHub13@192.30.252.42) left #scummvm.
[15:11] <logix> dreammaster: just curious, are you looking for/interested in a second pair of eyes for the math stuff in titanic/star_control?
[15:12] <dreammaster> Certainly. At this point, I've reached my own limit on how much I can sensibly rename in that folder. So extra cleanups/renamings would be welcome
[15:14] <logix> ok, cool
[15:14] <-- ignalina left irc: Quit: ignalina
[15:19] <logix> I just had a quick look the other day and thought I recognized what a few unnamed things actually meant (on a higher level), but I haven't looked at anything in detail yet
[15:20] <logix> anyway, I'll do that then
[16:00] <-- dreammaster left irc:
[16:14] --> LittleToonCat joined #scummvm.
[16:23] <-- Farmboy0 left irc: Remote host closed the connection
[16:23] <-- ny00123 left irc: Ping timeout: 255 seconds
[16:23] --> ny00123 joined #scummvm.
[16:46] <-- Strangerke left irc: Ping timeout: 248 seconds
[16:55] --> ignalina joined #scummvm.
[16:59] --> Littleboy joined #scummvm.
[16:59] #scummvm: mode change '+o Littleboy' by ChanServ!ChanServ@services.
[17:02] <-- Asterisk left irc: Ping timeout: 255 seconds
[17:04] --> Asterisk joined #scummvm.
[17:09] <-- Asterisk left irc: Ping timeout: 255 seconds
[17:15] --> Asterisk joined #scummvm.
[17:16] --> omer_mor joined #scummvm.
[17:19] <-- omer_mor_ left irc: Ping timeout: 260 seconds
[17:28] --> girafe joined #scummvm.
[17:40] --> Joefish_ joined #scummvm.
[17:40] #scummvm: mode change '+v Joefish_' by ChanServ!ChanServ@services.
[17:40] <-- Joefish left irc: Ping timeout: 246 seconds
[17:43] <-- girafe left irc: Read error: Connection reset by peer
[18:11] <-- Begasus left irc: Ping timeout: 255 seconds
[18:16] --> Henke37 joined #scummvm.
[18:23] --> Begasus joined #scummvm.
[18:51] --> dreammaster joined #scummvm.
[18:51] #scummvm: mode change '+o dreammaster' by ChanServ!ChanServ@services.
[19:00] --> girafe joined #scummvm.
[19:07] <-- girafe left irc: Quit: Leaving
[19:08] --> girafe joined #scummvm.
[19:50] --> madgoat joined #scummvm.
[19:52] madgoat (GK.1WM.SU@192.240.212.2) left #scummvm.
[20:57] <-- ignalina left irc: Quit: ignalina
[20:58] --> girafe2 joined #scummvm.
[21:02] <-- girafe left irc: Ping timeout: 276 seconds

[21:36] <dreammaster> Okay. It's official. I despise the VideoDecoder. Despite my earlier plans to have this remaining bug with reverse playback fixed quickly, I've ended up spending all afternoon wrestling with trying to figure out what was going on.
[21:37] <dreammaster> It turns out that when I initially set the movie frame of 97 on the elevator indicator to statically show it's at the top floor, it meant that when the animation started for going down, from frame 97 to 0, it thought that it didn't need to do a frame seek.
[21:37] <dreammaster> That turned out to be the problem. For reverse playback to work, after I called the _decoder->start() method, I explicitly needed to seek to frame 97, even though I was technically already on it. Then everything miraculously starts working :P
[21:37] <snover> dreammaster: ugh, that really does sound very frustrating.
[21:38] <snover> can that method be fixed/documented so that nobody runs into the same problem in the future?
[21:38] <dreammaster> Tell me about it. All these _startTme, and _lastPauseTime, and figuring out _startTime adjustments. Even now, I have no idea why it's working. At this point, I'm just happy to go with the flow that it does
[21:39] <dreammaster> Well, I'm a bit wary of doing so, with only AVIDecoder known about. After all, I needed to patch in some missing reverse playback code to begin with, so it's entirely possible it's a problem specific to AVIDecoder reverse playback rather than for all video decoders as a whole
[21:41] --> GitHub179 joined #scummvm.
[21:41] <GitHub179> [scummvm] dreammaster pushed 2 new commits to master: https://git.io/vQ0ZH
[21:41] <GitHub179> scummvm/master 372610a Paul Gilbert: TITANIC: Refactor out _isReversed from AVI Surface
[21:41] <GitHub179> scummvm/master 7242365 Paul Gilbert: TITANIC: Fix service lift indicator when going down floors
[21:41] GitHub179 (GitHub179@192.30.252.40) left #scummvm.
[21:45] <-- ny00123 left irc: Quit: Leaving
[21:48] --> GitHub136 joined #scummvm.
[21:48] <GitHub136> [scummvm] dreammaster pushed 1 new commit to master: https://git.io/vQ0nU
[21:48] <GitHub136> scummvm/master feef98d Paul Gilbert: TITANIC: Fix crash going up service elevator in prologue
[21:48] GitHub136 (GitHub136@192.30.252.34) left #scummvm.
[21:54] --> GitHub124 joined #scummvm.
[21:54] <GitHub124> [scummvm] dreammaster pushed 1 new commit to master: https://git.io/vQ0nZ
[21:54] <GitHub124> scummvm/master 4ca01a6 Paul Gilbert: TITANIC: Reset movie framerate when setting up decompressor
[21:54] GitHub124 (GitHub124@192.30.252.45) left #scummvm.
[21:55] --> ST1 joined #scummvm.
[21:55] <-- ST left irc: Disconnected by services
[21:58] --> GitHub147 joined #scummvm.
[21:58] <GitHub147> [scummvm] dreammaster pushed 1 new commit to master: https://git.io/vQ0n0
[21:58] <GitHub147> scummvm/master 58e6033 Paul Gilbert: TITANIC: Flag English version for testing, German version as unstable
[21:58] GitHub147 (GitHub147@192.30.252.42) left #scummvm.
[22:13] <-- waltervn left irc: Quit: Leaving
[22:40] <-- girafe2 left irc: Read error: Connection reset by peer
[23:09] --> ignalina joined #scummvm.
[23:13] <snover> dreammaster: 🎉
[23:14] <dreammaster> Thanks :)
[23:15] <snover> i promised _sev to fix my own bugs for the 1.10 release, but as soon as i am done with that, i am going to start playing!
[23:16] <dreammaster> I just need the engine and game to be added to the bug tracker; I'll whip up an announcement tomorrow once the daily downloads have updated.
[23:33] <-- Begas_bbl left irc: Quit: Vision[0.9.8]: i've been blurred!
[23:33] <-- Begasus left irc: Quit: Ex-Chat
[23:39] <-- WooShell left irc: Quit: If you understand or if you don't, if you believe or if you doubt - There's a universal justice, and the eyes of truth are always watching you.
[23:48] --> omer_mor_ joined #scummvm.
[23:51] <-- omer_mor left irc: Ping timeout: 276 seconds
[23:53] <-- m_kiewitz left irc: Quit: technology isn't intrinsically good or evil. It's how it's used. Like the Death Ray.
[23:57] <-- ignalina left irc: Quit: ignalina
[00:00] --- Sun Jul 2 2017