LinuxMCE Forums

General => Users => Topic started by: PeteK on November 11, 2007, 10:07:08 pm

Title: CD Ripping script does not use quality subsettings set in webadmin
Post by: PeteK on November 11, 2007, 10:07:08 pm
Hey guys--

I noticed that the CD ripping script, ripDiskWrapper.sh, is not using the quality settings set in the web admin when ripping CDs.  These settings affect MP3 and OGG encoding quality.  Currently, the arguments to the encoders are hard-coded in the script.  While the encoding type is passed in as an argument to the script, the quality subsettings are not.

I just wanted to point this out so someone doesn't rip an entire collection at a quality level that they didn't want.

I've filed a mantis report here:
http://mantis.linuxmce.org/view.php?id=3637

In the meantime, the way to correct it on your system is to modify ripDiskWrapper.sh to hard-code your desired bitrate/Bitrate arguments for lame and ogg

For mp3s: modify the following line in ripDiskWrapper.sh

            OutputFile='>(lame -h --tt "$FileName" --ta "$DARTIST" --tl "$DALBUM" --tn "$TrackNumber" --ty "$CDYEAR" --tg "$CDGENRE" - "$Dir/$FileName.'"$FinalExt"'.in-progress")' # encoder


You'll want to change the line that begins with "OutputFile='>(lame "
and add your quality settings there.  For example, add "-b bitrate"  where bitrate is your desired bitrate.  You could also add "-V"  for VBR encoding.

Similarly, modify the following line for .ogg encoding

            OutputFile='>(oggenc -Q --artist "$DARTIST" --album "$DALBUM" --date "$CDYEAR" --genre "$CDGENRE" --tracknum "$TrackNumber" -o "$Dir/$FileName.'"$FinalExt"'.in-progress" -)' # encoder

You'll want to change the line that begins with "OutputFile='>(oggenc"
and add your quality settings there.  For example, add "-q quality"  where quality is your desired quality level, -1 to 10.

Note that until the fix is added to LMCE, you'll have to check this script after updates (or lock the script file).

I haven't verified my arguments to the encoders, so you may want to verify that those are correct on a disk or two before ripping your entire collection.

-Pete



Title: Re: CD Ripping script does not use quality subsettings set in webadmin
Post by: darrenmason on November 11, 2007, 11:04:49 pm
Are you sure that the script isn't generated after a web admin change and reload router and/or reboot?
It may well not be, I have just seen a lot of the scripts done this way.
Title: Re: CD Ripping script does not use quality subsettings set in webadmin
Post by: PeteK on November 11, 2007, 11:54:16 pm
I don't think this one is, as it is tracked in SVN, but some experimentation might be in order to verify that.
Title: Re: CD Ripping script does not use quality subsettings set in webadmin
Post by: Zaerc on November 12, 2007, 12:41:57 am
Any idea how and from where this script is called?

I was expecting it to do a bunch of sql queries to get the settings information from the database as some other scripts do.  I might be able to hack that in anyway (if nobody else is working on fixing this up).  But it might be better to do that on a higher level and pass it as extra parameters to the script.
Title: Re: CD Ripping script does not use quality subsettings set in webadmin
Post by: PeteK on November 12, 2007, 01:14:15 am
Zearc--

I had looked into this before when I was digging into updatemedia and the ripping process.  If I recall correctly, the relevant module was the one that was in the same directory as the script file.  You can also grep ripDiskWrapper.sh, as I believe the .cpp source file that calls it does so by name.  I'd find the info for you, but I'm at the office right now catching up on a few things.  If you can't find it, I'll post what I can find at home tonight.
Title: Re: CD Ripping script does not use quality subsettings set in webadmin
Post by: PeteK on November 12, 2007, 01:57:46 am
My guess is that the program does in fact query the sql database to get the correct encoding method, but that those queries were not extended to include the subcategories for quality options.  It may be as simple as adding those queries to the ripping program and having it pass those as arguments to the script, just as it does currently with encoding type.  Please let me know what you find.
Title: Re: CD Ripping script does not use quality subsettings set in webadmin
Post by: Zaerc on November 12, 2007, 02:30:46 am
Found it in: LinuxMCE-1.1-SRC/src/Disk_Drive_Functions/RipTask.cpp and looking through that now to see if I can make heads and tails out of that.  It would probably be easier for me to hack it into the wrapper script, but I'll try to figure out how to do it properly in the source and pass it as extra parameters.

Looks like it needs to be added to "strParameters", so that it can be passed to the wrapper script.
Title: Re: CD Ripping script does not use quality subsettings set in webadmin
Post by: PeteK on November 12, 2007, 02:44:15 am
That sounds correct.  One thing to keep in mind is that we need to handle the case where the encoder requires additional parameters (MP3) as well as the one where it does not (FLAC).  My first instinct would be to have the program send the same number of parameters to the script, and the script will ignore the unnecessary parameters when it doesn't require them.  That would mean sending MP3 bit rate, VBR/CBR setting, MP3 custom settings (I assume that's what the text box is for on in the MP3 settings section) and Ogg quality level to the script every time it's called, and having the script decide which (if any) sub-parameters to use based on the type of encoding chosen. But if you've got a different method you'd like to try, I'm game.

-Pete
Title: Re: CD Ripping script does not use quality subsettings set in webadmin
Post by: Zaerc on November 12, 2007, 03:52:38 am
That was my idea as well, but I can't even seem to figure out how the mp3/ogg/flac/wav parameter gets filled from the database  (parameter $8 - $ripFormatString in the script / [m_]sFormat in the .cpp code).

I'm going to have a look at the DB now to see if I can figure out where the ripping settings are stored, maybe that will give me a lead.  But I think the cpp code is still way over my head, so I might just end up doing the SELECT's from the script.  That's probably less elegant but it would avoid the need to pass extra parameters altogether.
Title: Re: CD Ripping script does not use quality subsettings set in webadmin
Post by: PeteK on November 12, 2007, 04:05:30 am
Zaerc--

That's a good way to get it working in the short term.  I'm just worried about the script getting a little convoluted in that we're pulling in related parameters in two different ways.  I've already compromised myself in adding ripped file tagging inside the script for the 0710 release (I think it was pushed in an earlier update to 0704 as well), when it really belongs in UpdateMedia.  One day I'd like to get that cleaned up.  Maybe we can just add the select calls in the script to the list of things to clean up eventually, as well.
Title: Re: CD Ripping script does not use quality subsettings set in webadmin
Post by: Zaerc on November 12, 2007, 05:43:58 am
Digging from the other end, I found out that these settings are stored in the Device_DeviceData table as follows: "mp3;320;vbr" so the settings are already appended to the encoding type.  That either means that it gets stripped from the argument, or the info is already passed to the script.

That does seem to make it a bit redundant to retreive the value again in the script itself, doesn't it? :D

To test it I tried ripping a CD, only to find this in the logfile:
Code: [Select]
Spawn_rip_job_1_task_0_29990.log:Parameter: flac
In spite of not just changing that to mp3;320;vbr, but even having verified that value in the DB by looking it up. ???  Maybe I need to reboot (on a sidenote, my test core is acting up a little lately).

EDIT:

I just looked in the wrapper script again and it does keep the extra parameters into account... by stripping them off. :P
Code: [Select]
ripFormat=${ripFormatString%%;*}

So I would suggest something like this (around line 141):

                        mp3)
                                FinalExt="mp3"
                                BitRate="-b `echo $ripFormatString | cut -d';' -f2`"
                                if [ "`echo $ripFormatString | cut -d';' -f3`" = "vbr" ]
                                then
                                        BitRate="$BitRate -V"
                                fi

                                OutputFile='>(lame -h --tt "$FileName" --ta "$DARTIST" --tl "$DALBUM" --tn "$TrackNumber" --ty "$CDYEAR" --tg "$CDGENRE" - "$Dir/$FileName.'"$FinalExt"'.in-progress" $BitRate)'  # encoder
                        ;;


