cyanreg / cyanrip

Bule-ish CD ripper
GNU Lesser General Public License v2.1
222 stars 10 forks source link

Pre-Emphasis support? #42

Closed sodface closed 1 year ago

sodface commented 1 year ago

Hi, I recently recommended cyanrip in a forum discussion here: https://forums.slimdevices.com/forum/user-forums/ripping-encoding-transcoding-tagging/1630286-what-cd-drive-for-ripping-cds

Which prompted a discussion about pre-emphasis support, which I hadn't heard of. I haven't read much further on it beyond this article: https://archimago.blogspot.com/2020/09/how-to-cd-pre-emphasis-and-dealing-with.html

I don't see that pre-emphasis detection (or de-emphasis during processing) is currently supported? Is that correct? Is this something you've looked at before and elected not to support? Thanks!

cyanreg commented 1 year ago

Emphasis is detected and identified, it's visible in the logs. Currently, it's not wired up to automatically deemphasise, though it's easy enough to support, just haven't had any CDs that require it yet. I'll implement it.

sodface commented 1 year ago

I burned a CD that has preemphasis and ripped it back with cyanrip, detected in the log as you said, so I'm ready to test deemphasis once implemented :)

Track 2 ripped and encoded successfully!
    File(s):
        Quake [FLAC]/02 - Quake Theme.flac
    Metadata:
        mbid: 1d6c2de9-4612-4f00-88e4-a1ad219298a7
        title: Quake Theme
        artist: Trent Reznor and Nine Inch Nails
        comment: cyanrip 0.8.1
        track: 2
        tracktotal: 11
        date: 1996-06-22
        disc_mcn: 0000000000000
        discid: oWAGWCDCO.dMpqTyR7f9LEUFVLQ-
        cddb: 9d0e790b
        release_id: 046d66a3-ec8f-4d24-873f-21a11f67b0fb
        album: Quake
        barcode: 611350226660
        packaging: Digipak
        country: US
        status: Official
        album_artist: Trent Reznor and Nine Inch Nails
        totaldiscs: 1
        disc: 1
        format: Mixed Mode CD
        isrc: 000000000000
        creation_time: 2023-03-11T10:11:44
    Preemphasis:      present, deemphasis required
    Cover art:        Motion JPEG
    Duration:         00:05:08.300
    Samples:          13609260
    Frames:           23145
    Start LSN:        12695
    End LSN:          35839
    Offset end:       35840
    Accurip:          not found
    EAC CRC32:        CBF047FA
    Accurip v1:       BEEE6933
    Accurip v2:       DF9D16D5
    Accurip v1 450:   DAFDEE39
cyanreg commented 1 year ago

Alright, implemented deemphasis (enabled by default) in the deemphasis branch, https://github.com/cyanreg/cyanrip/tree/deemphasis Give it a test and let me know if the output is what you'd expect.

sodface commented 1 year ago

Thanks a lot for supporting this so quickly! It seems to be working as expected and as documented in the README update. I'd already done some preliminary testing with 0.8.1 with manual deemphasis after rip. 3 samples of 11s duration uploaded for comparison:

Unprocessed sample ripped with 0.8.1 with manual deemphasis:
ffmpeg -i 02-QuakeTheme_PREEMPHASIS.flac -filter_complex "aemphasis=mode=reproduction:type=cd" -sample_fmt s16 02-QuakeTheme_DEEMPHASIS.flac

02-QuakeTheme_PREEMPHASIS_11s.flac 02-QuakeTheme_DEEMPHASIS_11s.flac

Third sample ripped with 0.9.0 (deemphasis branch) 02-QuakeTheme_git_DEEMPHASIS_11s.flac

http://sodface.com/repo/index/misc/preemph_test

cyanreg commented 1 year ago

I was hoping for comparisons from something else that isn't FFmpeg, to make sure the filter in FFmpeg is correct. If you can't get anything, that's fine. Thanks for testing. I'll work on it a little more and push it to master, and see about making 0.9.1, since I have a patch to drop the use of deprecated FFmpeg APIs too.

