From c36459e1d22639375a44338de3bdf908bfa79d06 Mon Sep 17 00:00:00 2001 From: steve Date: Wed, 8 May 2019 06:40:13 +0100 Subject: [PATCH] o recorder/playback - fixed bug caused by interaction between basictv and digesttv - some roms (games_that_do_bad_things_to_hmove) failed playback when the screen limits changed - the best solution I could find was to prevent basictv calling registered renderers once the last possible scanline was reached - in previous versions, the last scanline was redrawn over and over until a new frame was ready - this however, caused a hashing issue in digesttv when playing back recording input. it is unclear to me why this hashing issue arose. however, the solution is a more natural response to end of frame conditions as well as a being a fix for this bug o regression - regression top level functions now output to an io.Writer passed as an argument, rather than to Stdout directly - playback will now save an image of the current frame (via imagetv) in the event of playback failure o television - NewFrame() and NewScanline() implementations both require a frameNum and scanlineNum argument --- gopher2600.go | 4 +- gui/sdl/sdl.go | 4 +- recorder/playback.go | 36 +++++++++++++++-- recorder/recorder.go | 2 +- regression/database.go | 6 ++- regression/regression.go | 7 +++- television/basic.go | 69 ++++++++++++++++++++------------ television/renderers/digesttv.go | 67 +++++++++++++++++++++++++------ television/renderers/imagetv.go | 44 ++++++++++++-------- television/television.go | 4 +- 10 files changed, 174 insertions(+), 69 deletions(-) diff --git a/gopher2600.go b/gopher2600.go index d303f945..599c3905 100644 --- a/gopher2600.go +++ b/gopher2600.go @@ -285,7 +285,7 @@ func main() { os.Exit(2) } if ok { - fmt.Printf("! deleted %s from regression database\n", path.Base(modeFlags.Arg(0))) + fmt.Printf("! deleted test #%s from regression database\n", path.Base(modeFlags.Arg(0))) } default: fmt.Printf("* only one entry can be deleted at at time when using %s/%s \n", mode, subMode) @@ -315,7 +315,7 @@ func main() { NumFrames: *numFrames} } - err := regression.RegressAdd(rec) + err := regression.RegressAdd(os.Stdout, rec) if err != nil { fmt.Printf("* error adding regression test: %s\n", err) os.Exit(2) diff --git a/gui/sdl/sdl.go b/gui/sdl/sdl.go index 907afdf6..aeb8e256 100644 --- a/gui/sdl/sdl.go +++ b/gui/sdl/sdl.go @@ -114,7 +114,7 @@ func (gtv *GUI) setDebugging(allow bool) { } // NewFrame implements television.Renderer interface -func (gtv *GUI) NewFrame() error { +func (gtv *GUI) NewFrame(frameNum int) error { defer gtv.scr.clearPixels() err := gtv.scr.stb.checkStableFrame() if err != nil { @@ -124,7 +124,7 @@ func (gtv *GUI) NewFrame() error { } // NewScanline implements television.Renderer interface -func (gtv *GUI) NewScanline() error { +func (gtv *GUI) NewScanline(scanline int) error { return nil } diff --git a/recorder/playback.go b/recorder/playback.go index 6d4c5f9a..b9972675 100644 --- a/recorder/playback.go +++ b/recorder/playback.go @@ -41,9 +41,23 @@ type Playback struct { sequences map[string]*playbackSequence vcs *hardware.VCS digest *renderers.DigestTV + + // image tv will produce an image if playback crashes + image *renderers.ImageTV + + // the last frame where an event occurs + endFrame int } -// NewPlayback is hte preferred method of implementation for the Playback type +func (plb Playback) String() string { + currFrame, err := plb.digest.GetState(television.ReqFramenum) + if err != nil { + currFrame = plb.endFrame + } + return fmt.Sprintf("%d/%d (%.1f%%)", currFrame, plb.endFrame, 100*(float64(currFrame)/float64(plb.endFrame))) +} + +// NewPlayback is the preferred method of implementation for the Playback type func NewPlayback(transcript string) (*Playback, error) { var err error @@ -73,7 +87,7 @@ func NewPlayback(transcript string) (*Playback, error) { } // loop through transcript and divide events according to the first field - // (the ID) + // (the peripheral ID) for i := numHeaderLines; i < len(lines)-1; i++ { toks := strings.Split(lines[i], fieldSep) @@ -106,6 +120,10 @@ func NewPlayback(transcript string) (*Playback, error) { return nil, errors.NewFormattedError(errors.PlaybackError, msg) } + // assuming that frames are listed in order in the file. update + // endFrame with the most recent frame every time + plb.endFrame = event.frame + event.scanline, err = strconv.Atoi(toks[fieldScanline]) if err != nil { msg := fmt.Sprintf("%s line %d, col %d", err, i+1, len(strings.Join(toks[:fieldScanline+1], fieldSep))) @@ -142,19 +160,26 @@ func (plb *Playback) AttachToVCS(vcs *hardware.VCS) error { return errors.NewFormattedError(errors.PlaybackError, "current TV type does not match that in the recording") } + var err error + // create digesttv, piggybacking on the tv already being used by vcs; // unless that tv is already a digesttv switch tv := plb.vcs.TV.(type) { case *renderers.DigestTV: plb.digest = tv default: - var err error plb.digest, err = renderers.NewDigestTV(plb.vcs.TV.GetSpec().ID, plb.vcs.TV) if err != nil { return errors.NewFormattedError(errors.RecordingError, err) } } + // image tv will produce an image if playback crashes + plb.image, err = renderers.NewImageTV(plb.vcs.TV.GetSpec().ID, plb.vcs.TV) + if err != nil { + return errors.NewFormattedError(errors.RecordingError, err) + } + // attach playback to controllers vcs.Ports.Player0.Attach(plb) vcs.Ports.Player1.Attach(plb) @@ -190,10 +215,13 @@ func (plb *Playback) GetInput(id string) (peripherals.Event, error) { return peripherals.NoEvent, errors.NewFormattedError(errors.PlaybackError, err) } - // compare current state with the state in the transcript + // compare current state with the recording nextEvent := seq.events[seq.eventCt] if frame == nextEvent.frame && scanline == nextEvent.scanline && horizpos == nextEvent.horizpos { if nextEvent.hash != plb.digest.String() { + if plb.image != nil { + plb.image.Save(fmt.Sprintf("playback_crash_%s", plb.transcript), true) + } return peripherals.NoEvent, errors.NewFormattedError(errors.PlaybackHashError, fmt.Sprintf("line %d", nextEvent.line)) } diff --git a/recorder/recorder.go b/recorder/recorder.go index 0ab2a612..313c1d50 100644 --- a/recorder/recorder.go +++ b/recorder/recorder.go @@ -14,8 +14,8 @@ import ( // Recorder records controller events to disk, intended for future playback type Recorder struct { vcs *hardware.VCS - digest *renderers.DigestTV output *os.File + digest *renderers.DigestTV } // NewRecorder is the preferred method of implementation for the FileRecorder type diff --git a/regression/database.go b/regression/database.go index 9a2d4920..0ba1b114 100644 --- a/regression/database.go +++ b/regression/database.go @@ -156,7 +156,11 @@ func (db regressionDB) list(output io.Writer) { output.Write([]byte(db.regressions[db.keys[k]].String())) output.Write([]byte("\n")) } - output.Write([]byte(fmt.Sprintf("Total: %d\n", len(db.keys)))) + if len(db.keys) == 0 { + output.Write([]byte("regression DB is empty\n")) + } else { + output.Write([]byte(fmt.Sprintf("Total: %d\n", len(db.keys)))) + } } // add adds a cartridge to the regression db diff --git a/regression/regression.go b/regression/regression.go index c7a88d19..99110872 100644 --- a/regression/regression.go +++ b/regression/regression.go @@ -88,11 +88,13 @@ func RegressDelete(output io.Writer, confirmation io.Reader, key string) (bool, } // RegressAdd adds a new regression handler to the database -func RegressAdd(reg Handler) error { +func RegressAdd(output io.Writer, reg Handler) error { + output.Write([]byte(fmt.Sprintf("adding: %s\r", reg))) ok, err := reg.regress(true) if !ok || err != nil { return err } + output.Write([]byte(fmt.Sprintf("added: %s\r", reg))) db, err := startSession() if err != nil { @@ -118,7 +120,8 @@ func RegressRunTests(output io.Writer, keys []string) (int, int, int, error) { for k := range keys { v, err := strconv.Atoi(keys[k]) if err != nil { - output.Write([]byte(fmt.Sprintf("invalid key [%s]\n", keys[k]))) + msg := fmt.Sprintf("invalid key [%s]", keys[k]) + return -1, -1, -1, errors.NewFormattedError(errors.RegressionDBError, msg) } keysV = append(keysV, v) } diff --git a/television/basic.go b/television/basic.go index 18c231a1..a000c2bb 100644 --- a/television/basic.go +++ b/television/basic.go @@ -19,6 +19,12 @@ type BasicTelevision struct { // the outOfSpec flags will be true outOfSpec bool + // endOfScreen is set to true once the scanline value has reached the value + // of spec.ScanlinesTotal. it remains true until a new frame is triggered + // + // pixels will not be sent to the renderer when endOfScreen is true + endOfScreen bool + // state of the television // - the current horizontal position. the position where the next pixel will be // drawn. also used to check we're receiving the correct signals at the @@ -91,12 +97,11 @@ func (btv BasicTelevision) MachineInfoTerse() string { // MachineInfo returns the television information in verbose format func (btv BasicTelevision) MachineInfo() string { s := strings.Builder{} - outOfSpec := "" + s.WriteString(fmt.Sprintf("TV (%s)", btv.spec.ID)) if btv.outOfSpec { - outOfSpec = " !!" + s.WriteString(" !! ") } - s.WriteString(fmt.Sprintf("TV (%s)%s:\n", btv.spec.ID, outOfSpec)) - s.WriteString(fmt.Sprintf(" Frame: %d\n", btv.frameNum)) + s.WriteString(fmt.Sprintf("\n Frame: %d\n", btv.frameNum)) s.WriteString(fmt.Sprintf(" Scanline: %d\n", btv.scanline)) s.WriteString(fmt.Sprintf(" Horiz Pos: %d [%d]", btv.horizPos, btv.horizPos+btv.spec.ClocksPerHblank)) @@ -159,7 +164,7 @@ func (btv *BasicTelevision) Signal(sig SignalAttributes) error { } else { if btv.vsyncCount >= btv.spec.VsyncClocks { btv.outOfSpec = btv.vsyncCount != btv.spec.VsyncClocks - + btv.endOfScreen = false btv.frameNum++ btv.scanline = 0 btv.vsyncCount = 0 @@ -169,7 +174,7 @@ func (btv *BasicTelevision) Signal(sig SignalAttributes) error { btv.visibleBottom = btv.pendingVisibleBottom for f := range btv.renderers { - err := btv.renderers[f].NewFrame() + err := btv.renderers[f].NewFrame(btv.frameNum) if err != nil { return err } @@ -186,7 +191,7 @@ func (btv *BasicTelevision) Signal(sig SignalAttributes) error { btv.horizPos = -btv.spec.ClocksPerHblank btv.scanline++ for f := range btv.renderers { - err := btv.renderers[f].NewScanline() + err := btv.renderers[f].NewScanline(btv.scanline) if err != nil { return err } @@ -197,8 +202,8 @@ func (btv *BasicTelevision) Signal(sig SignalAttributes) error { // continue with an implied VSYNC btv.outOfSpec = true - // repeat the last scanline (over and over if necessary) - btv.scanline-- + // indicate end of screen has been reached + btv.endOfScreen = true } } else { btv.horizPos++ @@ -211,36 +216,48 @@ func (btv *BasicTelevision) Signal(sig SignalAttributes) error { // push screen limits outwards as required if !sig.VBlank { - if btv.scanline > btv.pendingVisibleBottom { + if btv.endOfScreen && btv.scanline > btv.pendingVisibleBottom { btv.pendingVisibleBottom = btv.scanline + 2 + + // keep within limits + if btv.pendingVisibleBottom > btv.spec.ScanlinesTotal { + btv.pendingVisibleBottom = btv.spec.ScanlinesTotal + } } if btv.scanline < btv.pendingVisibleTop { btv.pendingVisibleTop = btv.scanline - 2 + + // keep within limits + if btv.pendingVisibleTop < 0 { + btv.pendingVisibleTop = 0 + } } } // record the current signal settings so they can be used for reference btv.prevSignal = sig - // current coordinates - x := int32(btv.horizPos) + int32(btv.spec.ClocksPerHblank) - y := int32(btv.scanline) + if !btv.endOfScreen { + // current coordinates + x := int32(btv.horizPos) + int32(btv.spec.ClocksPerHblank) + y := int32(btv.scanline) - // decode color using the regular color signal - red, green, blue := getColor(btv.spec, sig.Pixel) - for f := range btv.renderers { - err := btv.renderers[f].SetPixel(x, y, red, green, blue, sig.VBlank) - if err != nil { - return err + // decode color using the regular color signal + red, green, blue := getColor(btv.spec, sig.Pixel) + for f := range btv.renderers { + err := btv.renderers[f].SetPixel(x, y, red, green, blue, sig.VBlank) + if err != nil { + return err + } } - } - // decode color using the alternative color signal - red, green, blue = getColor(btv.spec, sig.AltPixel) - for f := range btv.renderers { - err := btv.renderers[f].SetAltPixel(x, y, red, green, blue, sig.VBlank) - if err != nil { - return err + // decode color using the alternative color signal + red, green, blue = getColor(btv.spec, sig.AltPixel) + for f := range btv.renderers { + err := btv.renderers[f].SetAltPixel(x, y, red, green, blue, sig.VBlank) + if err != nil { + return err + } } } diff --git a/television/renderers/digesttv.go b/television/renderers/digesttv.go index eebcc3d2..46037378 100644 --- a/television/renderers/digesttv.go +++ b/television/renderers/digesttv.go @@ -5,13 +5,16 @@ import ( "fmt" "gopher2600/errors" "gopher2600/television" + "os" ) // DigestTV is a television implementation that type DigestTV struct { television.Television - screenData []byte - digest [sha1.Size]byte + digest [sha1.Size]byte + + frameData []byte + frameNum int } // NewDigestTV initialises a new instance of DigestTV @@ -42,25 +45,29 @@ func NewDigestTV(tvType string, tv television.Television) (*DigestTV, error) { // register ourselves as a television.Renderer dtv.AddRenderer(dtv) - // memory for screenData has to be sufficient for the entirety of the + // memory for frameData has to be sufficient for the entirety of the // screen plus the size of a fingerprint. we'll use the additional space to // chain fingerprint hashes - dtv.screenData = make([]byte, len(dtv.digest)+((dtv.GetSpec().ClocksPerScanline+1)*(dtv.GetSpec().ScanlinesTotal+1)*3)) + dtv.frameData = make([]byte, len(dtv.digest)+((dtv.GetSpec().ClocksPerScanline+1)*(dtv.GetSpec().ScanlinesTotal+1)*3)) return dtv, nil } // NewFrame implements television.Renderer interface -func (dtv *DigestTV) NewFrame() error { +func (dtv *DigestTV) NewFrame(frameNum int) error { // chain fingerprints by copying the value of the last fingerprint // to the head of the screen data - copy(dtv.screenData, dtv.digest[:len(dtv.digest)]) - dtv.digest = sha1.Sum(dtv.screenData) + n := copy(dtv.frameData, dtv.digest[:]) + if n != len(dtv.digest) { + return errors.NewFormattedError(errors.DigestTV, fmt.Sprintf("unexpected amount of data copied")) + } + dtv.digest = sha1.Sum(dtv.frameData) + dtv.frameNum = frameNum return nil } // NewScanline implements television.Renderer interface -func (dtv *DigestTV) NewScanline() error { +func (dtv *DigestTV) NewScanline(scanline int) error { return nil } @@ -69,14 +76,16 @@ func (dtv *DigestTV) SetPixel(x, y int32, red, green, blue byte, vblank bool) er // preserve the first few bytes for a chained fingerprint offset := len(dtv.digest) - offset += dtv.GetSpec().ClocksPerScanline * int(y) * 3 + offset = dtv.GetSpec().ClocksPerScanline * int(y) * 3 offset += int(x) * 3 - // allow indexing to naturally fail if offset is too big + if offset >= len(dtv.frameData) { + return errors.NewFormattedError(errors.DigestTV, fmt.Sprintf("the coordinates (%d, %d) passed to SetPixel will cause an invalid access of the frameData array", x, y)) + } - dtv.screenData[offset] = red - dtv.screenData[offset+1] = green - dtv.screenData[offset+2] = blue + dtv.frameData[offset] = red + dtv.frameData[offset+1] = green + dtv.frameData[offset+2] = blue return nil } @@ -96,3 +105,35 @@ func (dtv *DigestTV) ResetDigest() { dtv.digest[i] = 0 } } + +// Save current frame data to filename - filename base supplied as an argument, the +// frame number and file extension is appended by the function +func (dtv *DigestTV) Save(fileNameBase string) error { + // prepare filename for image + outName := fmt.Sprintf("%s_digest_%d.bin", fileNameBase, dtv.frameNum) + + f, err := os.Open(outName) + if f != nil { + f.Close() + return errors.NewFormattedError(errors.DigestTV, fmt.Sprintf("output file (%s) already exists", outName)) + } + if err != nil && !os.IsNotExist(err) { + return errors.NewFormattedError(errors.DigestTV, err) + } + + f, err = os.Create(outName) + if err != nil { + return errors.NewFormattedError(errors.DigestTV, err) + } + defer f.Close() + + n, err := f.Write(dtv.frameData) + if n != len(dtv.frameData) { + return errors.NewFormattedError(errors.DigestTV, "output truncated") + } + if err != nil { + return errors.NewFormattedError(errors.DigestTV, err) + } + + return nil +} diff --git a/television/renderers/imagetv.go b/television/renderers/imagetv.go index 8b7d6f18..faaaed37 100644 --- a/television/renderers/imagetv.go +++ b/television/renderers/imagetv.go @@ -18,13 +18,13 @@ type ImageTV struct { screenGeom image.Rectangle - // currImage is the image we write to, until newFrame() is called again - currImage *image.NRGBA - currFrameNum int + // currFrameData is the image we write to, until newFrame() is called again + currFrameData *image.NRGBA + currFrameNum int // this is the image we'll be saving when Save() is called - lastImage *image.NRGBA - lastFrameNum int + lastFrameData *image.NRGBA + lastFrameNum int } // NewImageTV initialises a new instance of ImageTV @@ -58,7 +58,7 @@ func NewImageTV(tvType string, tv television.Television) (*ImageTV, error) { } // start a new frame imtv.currFrameNum = -1 // we'll be adding 1 to this value immediately in newFrame() - err = imtv.NewFrame() + err = imtv.NewFrame(imtv.currFrameNum) if err != nil { return nil, err } @@ -72,14 +72,22 @@ func NewImageTV(tvType string, tv television.Television) (*ImageTV, error) { // Save last frame to filename - filename base supplied as an argument, the // frame number and file extension is appended by the function // +// currentFrame should be true if the current frame (which may be incomplete) +// should be save. if the value is false then the previous frame will be saved +// // return tv.Save(filepath.Join(state.Group, state.Label)) -func (imtv *ImageTV) Save(fileNameBase string) error { - if imtv.lastImage == nil { +func (imtv *ImageTV) Save(fileNameBase string, currentFrame bool) error { + if imtv.lastFrameData == nil { return errors.NewFormattedError(errors.ImageTV, "no data to save") } // prepare filename for image - imageName := fmt.Sprintf("%s_%d.png", fileNameBase, imtv.lastFrameNum) + var imageName string + if currentFrame { + imageName = fmt.Sprintf("%s_%d.png", fileNameBase, imtv.currFrameNum) + } else { + imageName = fmt.Sprintf("%s_%d.png", fileNameBase, imtv.lastFrameNum) + } f, err := os.Open(imageName) if f != nil { @@ -97,7 +105,11 @@ func (imtv *ImageTV) Save(fileNameBase string) error { defer f.Close() - err = png.Encode(f, imtv.lastImage) + if currentFrame { + err = png.Encode(f, imtv.currFrameData) + } else { + err = png.Encode(f, imtv.lastFrameData) + } if err != nil { return errors.NewFormattedError(errors.ImageTV, err) } @@ -106,24 +118,24 @@ func (imtv *ImageTV) Save(fileNameBase string) error { } // NewFrame implements television.Renderer interface -func (imtv *ImageTV) NewFrame() error { - imtv.lastImage = imtv.currImage +func (imtv *ImageTV) NewFrame(frameNum int) error { + imtv.lastFrameData = imtv.currFrameData imtv.lastFrameNum = imtv.currFrameNum - imtv.currImage = image.NewNRGBA(imtv.screenGeom) + imtv.currFrameData = image.NewNRGBA(imtv.screenGeom) imtv.currFrameNum++ return nil } // NewScanline implements television.Renderer interface -func (imtv *ImageTV) NewScanline() error { +func (imtv *ImageTV) NewScanline(scanline int) error { return nil } // SetPixel implements television.Renderer interface func (imtv *ImageTV) SetPixel(x, y int32, red, green, blue byte, vblank bool) error { col := color.NRGBA{R: red, G: green, B: blue, A: 255} - imtv.currImage.Set(int(x)*imtv.pixelWidth, int(y), col) - imtv.currImage.Set(int(x)*imtv.pixelWidth+1, int(y), col) + imtv.currFrameData.Set(int(x)*imtv.pixelWidth, int(y), col) + imtv.currFrameData.Set(int(x)*imtv.pixelWidth+1, int(y), col) return nil } diff --git a/television/television.go b/television/television.go index fc8cc522..d22c0c93 100644 --- a/television/television.go +++ b/television/television.go @@ -48,8 +48,8 @@ type Television interface { // Renderer implementations display information from a television type Renderer interface { - NewFrame() error - NewScanline() error + NewFrame(frameNum int) error + NewScanline(scanline int) error SetPixel(x, y int32, red, green, blue byte, vblank bool) error SetAltPixel(x, y int32, red, green, blue byte, vblank bool) error }