#458 Tape loading error if there's a trailing pause block

[Alberto Garcia]

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).

[Alberto Garcia]

This patch for libspectrum fixes the problem.

[Fredrick Meunier]

Does this patch have any interaction with the proposed fix for [bugs:#431]?

[Alberto Garcia]

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

[Fredrick Meunier]

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.

• The 'current pulse level' after playing the blocks ID 10,11,12,13,14 or 19 is the opposite of the last pulse level played, so that a subsequent pulse will produce an edge.
• A 'Pause' block consists of a 'low' pulse level of some duration. 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'. At the end of a 'Pause' block the 'current pulse level' is low (note that the first pulse will therefore not immediately produce an edge). A 'Pause' block of zero duration is completely ignored, so the 'current pulse level' will NOT change in this case. This also applies to 'Data' blocks that have some pause duration included in them.

And by explicitly giving responsibility to TZX file creators to manage level when relevant using Pause blocks:

The writer of a TZX file should ensure that the 'current pulse level' is well-defined in every sequence of blocks where this is important, i.e. in any sequence that includes a 'Direct recording' block, or that depends on edges generated by 'Pause' blocks. The recommended way of doing this is to include a Pause after each sequence of 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?

[Alberto Garcia]

I admit that the spec is a bit ambiguous.

The 'current pulse level' after playing the blocks ID 10,11,12,13,14 or 19 is the opposite of the last pulse level played, so that a subsequent pulse will produce an edge.

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.

This also applies to 'Data' blocks that have some pause duration included in 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?).

there should be at least 1 ms. pause of the opposite level and only after that the pulse should go to 'low'

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.

[Fredrick Meunier]

Thanks, understood. Committed with a test case in [f1e33b].