cyanreg commented 1 year ago

Pushed to master Hopefully someone can test it Not sure about applying it by default, as it's not lossless per-se, but I do believe it's more necessary for listening properly than HDCD (which is mostly just nonsense).

sodface commented 1 year ago

I should have replied but didn't get a chance before work. What kind of tests are you looking for? I ripped to .wav with cdda2wav this morning both with and without the -T option which applies deemphasis. Was also going to run the unprocessed .wav through sox deemphasis, so I'd have two other deemphasis samples for comparison. But these are subjective tests so I don't know if it's really what you are after?

cyanreg commented 1 year ago

Just as long as the spectrum looks fine, I'd be happy with it. The lavfi filter seems to do the right thing, just don't know how right it is.

sodface commented 1 year ago

I replaced the sample files at http://sodface.com/repo/index/misc/preemph_test

02-QuakeTheme_PREEMPHASIS_11s.wav
02-QuakeTheme_cdda2wav_DEEMPHASIS_11s.wav
02-QuakeTheme_cyanrip_DEEMPHASIS.wav
02-QuakeTheme_sox_DEEMPHASIS_11s.wav
cyanreg commented 1 year ago

Thanks. I looked around, and those look fine. CD deemphasis wasn't really specified properly, and each vendor did their own, more or less. Our curve is a bit flatter because we normalize to avoid clipping, and we low-pass to avoid aliasing, but it's well within the spec. Thanks for testing.

cyanreg commented 1 year ago

By the way, I implemented a mode to force deemphasis via -E. I remember reading somewhere that such CDs exist which don't flag the audio as pre-emphasized. Do you think you could look/ask around if there's a list of such bad CDs? I assume it's not a long list, I can probably integrate it into cyanrip.

sodface commented 1 year ago

I'll do some research, though as I mentioned I'm no expert here, but I'm interested which is at least part of the battle.

As far as flagging for preemphasis, are you referring to the TOC specifically/only?

It’s important to note that some of these CD’s reveal their PRE-EMPHASIS flag only in the sub-code of the disc, not in the TOC as they should, and the majority of CD rippers only look for the flag in the TOC (apparently there are legal problems with software applications accessing the sub-code). Such is the case, for example, with Boz Scaggs - Silk Degrees, CBS Japan 35DP-20 which I acquired recently.

https://forum.audacityteam.org/t/cd-de-emphasis-eq-preset-for-audacity-3-x/63789/3

That same post has a couple links to cd lists. But I'll keep researching. Just wanted to verify what cyanrip is doing currently for preemphasis detection, TOC only?

//edit The link in the above forum post is broken, should be: https://www.studio-nibble.com/cd/index.php?title=Pre-emphasis_(release_list)

sodface commented 1 year ago

I just received The Blue Nile | Hatslisted on this page as Cat # CD 5284: https://www.studio-nibble.com/cd/index.php?title=Pre-emphasis_(release_list)

It's listed as not having Preemphasis flagged in the TOC but present in the subchannel.

cdrdao seems to confirm this:

~ $ doas cdrdao read-toc --device /dev/sr0 --rspeed 4 test_orig.toc
doas (sodface@desk-5070) password: 
Cdrdao version 1.2.4 - (C) Andreas Mueller <andreas@daneb.de>
/dev/sr0: ASUS SDRW-08U7M-U Rev: A101
Using driver: Generic SCSI-3/MMC - Version 2.0 (options 0x0000)

Setting reading speed 4.
Reading toc data...

Track   Mode    Flags  Start                Length
------------------------------------------------------------
 1      AUDIO   0      00:00:00(     0)     05:04:55( 22855)
 2      AUDIO   0      05:04:55( 22855)     06:31:25( 29350)
 3      AUDIO   0      11:36:05( 52205)     05:21:30( 24105)
 4      AUDIO   0      16:57:35( 76310)     06:16:73( 28273)
 5      AUDIO   0      23:14:33(104583)     04:02:50( 18200)
 6      AUDIO   0      27:17:08(122783)     05:11:05( 23330)
 7      AUDIO   0      32:28:13(146113)     06:27:62( 29087)
