Code Where Orbiter Sends DCE Event?

[hari]

The Powerfile_C200 should be managing its own defaults, not the Orbiter, anyway.

i disagree. It's a global setting and the command should include the global format parameter when sent from the orbiter. Otherwise other disc libraries would have to duplicate code.

best regards,
Hari

[Matthew]

The Powerfile_C200 should be managing its own defaults, not the Orbiter, anyway.

i disagree. It's a global setting and the command should include the global format parameter when sent from the orbiter. Otherwise other disc libraries would have to duplicate code.

Why is the orbiter any more global than is the Powerfile? The Orbiter is the GUI, and it doesn't even have an interface to set the ripping parameter. If it did, and it sent the format param, then that should indeed override the defaults. The Powerfile is the ripping device, so it should be able to access and use its default as set in the DB - unless overridden by the Orbiter.

I'm not sure what you mean by "other disc libraries". They're all going to be subclasses of the Powerfile, if they do differ from it at all. In the meantime, the Powerfile itself is enough of an abstraction that the VGP-XL1B that I'm using (and is the reference machine for LMCE) is, in SW, just a "Powerfile". If a future device different enough from a Powerfile that it isn't a subclass of the existing class, its new class should have code to get its settings from the DB, including the rip format.

However, the actual object that executes the ripDiskWrapper.sh script is a RipTask object that is contained (inside a RipJob object) in the Disk_Drive_Functions object. Which the Powerfile object sends to, and which any disc management object will send to for ripping. The right place for the rip format default is in the RipTask or the RipJob, which should know about its own rip job parameters. However, the current default code in the Powerfile class that converts empty format properties into "ogg" prevents the RipTask or RipJob from detecting whether it's "ogg" only because it went to default. That Powerfile code should be removed, and moved to the RipTask or RipJob class. And really, the default format shouldn't be OGG, because it's lossy. Since FLAC is a lossless compressed format, I'd like to make the default FLAC. So the Orbiter can set the format, or else the RipJob sets it to the Installation Settings in the DB, or else the RipJob sets it to FLAC.

src/Powerfile_C200/Powerfile_C200.cpp lines 577-579:

Code: [Select]

            RipJob *pRipJob = new RipJob(m_pPowerfileJukebox->m_pDatabase_pluto_media,
                m_pPowerfileJukebox->m_pJobHandler,NULL,pSlot,iPK_Users,0,
                pMessage ? pMessage->m_dwPK_Device_From : 0,[b]sFormat.empty() ? "ogg" : sFormat[/b],"",sFilename,"A",false,this);

src/Disk_Drive_Functions/RipTask.cpp lines 112-118:

Code: [Select]

    strParameters = StringUtils::itos(m_pRipJob->m_pDisk_Drive_Functions->m_pCommand_Impl->m_dwPK_Device) + "\t"
        + StringUtils::itos(m_pJob->m_iID_get()) + "\t"
        + StringUtils::itos(m_iID_get()) + "\t"
        + ((RipJob *) m_pJob)->m_sFileName + "\t" + m_pRipJob->m_pDisk_Drive_Functions->m_sDrive + "\t"
        + StringUtils::itos(m_pRipJob->m_pDisk_Drive_Functions->m_mediaDiskStatus) + "\t"
        + StringUtils::itos(m_pRipJob->m_iPK_Users) + "\t"
        + [b]m_pRipJob->m_sFormat[/b] + "\t" + m_sTrack;

src/Disk_Drive_Functions/RipTask.cpp lines 132-140:

Code: [Select]

    DCE::CMD_Spawn_Application
        spawnApplication(m_pRipJob->m_pDisk_Drive_Functions->m_pCommand_Impl->m_dwPK_Device,
                        m_pRipJob->m_pDisk_Drive_Functions->m_pDevice_AppServer->m_dwPK_Device,
                        "/usr/pluto/bin/ripDiskWrapper.sh",
                        m_sSpawnName,
                        strParameters,
                        sResultMessage + "e",
                        sResultMessage + "s",
                        false, false, false, true);


[Matthew]

However, the actual object that executes the ripDiskWrapper.sh script is a RipTask object that is contained (inside a RipJob object) in the Disk_Drive_Functions object. Which the Powerfile object sends to, and which any disc management object will send to for ripping. The right place for the rip format default is in the RipTask or the RipJob, which should know about its own rip job parameters. However, the current default code in the Powerfile class that converts empty format properties into "ogg" prevents the RipTask or RipJob from detecting whether it's "ogg" only because it went to default. That Powerfile code should be removed, and moved to the RipTask or RipJob class. And really, the default format shouldn't be OGG, because it's lossy. Since FLAC is a lossless compressed format, I'd like to make the default FLAC. So the Orbiter can set the format, or else the RipJob sets it to the Installation Settings in the DB, or else the RipJob sets it to FLAC.

