Author Topic: CD Ripping script does not use quality subsettings set in webadmin  (Read 2747 times)

PeteK

  • Guru
  • ****
  • Posts: 408
    • View Profile
Re: CD Ripping script does not use quality subsettings set in webadmin
« Reply #15 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 ;)

tschak909

  • LinuxMCE God
  • ****
  • Posts: 5498
  • DOES work for LinuxMCE.
    • View Profile
Re: CD Ripping script does not use quality subsettings set in webadmin
« Reply #16 on: November 12, 2007, 07:55:54 am »
*A-Team-theme-music* !

 :D

Zaerc

  • Alumni
  • LinuxMCE God
  • *
  • Posts: 2256
  • Department of Redundancy Department.
    • View Profile
Re: CD Ripping script does not use quality subsettings set in webadmin
« Reply #17 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...

« Last Edit: November 12, 2007, 05:50:10 pm by Zaerc »
"Change is inevitable. Progress is optional."
-- Anonymous


Zaerc

  • Alumni
  • LinuxMCE God
  • *
  • Posts: 2256
  • Department of Redundancy Department.
    • View Profile
Re: CD Ripping script does not use quality subsettings set in webadmin
« Reply #18 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. 

"Change is inevitable. Progress is optional."
-- Anonymous


PeteK

  • Guru
  • ****
  • Posts: 408
    • View Profile
Re: CD Ripping script does not use quality subsettings set in webadmin
« Reply #19 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.

Zaerc

  • Alumni
  • LinuxMCE God
  • *
  • Posts: 2256
  • Department of Redundancy Department.
    • View Profile
Re: CD Ripping script does not use quality subsettings set in webadmin
« Reply #20 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.

"Change is inevitable. Progress is optional."
-- Anonymous


PeteK

  • Guru
  • ****
  • Posts: 408
    • View Profile
Re: CD Ripping script does not use quality subsettings set in webadmin
« Reply #21 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.

Zaerc

  • Alumni
  • LinuxMCE God
  • *
  • Posts: 2256
  • Department of Redundancy Department.
    • View Profile
Re: CD Ripping script does not use quality subsettings set in webadmin
« Reply #22 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?
"Change is inevitable. Progress is optional."
-- Anonymous


wombiroller

  • Guru
  • ****
  • Posts: 340
    • View Profile
Re: CD Ripping script does not use quality subsettings set in webadmin
« Reply #23 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.