Bug 200925 - audio/cdparanoia: Update to cdparanoia-III-10.2 (3.10.2)
Summary: audio/cdparanoia: Update to cdparanoia-III-10.2 (3.10.2)
Status: In Progress
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Alexey Dokuchaev
URL: http://xiph.org/paranoia/
Keywords:
Depends on:
Blocks:
 
Reported: 2015-06-17 00:18 UTC by tuden0
Modified: 2024-07-04 06:48 UTC (History)
4 users (show)

See Also:
bugzilla: maintainer-feedback? (danfe)


Attachments
patch to update audio/cdparanoia to the latest released version (66.96 KB, patch)
2019-07-27 13:13 UTC, Tobias Rehbein
no flags Details | Diff
patch to update audio/cdparanoia to the latest released version (98.75 KB, patch)
2024-01-13 15:33 UTC, Tobias Rehbein
no flags Details | Diff
patch to update audio/cdparanoia to the latest released version (68.70 KB, patch)
2024-01-13 16:33 UTC, Tobias Rehbein
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description tuden0 2015-06-17 00:18:47 UTC
Update audio/cdparanoia from cdparanoia-III-alpha9.8 to cdparanoia-III-10.2 .
https://xiph.org/paranoia/
http://downloads.xiph.org/releases/cdparanoia/cdparanoia-III-10.2.src.tgz
https://svn.xiph.org/trunk/cdparanoia/
Comment 1 Alexey Dokuchaev freebsd_committer freebsd_triage 2015-07-16 14:42:43 UTC
Yes, I'm having a half-baked (actually more than half: it is runnable) sitting im my tree, but had experiences weird segmentation faults that did not occur with current (previous) version.

I know it's been a while, I really need to get my act together and finish it.  Thanks for bugging!
Comment 2 tuden0 2015-07-28 23:52:29 UTC
Maybe post the link to your tree in here or put the code somewhere
(github?) and post to multimedia@ maybe some other people will look
at it with you. I'm sure you will find these faults :)

As far as port version things...

Some other OS are based on cdparanoia RELEASE, but are carrying
some patches that have since been committed to cdparanoia HEAD,
which is redundant. Since cdparanoia is small and inactive, there
would be no need to do that, or to maintain a -dev version of it
in ports. So I would actually base the port on cdparanoia HEAD in
the following way and order:

the RELEASE tarball above + one patch file to HEAD + patch files
needed to port the HEAD to FreeBSD + any remaining bug and feature
patches from other OS that are not in HEAD.

cdparanoia HEAD r17289 2010-06-11
cdparanoia RELEASE 10.2 r15302 2008-09-11

svn log https://svn.xiph.org/trunk/cdparanoia/
svn diff -r 15302:HEAD https://svn.xiph.org/trunk/cdparanoia/ > HEAD.patch

Here are the other major independant OS repos for reference, review,
and inclusion. Note the kFreeBSD patch, and that both NetBSD and
all the Linux are on 10.2 (the rest of BSD are on 9.8).

http://cvsweb.netbsd.org/bsdweb.cgi/pkgsrc/audio/cdparanoia/
https://svnweb.freebsd.org/ports/head/audio/cdparanoia/
https://github.com/DragonFlyBSD/DPorts/tree/master/audio/cdparanoia
http://cvsweb.openbsd.org/cgi-bin/cvsweb/ports/audio/cdparanoia/

https://anonscm.debian.org/cgit/pkg-opt-media/cdparanoia.git
https://anonscm.debian.org/cgit/pkg-opt-media/cdparanoia.git/tree/debian/patches/05-kfreebsd.patch
https://projects.archlinux.org/svntogit/packages.git/?h=packages/cdparanoia
https://pkgs.fedoraproject.org/cgit/cdparanoia.git/
https://build.opensuse.org/package/show/openSUSE:Factory/cdparanoia
https://svnweb.mageia.org/packages/cauldron/cdparanoia/current/
Comment 3 Walter Schwarzenfeld 2018-01-12 07:37:31 UTC
Any advance here?
Comment 4 Walter Schwarzenfeld 2019-02-15 23:31:17 UTC
ping!
Comment 5 Tobias Rehbein 2019-07-24 07:33:01 UTC
I recently decided to give this update a try on my own, independent of the maintainers work. I have an experimental version of cdparanoia-III-10.2 that seems to work fine on my machine.