Looking more carefully at the RipJob and RipTask classes, it looks like they are just managing the hardware, not performing any logic on the rip itself, which is all in the Powerfile object. Which is wrong - the ripping operation itself should manage its defaults, even if other objects like the Orbiter that have more immediate data (like user override when requesting the rip) that should override the default. But the whole architecture is wrong. The Powerfile is the hardware executing the ripping, and the RipJob and RipTask are abstractions that should be calling the Powerfile's hardware. The Powerfile should know whether it uses ripDiskWrapper.sh or some other script that knows what code actually runs that rippable object's HW.

But I'm not going to fix the architecture all at once just to ensure the defaults are called by the right object. That will just make fixing it later harder, because the code has been further shuffled around the wrong objects in the wrong architecture. I'm going to make the minimum revision, which is to make the overly simplistic default format enforcement code also check the DB for the default setting before giving up and setting it to OGG. But I am also going to change the default from OGG to FLAC, because that will be the least destructive action to take if all the other exceptions to setting the format happen.

Unless someone can talk me out of it, first .

[1audio]

I first encountered the bug, simply because its wasn't ripping to flac. And flac is what I think it should default to. There are other related issues on getting the right metadata in the database. And my dual drive powerfile seems to have other intermittent problems ripping that I need to look at. its just time consuming.
So a word of thanks for digging into this and a vote for flac.

[Matthew]

I first encountered the bug, simply because its wasn't ripping to flac. And flac is what I think it should default to. There are other related issues on getting the right metadata in the database. And my dual drive powerfile seems to have other intermittent problems ripping that I need to look at. its just time consuming.
So a word of thanks for digging into this and a vote for flac.

I've heard elsewhere in the community that 0710bN has some new bug that causes any bulk rip to abort after just creating a directory (maybe the first disc, maybe a parent for the bulk rip), and aborts before any actual ripping starts. I haven't tested since downloading, and my source is in pieces. Is your 0704 bulk rip behavior preserved in 0710bN?

[1audio]

I have not set up a 710 beta system. My 704 is working pretty well and I didn't want to risk it yet. I will be setting up a 710 test system in the next week or so and I'll connect the drive to see whats happening.

[Matthew]

However, the actual object that executes the ripDiskWrapper.sh script is a RipTask object that is contained (inside a RipJob object) in the Disk_Drive_Functions object. Which the Powerfile object sends to, and which any disc management object will send to for ripping. The right place for the rip format default is in the RipTask or the RipJob, which should know about its own rip job parameters. However, the current default code in the Powerfile class that converts empty format properties into "ogg" prevents the RipTask or RipJob from detecting whether it's "ogg" only because it went to default. That Powerfile code should be removed, and moved to the RipTask or RipJob class. And really, the default format shouldn't be OGG, because it's lossy. Since FLAC is a lossless compressed format, I'd like to make the default FLAC. So the Orbiter can set the format, or else the RipJob sets it to the Installation Settings in the DB, or else the RipJob sets it to FLAC.

Looking more carefully at the RipJob and RipTask classes, it looks like they are just managing the hardware, not performing any logic on the rip itself, which is all in the Powerfile object. Which is wrong - the ripping operation itself should manage its defaults, even if other objects like the Orbiter that have more immediate data (like user override when requesting the rip) that should override the default. But the whole architecture is wrong. The Powerfile is the hardware executing the ripping, and the RipJob and RipTask are abstractions that should be calling the Powerfile's hardware. The Powerfile should know whether it uses ripDiskWrapper.sh or some other script that knows what code actually runs that rippable object's HW.

But I'm not going to fix the architecture all at once just to ensure the defaults are called by the right object. That will just make fixing it later harder, because the code has been further shuffled around the wrong objects in the wrong architecture. I'm going to make the minimum revision, which is to make the overly simplistic default format enforcement code also check the DB for the default setting before giving up and setting it to OGG. But I am also going to change the default from OGG to FLAC, because that will be the least destructive action to take if all the other exceptions to setting the format happen.

Unless someone can talk me out of it, first .

OK, nobody talked me out of it, but I think I'm going to insert the format setting lookup into the Orbiter code anyway. It doesn't belong there, but the Orbiter already has infrastructure for querying the DB. The RipTask class doesn't have any DB access, and when I try to add calls to db_wrapper_query_result() either in the DCE or DB_Wrapper namespace/classes, the build fails unable to resolve the symbol (even with the .h files #included). At least it will work. If I can't find out how to make the RipTask access the DB, I'll just make the Orbiter do it. And refactor rhese encapsulated properties properly later.