Leadout AUDIO   0      38:56:00(175200)

PQ sub-channel reading (audio track) is supported, data format is BCD.
Raw P-W sub-channel reading (audio track) is supported.
Cooked R-W sub-channel reading (audio track) is supported.
Analyzing track 01 (AUDIO): start 00:00:00, length 05:04:55...
Found 174 Q sub-channels with CRC errors.
WARNING: Pre-emphasis flag of track differs from TOC - toc file contains TOC setting.
Analyzing track 02 (AUDIO): start 05:04:55, length 06:31:25...
Found pre-gap: 00:02:25
Found 375 Q sub-channels with CRC errors.
WARNING: Pre-emphasis flag of track differs from TOC - toc file contains TOC setting.
...

Current cyanrip git output indicates preemphasis detection is only looking at the TOC so deemphasis wouldn't be applied as desired?

Track 1 ripped and encoded successfully!
    File(s):
        Hats [WAV]/1 - Over the Hillside.wav
    Metadata:
        mbid: 3d22ff59-e940-4214-a05a-18d6e4c155ff
        title: Over the Hillside
        artist: The Blue Nile
        comment: cyanrip 0.9.0
        track: 1
        tracktotal: 7
        disc_mcn: 0000000000000
        discid: 9VYlrN.hxSlyBd4R2jD6QFMpDGo-
        cddb: 62092007
        date: 1990-01-04
        release_id: daf83f63-9e96-49bf-9b8a-3fc1b29b3024
        album: Hats
        barcode: 075021528420
        packaging: Jewel Case
        country: US
        status: Official
        album_artist: The Blue Nile
        totaldiscs: 1
        disc: 1
        format: CD
        creation_time: 2023-03-27T18:12:36
    Preemphasis:      none
    Duration:         00:05:04.300
    Samples:          13438740
    Frames:           22855
    Start LSN:        0
    End LSN:          22854
    Offset end:       22855
    Accurip:          found (max confidence: 56)
    EAC CRC32:        3D13D9B2
    Accurip v1:       909ED44C (accurately ripped, confidence 43)
    Accurip v2:       F92D87FB (accurately ripped, confidence 56)

Tracks ripped accurately: 1/7

Can you add subchannel preemphasis detection and/or an option to force deemphasis (opposite of -W)?

cyanreg commented 1 year ago

Err, there already is an option to force deemphasis, -E, it's the most recent commit I made. I looked into cdio, and I don't think it offers the raw TOC track control, which seems to be what cdrdao uses. I'll look more.

sodface commented 1 year ago

Err, there already is an option to force deemphasis, -E, it's the most recent commit I made.

Good grief, yes, sorry about that, you only mentioned it two comments ago and it's one of the reasons I bought the Hats CD for testing!

I looked into cdio, and I don't think it offers the raw TOC track control, which seems to be what cdrdao uses. I'll look more.

I won't be much use with the solution but standing by to test if needed.

Incidentally, here's an hour long presentation on pre & deemphasis that's way over my head but maybe of academic interest to someone reading this issue:

https://youtu.be/WL2t0X8Mb-w

cyanreg commented 1 year ago

@sodface I think I found a way, could you test the https://github.com/cyanreg/cyanrip/tree/deemphasis branch to check if preemphasis is properly detected now?

sodface commented 1 year ago

60s sample files here: http://sodface.com/repo/index/misc/preemph_test

Preemphasis detection worked with command doas cyanrip -s 6 -R 1 -l 1 -o wav:

Track 1 ripped and encoded successfully!
    File(s):
        Hats [WAV]/1 - Over the Hillside.wav
    Metadata:
        mbid: 3d22ff59-e940-4214-a05a-18d6e4c155ff
        title: Over the Hillside
        artist: The Blue Nile
        comment: cyanrip 0.9.0
        track: 1
        tracktotal: 7
        disc_mcn: 0000000000000
        discid: 9VYlrN.hxSlyBd4R2jD6QFMpDGo-
        cddb: 62092007
        date: 1990-01-04
        release_id: daf83f63-9e96-49bf-9b8a-3fc1b29b3024
        album: Hats
        barcode: 075021528420
        packaging: Jewel Case
        country: US
        status: Official
        album_artist: The Blue Nile
        totaldiscs: 1
        disc: 1
        format: CD
        creation_time: 2023-03-28T17:45:25
    Preemphasis:      removed
    Duration:         00:05:04.300
    Samples:          13438740
    Frames:           22855
    Start LSN:        0
    End LSN:          22854
    Offset end:       22855
    Accurip:          found (max confidence: 56)
    EAC CRC32:        3D13D9B2
    Accurip v1:       909ED44C (accurately ripped, confidence 43)
    Accurip v2:       F92D87FB (accurately ripped, confidence 56)

Tracks ripped accurately: 1/7

Paranoia status counts:
    READ:          7051
    VERIFY:        1251
    OVERLAP:       277

Ripping errors: 0
Ripping finished at 2023-03-28T17:51:17

Quite a difference in file sizes:

205.1M  1-Over_the_Hillside_DEEMPHASIS.wav
51.3M   1-Over_the_Hillside_PREEMPHASIS.wav

Sox complained when creating the 60s snip, but only for the deemphasis rip:

$ sox 1-Over_the_Hillside_DEEMPHASIS.wav 1-Over_the_Hillside_DEEMPHASIS_60s.wav trim 0 60
sox WARN wav: wave header missing extended part of fmt chunk
cyanreg commented 1 year ago

So it works, nice. I'll polish it a bit and push it. The deemphasis filter outputs 64-bit floats, which then get converted to 64-bit ints, which .wav files preserve. 64/16 = 4, hence the 4x size difference. Rounding the output to 16-bits is a one-line change, I should probably do that, rather than waste massive amounts of space for a little lower quantization noise.

cyanreg commented 1 year ago

Patch pushed. Would be nice if you could test again, just in case.

sodface commented 1 year ago

Tests from latest master:

Quake CD which has preemphasis flagged in TOC:

        album_artist: Trent Reznor and Nine Inch Nails
        totaldiscs: 1
        disc: 1
        format: Mixed Mode CD
        isrc: 000000000000
        creation_time: 2023-03-29T20:32:13
    Preemphasis:      removed

The Blue Nile CD which does not have preemphasis flagged in the TOC but present in SUBQ:

        album_artist: The Blue Nile
        totaldiscs: 1
        disc: 1
        format: CD
        creation_time: 2023-03-29T20:05:31
    Preemphasis:      removed

Opal CD, no preemphasis:

        album_artist: Opal
        totaldiscs: 1
        disc: 1
        format: CD
        creation_time: 2023-03-29T20:47:44
    Preemphasis:      none

So the preemphasis detection seems to work well. I'm still seeing big file size differences in the deemphasis rips though:

207.7M  Quake_Deemph/
52.0M   Quake_Preemph/
205.1M  Hats_Deemph/
51.3M   Hats_Preemph/
cyanreg commented 1 year ago

Right, I missed a spot. Fixed it, and made preemphasis logging better. Thanks for testing.

sodface commented 1 year ago

Awesome, looks good:

        album_artist: The Blue Nile
        totaldiscs: 1
        disc: 1
        format: CD
        creation_time: 2023-03-29T22:05:25
    Preemphasis:      present (subcode) (deemphasis applied)
51.3M   Hats [WAV]/
        album_artist: Trent Reznor and Nine Inch Nails
        totaldiscs: 1
        disc: 1
        format: Mixed Mode CD
        isrc: 000000000000
        creation_time: 2023-03-29T22:18:57
    Preemphasis:      present (TOC) (deemphasis applied)
52.0M   Quake [WAV]/

Deemphasis section of the README needs an update for the changes in log output.

richardpl commented 4 months ago

Note that I removed 20k Hz lowpass on reproduction mode for all types of aemphasis filter completely as its pointless, available in librempeg repo. (You can not reduce aliasing if it already happened in input...)