On another sidenote, I'm a bit puzzled by the fact that cdparanoia seems to be run at a very low priority (nice -n 15):
Code: [Select]
command='nice -n 15 cdparanoia -e -d "$sourceDevice" "$Track" - 2> '"$ProgressOutput"' > '"$OutputFile"
Seems to me that could deprive cdparanoia of some precious cpu cycles and make it more error prone.

I haven't properly tested any of this as it's now way past my bedtime, so I'm hoping you can take it from here.  The ogg part should be very similar as well.


EDIT: fixed a type-o in the modification above!
Title: Re: CD Ripping script does not use quality subsettings set in webadmin
Post by: 1audio on November 12, 2007, 06:28:06 am
Quote
cdparanoia seems to be run at a very low priority

Perhaps that explains why some disks can take hours to rip (I know they are not pristine, but they seem to be flawless on the CD player).
Title: Re: CD Ripping script does not use quality subsettings set in webadmin
Post by: Zaerc on November 12, 2007, 06:43:01 am
I have noticed that as well and I'm considering hardcoding cdparanoia to read at single speed, not to make it faster but to hopefully reduce the possible number of reading errors. 

My roomie has a cd collection that looks like he's been using them as coasters.  :o
Title: Re: CD Ripping script does not use quality subsettings set in webadmin
Post by: PeteK on November 12, 2007, 06:44:10 am
Zaerc--