I am still testing with some audio CDs that triggered reproducible SCSI errors using 9.8 (this was the reason I decided to work on this issue) and it looks good so far.

In other words: I don't feel ready to officially submit this update right now (in the end this stuff is surprisingly low level), but if someone wants to try the update and provide feedback or if danfe is interested in merging my stuff into their work, feel free: 

https://github.com/blabber/freebsd-ports-wip/tree/master/audio/cdparanoia
Comment 6 Tobias Rehbein 2019-07-27 13:13:11 UTC
Created attachment 206100 [details]
patch to update audio/cdparanoia to the latest released version
Comment 7 Tobias Rehbein 2019-07-27 13:14:53 UTC
audio/cdparanoia: Update to cdparanoia-III-10.2 (3.10.2)

The attached patch updates cdparanoia to the latest release. Additionally the patch adds support for audio CDs containing data tracks.

Three patches committed to the upstream SVN are also included:
-r15314: Optimizes autosense and fixes a bug in verify_read_command.
-r15337: fixes the "-p" option.
-r15338: fixes build on gcc architectures.

* portlint: looks fine
* poudriere: amd64 12.0 and amd64 11.3 are ok

I ripped several enhanced CDs (audio + data) using this version without any issues. I also managed to rip a CD that reproducible triggered a SCSI transport error with the old version. Another CD that reported a scratch using the old version, reported only jitter using this version.
Comment 8 Kubilay Kocak freebsd_committer freebsd_triage 2019-08-03 08:32:35 UTC
Thank you for the QA'd update Tobias!
Comment 9 Tobias Rehbein 2019-09-10 19:29:16 UTC
A final report from my side: I ripped 147 audio CDs and enhanced audio cds using the attached patch. I am quite confident, that the update works fine.

