If you remove the pause embedded in a TZX data block and replace it with with a pause block of the same length then this can cause a tape loading error because the pause block does not change the pulse level in order to finish the last edge of the data block.
See attached test case (try with --no-detect-loader --no-accelerate-loader).
This patch for libspectrum fixes the problem.
Does this patch have any interaction with the proposed fix for [bugs:#431]?
Related
Bugs: #431
No, they are independent things, you can actually try any combination of both test cases and both patches (Mask.tzx for #431 and trailing-pause-block.tzx for #458).
I'll explain:
Bug #458 deals with pauses: they should always produce an edge to ensure that the previous data block is correctly terminated. This should happen for trailing pauses in a data block and also for separate pause blocks (as in the attached test case). So this patch fixes the latter case by simply ensuring that a pause block does not set any
LIBSPECTRUM_TAPE_FLAGS_LEVEL_*
flags.Bug #431 deals with happens after the pauses: if a block ends with a pause (or if we started playback from the middle of the tape) then the next block should start with
LIBSPECTRUM_TAPE_FLAGS_LEVEL_LOW.
In this case there is no previous block to terminate (because the pause block already took care of that) so in principle the pulse level does not really matter, but we want it to be always low in order to ensure that polarity-sensitive loaders always get the same data from the tape.In the case of #431 there is an additional patch that adds a "Reverse tape polarity" option to Fuse. This is not strictly necessary in my opinion, I'm open to feedback about whether we should include it or not: https://sourceforge.net/p/fuse-emulator/bugs/431/?page=1&limit=25#f102
Thanks, I'm in two minds about this one - I feel like the TZX spec doesn't seem to explicity say that a pause block following another block is the equivalent of the embedded pause, e.g.
And by explicitly giving responsibility to TZX file creators to manage level when relevant using Pause blocks:
Looking at this, I am not sure if we have pauses right in libspectrum as I don't think we do this "To ensure that the last edge produced is properly finished there should be at least 1 ms. pause of the opposite level and only after that the pulse should go to 'low'. ".
What do you think?
I admit that the spec is a bit ambiguous.
My understanding is that this is talking about when those blocks don't have an embedded pause, because you obviously want to be able to concatenate two data blocks without a pause between them.
And what I understant from this is that pause blocks and embedded pauses should be handled equally (or is there a reason why they shouldn't?).
I also tried to implement this part of the spec but that seemed to break many other loaders. See https://sourceforge.net/p/fuse-emulator/bugs/431/?page=1&limit=25#4322 and Sergio's reply for more details. So I went for the simpler approach.
If you have a 100ms pause after a data block (whether the pause is embedded in the block or not) there should be an edge or else the loader will fail, but in the end I think it doesn't matter whether the pulse goes low 1ms after that or at the end of the pause.
Thanks, understood. Committed with a test case in [f1e33b].
Related
Commit: [f1e33b]