Deal with CDs where the start track number is not 1.

[vext01]

As discussed on the mailing list.

(Also a small OpenBSD build-system fix).

[rocky]

Looks very good. thanks!

Would you please add in test/data a suitable image that can be used for testing when the track is not one?

Even better would be to then use that in test.

Thanks again.

[vext01]

If I understand correctly, I should add a disk image to the repo in the form of a cue and bin and then add a test which uses it.

What tools should I use to make the cue and bin? Does the audio contained have to be meaningful?

[rocky]

The disk image doesn't have to be a CDWIN cue and bin image, Neror nrg or or cdrdao toc would work as well.

Presumably you burned a disk to do manual testing. How did you create that? Perhaps some sort of semi or fully automated way that can create a disk would be fine.

However if you use a disk image like bin/cue or nrg, then that makes automated testing easy.

[vext01]

I used a command from Thomas to create the CD:

cdrskin -v dev=/dev/rcd0d cd_start_tno=7 trackA.wav trackB.wav trackC.wav

That burns a physical CD, but we need an on-disk image.

[rocky]

Even having the above as a shell script along with the wav files that it uses is better than nothing.

You get the idea: there was effort put into testing that this code works. How can we package this so that others might be able to benefit from it and replicate the results, to see that things work?

[vext01]

Hi Rocky,

Thomas has showed me how to make a bin and cue file for the test, but I'm at a loss as to where/how to add my test.

I envisage a test which uses a bin/cue as input, and which checks the TOC against an expected outcome. Should I add a new test script to do this?

Cheers

[rocky]

The CD image can go in test/data. And yes, create a script in the test directory, like endian.sh. A zero exit code is success, exit code 77 means that the test was skipped. I don't think this would happen here, but it would say if you were testing a real CD-ROM with a CDDA format Disc in it and when running the test test you find you either don't have a CD-ROM accessible or that it doesn't have a CDDA disc. Any other return code is an error. And thanks!

[vext01]

Hi,

I've started writing the test.

Sadly, there seem to be problems. If I run cd-paranoia -Q -d start_track_not_one.cue, where the cue file contains:

FILE "/home/edd/source/libcdio-paranoia/test/start_track_not_one.bin" BINARY
  TRACK 07 AUDIO
    FLAGS DCP
    INDEX 01 00:00:00
  TRACK 08 AUDIO
    FLAGS DCP
    INDEX 01 00:04:02
  TRACK 09 AUDIO
    FLAGS DCP
    INDEX 01 00:08:04

I get:

cdparanoia III release 10.2 libcdio 2.0.0 x86_64-unknown-openbsd6.4
(C) 2001 Monty <monty@xiph.org> and Xiphophorus
(C) 2004, 2005, 2008 Rocky Bernstein <rocky@gnu.org>
(C) 2014 Robert Kausch <robert.kausch@freac.org>

Report bugs to bug-libcdio@gnu.org

405: Option not supported by drive

Table of contents (audio tracks only):
track        length               begin        copy pre ch
===========================================================
  1.      302 [00:04.02]        0 [00:00.00]    OK   no  2
  2.      302 [00:04.02]      302 [00:04.02]    OK   no  2
  3.      302 [00:04.02]      604 [00:08.04]    OK   no  2
TOTAL     906 [00:12.06]    (audio only)

So the track numbers are wrong!

This is with all of my fixes to date, so I'm guessing there's another part of the code that assumes tracks start at #1. The cue parser perhaps?

Will look into it when I find some time.

[rocky]

I'm guessing there's another part of the code that assumes tracks start at #1. The cue parser perhaps?

Probably. That would be in lib/image/bincue.c somewhere.

Will look into it when I find some time.

Thanks.

[vext01]

OK, here's my test. What do you think?

Please don't merge yet, as it needs Thomas' changes first.

If this looks good, then I will squash the two last commits together and offer them to Thomas as an example for the test he is writing for libcdio.

To prove it works:

$ gmake test
...
PASS: start_track_not_one.sh
============================================================================
Testsuite summary for libcdio-paranoia 10.2+0.94+2git
============================================================================
# TOTAL: 5
# PASS:  4
# SKIP:  1
# XFAIL: 0
# FAIL:  0
# XPASS: 0
# ERROR: 0
============================================================================

Now if we remove track 7 from the input cue file, leaving only tracks 8 and 9, we get in test-suite.log:

FAIL: start_track_not_one.sh
============================

Expected track 7, got 8.
Expected track 8, got 9.
Number of tracks was not 3.
FAIL start_track_not_one.sh (exit status: 1)

[rocky]

As before looks good. Sure - please pass this onto Thomas and we'll merge when everything is ready.

Now if we remove track 7 ...

If you want do add that expected failure cue as its own test, extend the script and add that to XFAIL_TEST in automake. See https://www.gnu.org/software/automake/manual/html_node/Scripts_002dbased-Testsuites.html

[vext01]

If you want do add that expected failure cue as its own test

If it's OK with you, I'd rather leave it as is. Such a test would be testing the awk script and not libcdio-paranoia.

I'll now squash the commits and show Thomas.

[vext01]

Taking the email discussion back to the PR:

For the github PR, I think what should be done is run the test only if
VERSION is greater than "2.0.0". I have just bumped the version set to
"2.0.1" . Otherwise we skip that particular test (or have expected to fail
since it shows that there is a bug - your choice).

Why don't we just let it fail, so as to prompt people to upgrade libcdio?

[rocky]

I don't want CI to fail in this project.

If you want to update CI in this project to pull libcdio from git sources and then build that before building libcdio-paranoia that is another possible solution.

[vext01]

OK, let's xfail it then.

For the github PR, I think what should be done is run the test only if VERSION is greater than "2.0.0"

It's the libcdio version we are talking about, isn't it? How can I get the version? I was hoping PKG_CHECK_MODULES would set a substitution variable, but I don't think it does...

[rocky]

In include file cdio/version.h is C Preprocessor symbol LIBCDIO_VERSION_NUM. For the last release it is 20000. I just bumped it to 20001.

[vext01]

I see. I'll have a play around and see if I can get at that from the test Makefile somehow and conditionally add my test to XFAIL_TESTS.

Thanks

[vext01]

Hi Rocky,

The best course of action I've found so far is to do something like this:
https://stackoverflow.com/a/46738777/109414

So we could have a cdio_ge_2 variable which I can subsitute into the test Makefile.

Actually I might be able to achieve the same using PKG_CHECK_MODULES without an error in the false branch.

It would be better if we could get the actual version number into a variable though. Then we wouldn't need a different variable each time we need to do a different version check.

Do you know if it's possible?

Cheers and Happy New Year!

[rocky]

Branch track1_test has some addiional changes (that is not the PR, but just changes to the PR testing) that coiuld be used to make this work. If you don't want to use but do some other way, that's okay too. I didn't spend a lot of time thinking about this.

I'm also cool to adding additional stuff to libcdio to make this kind of thing easier in the future. So feel free to make a suggestion there.

BTW it would be cool to use the same testing convention in the tests: C tests start out with the word test and shell script test start out with the word check. I've made that change in that branch.

[vext01]

How about this rocky? Rather than having a program which checks a specific version of libcdio, we have a program which simply prints the version, then the check scripts can do whatever they want with it.

I also renamed the script, as you suggested.

If you like it, I'd like to squash the commits before merge.