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

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.

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.

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.

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.

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.

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.

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.

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.

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.

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?

Spawn_rip_job_1_task_0_29990.log:Parameter: flacIn 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.

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.

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! 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.