That's a great find!  I'm much weaker at scripting than I am at general c++ coding so I'm hoping you or someone else can work this issue and maybe attach an updated file to the mantis report to fix this issue.  If you can't, I'll take a look and test out your script changes.  I'm not sure whey you're not getting a change in the parameter passed to the script.  It always seemed to work correctly for me after doing a quick reload router between changes.

-PeteK
Title: Re: CD Ripping script does not use quality subsettings set in webadmin
Post by: Zaerc on November 12, 2007, 07:34:05 am
Ah I think I forgot about the reload.  And I discovered an obvious type-o in my addition which I just fixed above.  From the logs it seems that the paramers are appended correctly now. 

I should be able to come up with a patch tomorrow, if nobody beats me to it. 

LOL I was scratching my head why I couldn't rip to mp3, until I realized... I did't have LAME installed, how lame is that! ;D  Would be nice if that produced a bit more meaningful error though, maybe I can look into that as well.  Seems to be working fine now.
Title: Re: CD Ripping script does not use quality subsettings set in webadmin
Post by: PeteK on November 12, 2007, 07:46:50 am
From the logs it seems that the paramers are appended correctly now. 
I should be able to come up with a patch tomorrow, if nobody beats me to it. 

I love it when a plan comes together.  Now I feel like a cigar ;)
Title: Re: CD Ripping script does not use quality subsettings set in webadmin
Post by: tschak909 on November 12, 2007, 07:55:54 am
*A-Team-theme-music* !

 :D
Title: Re: CD Ripping script does not use quality subsettings set in webadmin
Post by: Zaerc on November 12, 2007, 04:02:01 pm
Being more of a FLAC person myself, I was wondering: what would be the best/easiest way to double check that the MP3s are in fact properly encoded with the new quality settings?  opening them with mpg123 gives me some info and it seems that the bitrate is indeed set correctly, but I don't see anything about VBR in it's output...

EDIT:

I love it when a plan comes together.  Now I feel like a cigar ;)