My main issue is, that I only have a single drive to test, but I got a success report by one person on IRC who volunteered to test the update.
Comment 10 Alexey Dokuchaev freebsd_committer freebsd_triage 2019-09-23 15:42:57 UTC
(In reply to tobias.rehbein from comment #9)

> I ripped 147 audio CDs and enhanced audio CDs using the attached patch.
> I am quite confident, that the update works fine.
I did a ripping of my CD collection back in the time when actively testing 10.2 update, and discovered that several CDs did not rip identically compared to EAC, albeit EAC log reported 100% match with AccurateRip database on a calibrated drive (same hardware, just booted under Windoze).  Hex-diff showed some bytes/words were swapped here and there, as well as some other minor artefacts AFAIR.

Did you see anything alike?  Existing version did not exhibit these issues, thus blocking the update.  I need to retest it again, since time had passed and things might have changed.
Comment 11 Tobias Rehbein 2019-09-26 18:00:42 UTC
(In reply to Alexey Dokuchaev from comment #10)

Oh, sorry. I don't have a windows system and therefore can't compare. What I have checked is, that the old and the new version produce identical WAV files if cdparanoia signals a perfect rip. But I have done the comparison just for a few CDs.
Comment 12 Daniel Engberg freebsd_committer freebsd_triage 2021-12-07 00:23:28 UTC
Any updates on this one?
Comment 13 Daniel Engberg freebsd_committer freebsd_triage 2022-02-12 08:20:47 UTC
Can we close this by now since there's no upstream development and I guess bugs remain? A wild guess is that it may have something to do with the new drive cache changes/workarounds?

@danfe 
If you can, can you give perhaps give audio/cyanrip a quick try to see if it exhibits the same issue? It works fine for me at least
Comment 14 Tobias Rehbein 2022-02-12 09:42:32 UTC
I am still using this patch together with audio/abcde without any issues. That said it is okay for me too close this issue, if there are doubts. I will just keep this in my local ports tree (as I did for six years now).
Comment 15 Tobias Rehbein 2022-02-12 09:50:44 UTC
(In reply to Tobias Rehbein from comment #14)
s/six years/three years/
Comment 16 Alexey Dokuchaev freebsd_committer freebsd_triage 2022-02-13 06:07:21 UTC
(In reply to Daniel Engberg from comment #13)
Thanks for the pointer, I'll check but I'd rather keep this PR open, as it's still valid, just low priority ATM.  I still intend to ensure there would be no regressions and update the port one day.
Comment 17 Tobias Rehbein 2024-01-13 15:33:05 UTC
Created attachment 247627 [details]
patch to update audio/cdparanoia to the latest released version

Updated the patch to one created using `git format-patch`. I also removed some new issues created by stage-qa and portlint:

* strip binary
* move WWW from pkg-descr to Makefile
Comment 18 Daniel Engberg freebsd_committer freebsd_triage 2024-01-13 16:04:55 UTC
Hi Tobias,

I'd like to test this one but there seems to be a couple of stray files?

extra-patch-upstream-patch-r15314
extra-patch-upstream-patch-r15337
extra-patch-upstream-patch-r15338
patch-main.c
patch-paranoia_overlap.c

Thanks for working on this!

Best regards,
Daniel
Comment 19 Tobias Rehbein 2024-01-13 16:33:21 UTC
Created attachment 247628 [details]
patch to update audio/cdparanoia to the latest released version

Hi Daniel. Interesting, I don't know what happened, but I rerolled the patch.
Comment 20 Daniel Engberg freebsd_committer freebsd_triage 2024-01-13 17:23:55 UTC
Thanks!

Appears to work on my end and results seems to similar to what cyanrip produces but offset seems a bit different which is to be expected I guess?

Best regards,
Daniel
Comment 21 Tobias Rehbein 2024-01-13 18:16:10 UTC
(In reply to Daniel Engberg from comment #20)
To be honest, I don't know. I submitted my initial patch 2019, when I was ripping my whole CD collection and the version in ports failed on some CDs. I ripped a total of 147 CDs and was happy with the result. That's all I can say ;)

I just happened to look through the PRs I am involved in and got the idea to update the patch, to make life easier for others.
Comment 22 Alexey Dokuchaev freebsd_committer freebsd_triage 2024-07-03 15:27:27 UTC
So I've finally sat down and worked through the suggested patch.  With some tweaking, version 3.10.2 builds and runs without issues; I didn't get any segfaults I'd seen several years ago.  Another problem was some occasional swapped bytes compared to EAC rip for a couple of CDs of mine, but I cannot reassess whether it still happens today, so we can disregard this point.

(In reply to Tobias Rehbein from comment #7)
> I ripped several enhanced CDs (audio + data) using this version without
> any issues. I also managed to rip a CD that reproducible triggered a SCSI
> transport error with the old version. Another CD that reported a scratch
> using the old version, reported only jitter using this version.
I've ripped a few CDs from various years with the new version and sound-wise results seem fine; I've waived the deeper AccurateRip comparison and/or finding out proper offset for now as I don't want to delay the update further.  I'll be committing modified (cleaned up and improved) version of the patch once my small tinderbox exp-run finishes.  Thanks everyone for testing and patience.
Comment 23 Daniel Engberg freebsd_committer freebsd_triage 2024-07-03 18:15:26 UTC
I'm just curious here, by offset do you mean read offset? That should be adjusted by/in the client (consumer) software otherwise you'd break a lot of consumers?
Comment 24 Alexey Dokuchaev freebsd_committer freebsd_triage 2024-07-04 03:24:52 UTC
(In reply to Daniel Engberg from comment #23)
> I'm just curious here, by offset do you mean read offset?
I mean --sample-offset so that different drives can produce identical rips from the same CD press batch.  You might find the following interesting:

  https://hydrogenaud.io/index.php/topic,123490.0.html
  http://www.accuraterip.com/driveoffsets.htm

> That should be adjusted by/in the client (consumer)
Right, as it depends on the particular hardware (drive model) user has on her hands.
Comment 25 commit-hook freebsd_committer freebsd_triage 2024-07-04 06:27:41 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=d0b1d9ec73207ca2400394af1db811ddcf3d2759

commit d0b1d9ec73207ca2400394af1db811ddcf3d2759
Author:     Tobias Rehbein <tobias.rehbein@web.de>
AuthorDate: 2024-07-04 06:24:52 +0000
Commit:     Alexey Dokuchaev <danfe@FreeBSD.org>
CommitDate: 2024-07-04 06:24:52 +0000

    audio/cdparanoia: the port had been updated to version III-10.2

    - GC no longer useful post-patch target and GNU_CONFIGURE_MANPREFIX
    - Replace stripping in post-install with proper BSD_INSTALL_* calls

    PR:     200925

 audio/cdparanoia/Makefile                          |  17 +-
 audio/cdparanoia/distinfo                          |   5 +-
 audio/cdparanoia/files/patch-Makefile.in           |  20 +-
 audio/cdparanoia/files/patch-configure (gone)      |  15 -
 audio/cdparanoia/files/patch-interface_Makefile.in |  24 +-
 .../files/patch-interface_cdda__interface.h        |  22 +-
 .../files/patch-interface_common__interface.c      |  35 ++-
 audio/cdparanoia/files/patch-interface_interface.c |  36 ++-
 .../files/patch-interface_low__interface.h         |  14 +-
 .../files/patch-interface_scan__devices.c          |  70 +++--
 .../files/patch-interface_scsi__interface.c        | 238 +++++++++-------
 audio/cdparanoia/files/patch-main.c (new)          |  15 +
 .../files/patch-paranoia_overlap.c (new)           |  20 ++
 audio/cdparanoia/files/patch-upstream-r15314 (new) | 312 +++++++++++++++++++++
 audio/cdparanoia/files/patch-upstream-r15337 (new) |  31 ++
 audio/cdparanoia/files/patch-version.h (gone)      |  17 --
 audio/cdparanoia/pkg-plist                         |   4 +-
 17 files changed, 643 insertions(+), 252 deletions(-)
Comment 26 Alexey Dokuchaev freebsd_committer freebsd_triage 2024-07-04 06:48:04 UTC
Actually we don't have to (ab)use EXTRA_PATCHES in order to keep upstream SVN cherry-picks apart from FreeBSD-specific patches, we can simply name them as such.  I've also dropped r15338 (private -> private_data rename) after some hesitation because it's a mechanical change which does not affect us, at least for the moment.  Other than that it's pretty much what Tobias had submitted, good work!

One particular change I wanted to ask about but forgot: in the handle_scsi_cmd() function, regarding out_size vs. in_size:

> -+  if (bytecheck && out_size == 0)
> ++  if (bytecheck && in_size == 0)
> 
> -+  /* flags */ CAM_DEV_QFRZDIS | (out_size ? CAM_DIR_OUT : CAM_DIR_IN),
> ++  /* flags */ CAM_DEV_QFRZDIS | (in_size ? CAM_DIR_OUT : CAM_DIR_IN),
> 
> -+  /* data_ptr */ out_size ? d->sg_buffer + cmd_len : d->sg_buffer,
> ++  /* data_ptr */ in_size ? cmd + cmd_len : d->private->sg_buffer,
Comparing with Linux code didn't give me immediate clue if this is (was)
a bug that got fixed in the new version, or just the logic had changed?