Well I found a suitable track to test with:  ;D
Code: [Select]
Parameter: 53
Parameter: 1
Parameter: 0
Parameter: /mnt/device/75/data/audio/Pink Floyd/Wish You Were Here
Parameter: /dev/cdrom
Parameter: 0
Parameter: 1
Parameter: mp3;320;vbr
Parameter: 1,Shine on You Crazy Diamond, Pts. 1-5|2,Welcome to the Machine|3,Have a Cigar|4,Wish You Were Here|5,Shine on You Crazy Diamond, Pts. 6-9|
Ripping /dev/cdrom to "/mnt/device/75/data/audio/Pink Floyd/Wish You Were Here" with a disk of type 0 for user 1
Query Successful
Command: nice -n 15 cdparanoia -e -d "$sourceDevice" "$Track" - 2> >(/usr/pluto/bin/Paranoia_Progress.sh|/usr/pluto/bin/Pluto_Progress.sh "$diskDriveDeviceID" "$jobID" "$taskID" "$Dir/$F
ileName") > >(lame -h --tt "$FileName" --ta "$DARTIST" --tl "$DALBUM" --tn "$TrackNumber" --ty "$CDYEAR" --tg "$CDGENRE" -b 320 -v - "$Dir/$FileName.mp3.in-progress")
...
Track: 3; Filename: Have a Cigar
RESULT: Is not a data track
Executing: nice -n 15 cdparanoia -e -d /dev/cdrom 3 - 2> >(/usr/pluto/bin/Paranoia_Progress.sh|/usr/pluto/bin/Pluto_Progress.sh 53 1 0 /mnt/device/75/data/audio/Pink Floyd/Wish You Were Here/Have a Cigar) > >(lame -h --tt Have a Cigar --ta Pink Floyd --tl Wish You Were Here --tn 3 --ty 1975 --tg Rock -b 320 -v - /mnt/device/75/data/audio/Pink Floyd/Wish You Were Here/Have a Cigar.mp3.in-progress)
File ripped ok; moving: /mnt/device/75/data/audio/Pink Floyd/Wish You Were Here/Have a Cigar.mp3.in-progress
...

Code: [Select]
# mpg123 /mnt/device/75/data/audio/Pink\ Floyd/Wish\ You\ Were\ Here/Have\ a\ Cigar.mp3
High Performance MPEG 1.0/2.0/2.5 Audio Player for Layers 1, 2 and 3
        version 0.61; written and copyright by Michael Hipp and others
        free software (LGPL/GPL) without any warranty but with best wishes

Directory: /mnt/device/75/data/audio/Pink Floyd/Wish You Were Here/
Playing MPEG stream 1 of 1: Have a Cigar.mp3 ...
[../../../src/id3.c:198] error: ID3v2: non-syncsafe frame size, aborting
Note: Xing/Lame/Info header detected

        Title:   Have a Cigar
        Artist:  Pink Floyd
        Album:   Wish You Were Here
        Year:    1975
        Genre:   Rock
        Comment:

MPEG 1.0 layer III, 320 kbits/s, 44100 Hz joint-stereo

[5:08] Decoding of Have a Cigar.mp3 finished.

I'm using -v for specifying VBR instead of -V as that needs an argument.  Alternatively --vbr-new could be used to speed things up a bit, but I figured better safe then sorry.

Now I'm going to do one last test on the ogg part and then I'll throw the patch over the wall...

Title: Re: CD Ripping script does not use quality subsettings set in webadmin
Post by: Zaerc on November 12, 2007, 06:51:13 pm

Lowest quality ogg:

Code: [Select]
Parameter: 53
Parameter: 1
Parameter: 0
Parameter: /mnt/device/75/data/audio/Pink Floyd/Wish You Were Here
Parameter: /dev/cdrom
Parameter: 0
Parameter: 1
Parameter: ogg;1
Parameter: 4,Have a Cigar|
Ripping /dev/cdrom to "/mnt/device/75/data/audio/Pink Floyd/Wish You Were Here" with a disk of type 0 for user 1
Query Successful
Command: nice -n 15 cdparanoia -e -d "$sourceDevice" "$Track" - 2> >(/usr/pluto/bin/Paranoia_Progress.sh|/usr/pluto/bin/Pluto_Progress.sh "$diskDriveDeviceID" "$jobID" "$taskID" "$Dir/$F
ileName") > >(oggenc -Q --artist "$DARTIST" --album "$DALBUM" --date "$CDYEAR" --genre "$CDGENRE" --tracknum "$TrackNumber" -q 1 -o "$Dir/$FileName.ogg.in-progress" -)
Track: 4; Filename: Have a Cigar
RESULT: Is not a data track
Executing: nice -n 15 cdparanoia -e -d /dev/cdrom 4 - 2> >(/usr/pluto/bin/Paranoia_Progress.sh|/usr/pluto/bin/Pluto_Progress.sh 53 1 0 /mnt/device/75/data/audio/Pink Floyd/Wish You Were
Here/Have a Cigar) > >(oggenc -Q --artist Pink Floyd --album Wish You Were Here --date 1975 --genre Rock --tracknum 4 -q 1 -o /mnt/device/75/data/audio/Pink Floyd/Wish You Were Here/Have
 a Cigar.ogg.in-progress -)
File ripped ok; moving: /mnt/device/75/data/audio/Pink Floyd/Wish You Were Here/Have a Cigar.ogg.in-progress
Ripping successful

Code: [Select]
# ogginfo /mnt/device/75/data/audio/Pink\ Floyd/Wish\ You\ Were\ Here/Have\ a\ Cigar.ogg
Processing file "/mnt/device/75/data/audio/Pink Floyd/Wish You Were Here/Have a Cigar.ogg"...

Warning: Hole in data found at approximate offset 4500 bytes. Corrupted ogg.
New logical stream (#1, serial: 37f2543c): type vorbis
Vorbis headers parsed for stream 1, information follows...
Version: 0
Vendor: Xiph.Org libVorbis I 20050304
Channels: 2
Rate: 44100

Nominal bitrate: 80.000000 kb/s
Upper bitrate not set
Lower bitrate not set
User comments section follows...
        artist=Pink Floyd
        genre=Rock
        date=1975
        album=Wish You Were Here
        tracknumber=4
Vorbis stream 1:
        Total data length: 3186327 bytes
        Playback length: 5m:40.333s
        Average bitrate: 74.898970 kb/s
Logical stream 1 ended
Warning: Hole in data found at approximate offset 3190784 bytes. Corrupted ogg.


Highest quality ogg:

Code: [Select]
Parameter: 53
Parameter: 1
Parameter: 0
Parameter: /mnt/device/75/data/audio/Pink Floyd/Wish You Were Here
Parameter: /dev/cdrom
Parameter: 0
Parameter: 1
Parameter: ogg;10
Parameter: 4,Have a Cigar|
Ripping /dev/cdrom to "/mnt/device/75/data/audio/Pink Floyd/Wish You Were Here" with a disk of type 0 for user 1
Query Successful
Command: nice -n 15 cdparanoia -e -d "$sourceDevice" "$Track" - 2> >(/usr/pluto/bin/Paranoia_Progress.sh|/usr/pluto/bin/Pluto_Progress.sh "$diskDriveDeviceID" "$jobID" "$taskID" "$Dir/$F
ileName") > >(oggenc -Q --artist "$DARTIST" --album "$DALBUM" --date "$CDYEAR" --genre "$CDGENRE" --tracknum "$TrackNumber" -q 10 -o "$Dir/$FileName.ogg.in-progress" -)
Track: 4; Filename: Have a Cigar
RESULT: Is not a data track
Executing: nice -n 15 cdparanoia -e -d /dev/cdrom 4 - 2> >(/usr/pluto/bin/Paranoia_Progress.sh|/usr/pluto/bin/Pluto_Progress.sh 53 1 0 /mnt/device/75/data/audio/Pink Floyd/Wish You Were
Here/Have a Cigar) > >(oggenc -Q --artist Pink Floyd --album Wish You Were Here --date 1975 --genre Rock --tracknum 4 -q 10 -o /mnt/device/75/data/audio/Pink Floyd/Wish You Were Here/Hav
e a Cigar.ogg.in-progress -)
File ripped ok; moving: /mnt/device/75/data/audio/Pink Floyd/Wish You Were Here/Have a Cigar.ogg.in-progress
Ripping successful

Code: [Select]
# ogginfo /mnt/device/75/data/audio/Pink\ Floyd/Wish\ You\ Were\ Here/Have\ a\ Cigar.ogg
Processing file "/mnt/device/75/data/audio/Pink Floyd/Wish You Were Here/Have a Cigar.ogg"...

New logical stream (#1, serial: 5d675cc4): type vorbis
Vorbis headers parsed for stream 1, information follows...
Version: 0
Vendor: Xiph.Org libVorbis I 20050304
Channels: 2
Rate: 44100

Nominal bitrate: 499.821000 kb/s
Upper bitrate not set
Lower bitrate not set
User comments section follows...
        artist=Pink Floyd
        genre=Rock
        date=1975
        album=Wish You Were Here
        tracknumber=4
Vorbis stream 1:
        Total data length: 19429228 bytes
        Playback length: 5m:40.333s
        Average bitrate: 456.710550 kb/s
Logical stream 1 ended

Just for completeness and to be able to say that I've really tested it, low quality CBR MP3:

Code: [Select]
Parameter: 53
Parameter: 1
Parameter: 0
Parameter: /mnt/device/75/data/audio/Pink Floyd/Wish You Were Here
Parameter: /dev/cdrom
Parameter: 0
Parameter: 1
Parameter: mp3;96;cbr
Parameter: 4,Have a Cigar|
Ripping /dev/cdrom to "/mnt/device/75/data/audio/Pink Floyd/Wish You Were Here" with a disk of type 0 for user 1
Query Successful
Command: nice -n 15 cdparanoia -e -d "$sourceDevice" "$Track" - 2> >(/usr/pluto/bin/Paranoia_Progress.sh|/usr/pluto/bin/Pluto_Progress.sh "$diskDriveDeviceID" "$jobID" "$taskID" "$Dir/$FileName") > >(lame -h --tt "$FileName" --ta "$DARTIST" --tl "$DALBUM" --tn "$TrackNumber" --ty "$CDYEAR" --tg "$CDGENRE" -b 96 - "$Dir/$FileName.mp3.in-progress")
Track: 4; Filename: Have a Cigar
RESULT: Is not a data track
Executing: nice -n 15 cdparanoia -e -d /dev/cdrom 4 - 2> >(/usr/pluto/bin/Paranoia_Progress.sh|/usr/pluto/bin/Pluto_Progress.sh 53 1 0 /mnt/device/75/data/audio/Pink Floyd/Wish You Were Here/Have a Cigar) > >(lame -h --tt Have a Cigar --ta Pink Floyd --tl Wish You Were Here --tn 4 --ty 1975 --tg Rock -b 96 - /mnt/device/75/data/audio/Pink Floyd/Wish You Were Here/Have a Cigar.mp3.in-progress)
File ripped ok; moving: /mnt/device/75/data/audio/Pink Floyd/Wish You Were Here/Have a Cigar.mp3.in-progress
Ripping successful

Code: [Select]
# mpg123 /mnt/device/75/data/audio/Pink\ Floyd/Wish\ You\ Were\ Here/Have\ a\ Cigar.mp3
High Performance MPEG 1.0/2.0/2.5 Audio Player for Layers 1, 2 and 3
        version 0.61; written and copyright by Michael Hipp and others
        free software (LGPL/GPL) without any warranty but with best wishes

Directory: /mnt/device/75/data/audio/Pink Floyd/Wish You Were Here/
Playing MPEG stream 1 of 1: Have a Cigar.mp3 ...
Note: Xing/Lame/Info header detected
Title  : Have a Cigar                    Artist: Pink Floyd
Album  : Wish You Were Here              Year  : 1975
Comment:                                 Genre : Rock
MPEG 1.0 layer III, 96 kbits/s, 44100 Hz joint-stereo

[5:40] Decoding of Have a Cigar.mp3 finished.

And now I hear the track "Wish you were here" playing, which was not what I ripped so I guess I've stumbeled across another (unrelated) bug here.  As this obviously has nothing to do with the changes I've made I'm uploading the patch anyway. 

Then I'll have a stab at figuring out why the ripDiskWrapper.sh script is being told to actually read the wrong track, as can clearly be seen in the logs samples I've posted. 

Title: Re: CD Ripping script does not use quality subsettings set in webadmin
Post by: PeteK on November 12, 2007, 07:45:33 pm
Zaerc--

I implemented the file tagging in that script.  It uses its own call to the cddb database, with only track number being passed from the calling program.  There may be an error either in my implementation or in my assumptions about the track number data that the script gets from the rip program.  I'll put this on the front burner tonight when I get home, but if you get a chance to poke around in it, please do so and let me know.  Like I said before, I'm really a hack when it comes to scripts, so I may have missed something.

Thanks,
-Pete

BTW, good choice of song to rip.
Title: Re: CD Ripping script does not use quality subsettings set in webadmin
Post by: Zaerc on November 12, 2007, 08:17:34 pm
Well as far as I can tell from the logs the script does it's job properly, it just gets told to rip track 4 instead of track 3 (in this particular case).  It seems to have nothing to do with the retrieval of the cddb information.

As you can see here from the first test I did, "Have a cigar" is track3:

Parameter: 1,Shine on You Crazy Diamond, Pts. 1-5|2,Welcome to the Machine|3,Have a Cigar|4,Wish You Were Here|5,Shine on You Crazy Diamond, Pts. 6-9|
...
Track: 3; Filename: Have a Cigar


Then all the times I ripped the single track, the script gets told to rip track 4 erroneously (and with the name of track 3) instead:

Parameter: 4,Have a Cigar|
...
Track: 4; Filename: Have a Cigar


I'll see if I can dig something up in the cpp code, although I haven't really found my way around with that.

Title: Re: CD Ripping script does not use quality subsettings set in webadmin
Post by: PeteK on November 12, 2007, 08:44:32 pm
Well, that's good to know I didn't screw that part up at least.  Thanks for the good work so far.  This is an important feature that I think has to be solid.  I'd be pretty pissed if I ripped my entire collection and found out LMCE screwed up and I had to do it again.
Title: Re: CD Ripping script does not use quality subsettings set in webadmin
Post by: Zaerc on November 13, 2007, 03:26:33 pm
I've made a small baby step here, a new instance of the "RipJob" class is created in a member called Disk_Drive::CMD_Rip_Disk, which can be found in LinuxMCE-1.1-SRC/src/Disk_Drive/Disk_Drive.cpp

Code: [Select]
        /** @brief COMMAND: #337 - Rip Disk */
        /** This will try to RIP a DVD to the HDD. */
                /** @param #2 PK_Device */
                        /** The ID of the disk drive or jukebox */
                /** @param #13 Filename */
                        /** The target disk name, or for cd's, a comma-delimited list of names for each track. */
                /** @param #17 PK_Users */
                        /** The user who needs this rip in his private area. */
                /** @param #20 Format */
                        /** wav, flac, ogg, etc. */
                /** @param #121 Tracks */
                        /** For CD's, this must be a comma-delimted list of tracks (1 based) to rip. */
                /** @param #131 EK_Disc */
                        /** The ID of the disc to rip.  If not specified this will be whatever disc is currently playing the entertainment area. */
                /** @param #151 Slot Number */
                        /** The slot if this is a jukebox */
                /** @param #233 DriveID */
                        /** The PK_Device ID of the storage drive that will be ripped to. Can be the ID of the core to store in /home */
                /** @param #234 Directory */
                        /** The relative directory for the file to rip */

void Disk_Drive::CMD_Rip_Disk(int iPK_Device,string sFilename,int iPK_Users,string sFormat,string sTracks,int iEK_Disc,int iSlot_Number,int iDriveID,string sDirectory,string &sCMD_Result,Message *pMessage)
//<-dceag-c337-e->
{
        RipJob *pRipJob = new RipJob(m_pDatabase_pluto_media,m_pJobHandler,m_pDisk_Drive_Functions,NULL,iPK_Users,iEK_Disc,
                pMessage ? pMessage->m_dwPK_Device_From : 0,sFormat,sFilename,sDirectory,sTracks,true,this);
        m_pJobHandler->AddJob(pRipJob);
}

I suppose this is getting called after a DCE (command) message?  Maybe to the Media_Plugin?
Title: Re: CD Ripping script does not use quality subsettings set in webadmin
Post by: wombiroller on March 11, 2009, 12:07:57 pm
Digging from the other end, I found out that these settings are stored in the Device_DeviceData table as follows: "mp3;320;vbr" so the settings are already appended to the encoding type.  That either means that it gets stripped from the argument, or the info is already passed to the script.

That does seem to make it a bit redundant to retreive the value again in the script itself, doesn't it? :D

To test it I tried ripping a CD, only to find this in the logfile:
Code: [Select]
Spawn_rip_job_1_task_0_29990.log:Parameter: flac
In spite of not just changing that to mp3;320;vbr, but even having verified that value in the DB by looking it up. ???  Maybe I need to reboot (on a sidenote, my test core is acting up a little lately).

EDIT:

I just looked in the wrapper script again and it does keep the extra parameters into account... by stripping them off. :P
Code: [Select]
ripFormat=${ripFormatString%%;*}

So I would suggest something like this (around line 141):

                        mp3)
                                FinalExt="mp3"
                                BitRate="-b `echo $ripFormatString | cut -d';' -f2`"
                                if [ "`echo $ripFormatString | cut -d';' -f3`" = "vbr" ]
                                then
                                        BitRate="$BitRate -V"
                                fi

                                OutputFile='>(lame -h --tt "$FileName" --ta "$DARTIST" --tl "$DALBUM" --tn "$TrackNumber" --ty "$CDYEAR" --tg "$CDGENRE" - "$Dir/$FileName.'"$FinalExt"'.in-progress" $BitRate)'  # encoder
                        ;;


On another sidenote, I'm a bit puzzled by the fact that cdparanoia seems to be run at a very low priority (nice -n 15):
Code: [Select]
command='nice -n 15 cdparanoia -e -d "$sourceDevice" "$Track" - 2> '"$ProgressOutput"' > '"$OutputFile"
Seems to me that could deprive cdparanoia of some precious cpu cycles and make it more error prone.

I haven't properly tested any of this as it's now way past my bedtime, so I'm hoping you can take it from here.  The ogg part should be very similar as well.


EDIT: fixed a type-o in the modification above!


Heya All,

This fix worked (not sure if this is an antiquated fix but it's all I could find??) but I noticed that it now rips the CD into a folder named something like: RP85G1~P...

Easy enough to rename again but I thought I should note that down...

Cheers,
WR.