DriveDetector

Archive of historical development discussions
Discussions / Development has moved to GitHub
Forum rules
*******************************
Please be aware we are now using GitHub for issue tracking and feature requests.
- This section of the forum is now closed to new topics.

*******************************
Post Reply
dynaflash
Veteran User
Posts: 3820
Joined: Thu Nov 02, 2006 8:19 pm

DriveDetector

Post by dynaflash »

Just an update, I just compiled rev69 (titers most recent at the official svn) and, voila, Detecting the optical drive does NOT work on that rev. so its no surprise that it doesnt work on rev.70 > here.

However, if I use the posted binary off of the HB site it does work.

I am now trying some regression testing on builds back to titers last posted change change (rev66) which is right before where he notes changes in rev68 regarding "Fixed Scanning of DVD Folders".

I wonder if the binary build posted on his website reflects source files other than or earlier than rev.69?

Actually, I may try back to 64 where his notes say "Show the name of DVD instead of /ref/rdiskX". As that is exactly what the most current posted binary of HB off the HB site says.
dynaflash
Veteran User
Posts: 3820
Joined: Thu Nov 02, 2006 8:19 pm

Post by dynaflash »

Started my regression testing and decided to start with rev63 right off of the official handbrake svn. This is the last rev before 64 where Titers notes state "Show the name of the DVD instead of /dev/rdiskx" wich is exactly what the binary downloaded from the official site does. I think this is the build that is in the binary.

Results: DriveDetector works!

So, I am now working upwards and am currenly building rev64 which is where titers svn notes says the it is supposed to show the DVD name.

If that one crashes, then I will start there in figuring out the problem with the DriveDetector.

In the meantime, maybe we should see if we dont want to implement the working drivedetector (newest working rev) into a new rev 72 so we can all start working from a platform that doesn crash the app on drive detection.

What are your thoughts on this idea ?
dynaflash
Veteran User
Posts: 3820
Joined: Thu Nov 02, 2006 8:19 pm

Post by dynaflash »

Okay tested fresh built of rev64.

Crashes on drive detection same as everyone before it.

So, we now know why ours crashes. Near as I can tell, the official binary up on the HB website is a build of rev63 or at least has the DriveDetector portion of rev63.

So, I think to troubleshoot, I will try manually patching DriveDetector from 63 into our rev71 and see if it works. If so, then we can see about properly adding the DVD name feature that 64 supposedly added (although apparently unsuccessfully, at least for the Mac gui). As a side note, there were also later notes like, for rev67 is says "Fixed scanning of DVD folder". I am not sure what this fix is, but I think we should start back on rev 63 and fix the crashing issue first.

I will try to get to this later tonight, but the holidays might slow me up.
mk2000
Posts: 28
Joined: Mon Dec 18, 2006 3:38 am

Post by mk2000 »

Good work on this investigation DynaFlash!

If we can iron out that issue, it will be a home run hit!

mk2000
dynaflash
Veteran User
Posts: 3820
Joined: Thu Nov 02, 2006 8:19 pm

Post by dynaflash »

No doubt about it as far as I can see. The binary that probably 90% of users download from the official HB site is rev63. The next six, including 69 which rhester and company took to move forward with our rev 70 + crash upon trying to detect the optical drive.

In retrospect, had we known that ( I assume that rhester figured that the binary everyone was using was compiled from the latest in the svn, which I would have) it may have been smart to move to our new platform from titers rev63. But, alas, that is not the road we took, near as I can tell.

So, in summary, we inherited the whole drive detection bug from titers last rev69 (and the five or so preceding it).

Near as I can tell, the files that changed (have the bug code) from 63 (last rev where drive detection works on gui) are all mac gui files except for hb.h and dvd.c now knowing that the drive detection does work on mac using the CLI version, I have to assume that the errors exist in how the mac GUI files (I think particularly DriveDetector.m) interacts with the libhb files.

So, for the interim, we can either try to fix the gui files or use the dvd.c file from 63 (which I dont think changed after that) and then try to insert rhesters changes made to hb.h which gave us the new h.264 baseline 3.0 format into the rev63 hb.h.

Either way, there may not be a quick fix. My guess is that titer knew that there was a problem with the mac gui from 63 to 64 and so even though the svn goes to 69, and those features work in the CLI. For the mac binary which we know as version 0.7.1 he actually compiled rev63 from the svn.

Being the holiday weekend, I may not get this sorted out in the next few days. Anyone have any ideas please jump on board. The above analysis is as far as I am right now.

Have tried several different combinations of replacing gui files in the hopes that there was a quick fix, but, alas, it will take a bit of coding.

Will work on it as time permits over the weekend. I have switched from my new gui to getting this resolved so at least the new gui doesnt inherit and existing problem in the code base.

Ideas/Thoughts/Help ?
jbrjake
Veteran User
Posts: 4805
Joined: Wed Dec 13, 2006 1:38 am

Post by jbrjake »

I've been doing some diffs in FileMerge this morning to try to help you out, dynaflash.

Have you tried downloading the 0.7.1. source tarball from the official HandBrake download page ( http://download.m0k.org/handbrake/HandB ... 7.1.tar.gz ) instead of using titer's svn? I'm finding some big differences between it and the code in titer's svn trac. This would seem to verify what you discovered, about titer using an older source to make his last release instead of using his latest code.

The whole DriveDetector.m file is new. It looks like it used to be part of ScanController.mm. If I was going to bet, I'd say the problem is related to how the drive name went from being stored in drivesList to fDrives.

From ScanController.mm source from titer's last binary:

Code: Select all

NSMutableArray * drivesList; 
    drivesList = [NSMutableArray arrayWithCapacity: 1];

    next_media = IOIteratorNext( media_iterator );
    if( next_media )
    {
        char psz_buf[0x32];
        size_t dev_path_length;
        CFTypeRef str_bsd_path;
        do
        {
            str_bsd_path =
                IORegistryEntryCreateCFProperty( next_media,
                                                 CFSTR( kIOBSDNameKey ),
                                                 kCFAllocatorDefault,
                                                 0 );
            if( str_bsd_path == NULL )
            {
                IOObjectRelease( next_media );
                continue;
            }

            snprintf( psz_buf, sizeof(psz_buf), "%s%c", _PATH_DEV, 'r' );
            dev_path_length = strlen( psz_buf );

            if( CFStringGetCString( (CFStringRef) str_bsd_path,
                                    (char*)&psz_buf + dev_path_length,
                                    sizeof(psz_buf) - dev_path_length,
                                    kCFStringEncodingASCII ) )
            {
                [drivesList addObject:
                    [NSString stringWithCString: psz_buf]];
            }

            CFRelease( str_bsd_path );

            IOObjectRelease( next_media );
DriveDetector.m from our r71:

Code: Select all

 [fDrives removeAllObjects];

    next_media = IOIteratorNext( media_iterator );
    if( next_media )
    {
        char * name;
        char psz_buf[0x32];
        size_t dev_path_length;
        CFTypeRef str_bsd_path;
        do
        {
            str_bsd_path =
                IORegistryEntryCreateCFProperty( next_media,
                                                 CFSTR( kIOBSDNameKey ),
                                                 kCFAllocatorDefault,
                                                 0 );
            if( str_bsd_path == NULL )
            {
                IOObjectRelease( next_media );
                continue;
            }

            snprintf( psz_buf, sizeof(psz_buf), "%s%c", _PATH_DEV, 'r' );
            dev_path_length = strlen( psz_buf );

            if( CFStringGetCString( (CFStringRef) str_bsd_path,
                                    (char*)&psz_buf + dev_path_length,
                                    sizeof(psz_buf) - dev_path_length,
                                    kCFStringEncodingASCII ) )
            {
                if( ( name = hb_dvd_name( psz_buf ) ) )
                {
                    [fDrives setObject: [NSString stringWithCString: psz_buf]
                        forKey: [NSString stringWithCString: name]];
                }
            }

            CFRelease( str_bsd_path );

            IOObjectRelease( next_media );
dynaflash
Veteran User
Posts: 3820
Joined: Thu Nov 02, 2006 8:19 pm

Post by dynaflash »

The mystery deepens. Well, I wont have too much time to work on it for the next day or two, but will be sure to tune in here as I can to see if youve made any discoveries or progress.

Once again, thanks for jumping in jbrjake. As always, your input and work are excellent.
dynaflash
Veteran User
Posts: 3820
Joined: Thu Nov 02, 2006 8:19 pm

Post by dynaflash »

Am just heading out. But, I did get drive detection working on our release, but going to Picture Settings crashes it off of he optical drive.

Will update later tonight.
dynaflash
Veteran User
Posts: 3820
Joined: Thu Nov 02, 2006 8:19 pm

Post by dynaflash »

Not much time to update, got intial drive detection to work as well as ripping. BUT, it appears that when you hit "Picture Settings" button and HB tries to access the optical drive to generate the picture resolution settings in the Picture Screen, HB crashes. So, I guess I am partway there.

Frustrating. I am wondering if we are not better off trying to the changes rhester and company made to titers rev 69 to 63 and just bypass titers changes he made from 64 to 69 and just start from there.

Cannot say totally how much that would entail, but it may be the shorter route. Plus, there must be a reason titer never distributed a binary of 64 - 69.

Plus, we can always go back later and try to analyse what was wrong with the "use DVD name instead of /rev... for dvd name" upgrade that apparently didnt work right anyway, at least not for the gui.

From people downloading the binary perspective, it would be transparent as the features that titer notes in the svn between 64 and 69 were never in the official 0.7.1 binary release anyway.

Which route do you guys think is best ?
mk2000
Posts: 28
Joined: Mon Dec 18, 2006 3:38 am

Post by mk2000 »

dynaflash wrote:Not much time to update, got intial drive detection to work as well as ripping. BUT, it appears that when you hit "Picture Settings" button and HB tries to access the optical drive to generate the picture resolution settings in the Picture Screen, HB crashes. So, I guess I am partway there.

Frustrating. I am wondering if we are not better off trying to the changes rhester and company made to titers rev 69 to 63 and just bypass titers changes he made from 64 to 69 and just start from there.

Cannot say totally how much that would entail, but it may be the shorter route. Plus, there must be a reason titer never distributed a binary of 64 - 69.

Plus, we can always go back later and try to analyse what was wrong with the "use DVD name instead of /rev... for dvd name" upgrade that apparently didnt work right anyway, at least not for the gui.

From people downloading the binary perspective, it would be transparent as the features that titer notes in the svn between 64 and 69 were never in the official 0.7.1 binary release anyway.

Which route do you guys think is best ?
I would say that we should go back to rev 63 and implement rhester's changes from there.

If we can release something stable, it will help keep HandBrake's market share (otherwise, people might lose interest).

Once we have something stable, we can look at where titer was heading with the Rev 64-69 changes.


mk2000
rhester
Veteran User
Posts: 2888
Joined: Tue Apr 18, 2006 10:24 pm

Post by rhester »

There were far too many under-the-hood infrastructural changes to the HandBrake source between revisions 63 and 69 to have much hope of backporting the 640x480 L3 changes to revision 63...I tried. You're possibly better off trying to "forward port" revision 63's DVD handling to revision 71.

Rodney
dynaflash
Veteran User
Posts: 3820
Joined: Thu Nov 02, 2006 8:19 pm

Post by dynaflash »

Loud and clear on that, rhester, you would be the one to know as you did all of the changes.

At least that gives us the right direction. I will try to work on as I can over the next couple of days.

Thanks for jumping in rhester. Have a happy holiday.
dynaflash
Veteran User
Posts: 3820
Joined: Thu Nov 02, 2006 8:19 pm

Post by dynaflash »

Okay, Here is a shot of Rev 71 running the new gui with drive detection working:

Image

This shot is me running my first test off of the optical disc. I want to continue testing for a bit and make sure I didn't overlook something.

I think we are a step closer ...
dynaflash
Veteran User
Posts: 3820
Joined: Thu Nov 02, 2006 8:19 pm

Post by dynaflash »

Okay, the h.264 level 3.0 rip above worked out great and all of the PicturController functions were super! Detected the drive in a big way!!

Incidentally, just to make sure, I did transfer the clip to my ipod to make sure nothing got messed up in rhesters h264 level 3 code (though I had no reason to think it did).

One other advantage to rhesters suggestion of porting the 63 scan controllers and drive detectors to our newer rev 71 is that we take advantage (I think) of a change titer made in rev 68 where his notes say: "Adds a 'colr' box to mp4 files, indicating an NTSC profile. Makes the
picture look brighter in QuickTime." which is an upgrade to patch-ffmpeg.patch and Jamfile which should make the mpeg 4 output brighter, which has been a long standing issue for some with handbrakes mpeg 4 output. I dont think the Official Binary 0.7.1 (which again, I believe to be rev 63) took advantage of that. I ran my own test of our rev 71 mpeg output with the same scene/settings in the rev 63 and 71 does look brighter (whether or not thats the power of suggestion is up to debate).

So, if some more of my testing on several discs pans out. How best to get this out to the group?
jbrjake
Veteran User
Posts: 4805
Joined: Wed Dec 13, 2006 1:38 am

Post by jbrjake »

Shiny!

This is incredibly cool, dynaflash. The new GUI, working dvd read, and better color reproduction totally justify a binary release, imo.

What did you end up having to change to port r63's controllers?
mk2000
Posts: 28
Joined: Mon Dec 18, 2006 3:38 am

Post by mk2000 »

That sounds great!

I'm sure you guys already know how to do this, but from the command line with the Apple Developer Tools installed, using the lipo command can merge a PPC and Intel Binary together to form a Universal Binary:

The usage would be:

Code: Select all

lipo -create [inputPPCfile1 inputINTELfile2] -output [outputUBfile]
Example:

Code: Select all

lipo -create /Applications/HandBrake-0.7.1a2-PPC/HandBrake.app/Contents/MacOS/HandBrake /Applications/HandBrake-0.7.1a2-INTEL/HandBrake.app/Contents/MacOS/HandBrake -output HandBrake
Then just reinsert the output HandBrake binary back into a HandBrake application bundle (replace existing binary).

Probably do a duplicate command from the Finder on the Application bundle to force the Finder to recognize the resulting duplicate as a Universal Binary.

Done!

Once again, I apologize if this does not contribute to anyone's current knowledge.

mk2000
dynaflash
Veteran User
Posts: 3820
Joined: Thu Nov 02, 2006 8:19 pm

Post by dynaflash »

Cool. Did not know that (never even looked into how to do a universal binary).

I suppose first we should get it into the svn properly and have some of you other developers check it out.

Since I departed from my gui user prefs, etc, and started on the drive detector issue (seemed more important at the time), I still have to get my prefs code into the new drive enabled rev 71 (not even sure how to refer to it anymore!) also seems that it should incorporate the new rev 72 change to Instant Handbrake that makes it iPod h264 480p capable.

Basically get everything back under one roof. What do you guys think?
dynaflash
Veteran User
Posts: 3820
Joined: Thu Nov 02, 2006 8:19 pm

Post by dynaflash »

Okay, not too sure how to summarize where I am and what you guys want me to do.

Handbrake: As I said a few posts ago. Handbrake is working just fine detecting drive and everything using rev 63 drive dector and Scan Controller. No problems there.

Instant Handbrake: johnallen submitted an updated Instant Handbrake as rev. 72 which uses the new Level 3.0 h.264 specs. I have since taken a look at that to glean the drive detector, etc. that it uses since Instant Handbrake shows the DVD name instead of the earlier /rev/ .... type indicator rev63 uses. In doing so, I started playing around a bit and instead of changing the existing iPod h.264, I added an extra menu item to let the user use pre 1.2 ipod firmware or post 1.2 firmware. SO, there are now two iPod h.264 settings you can choose from.

I am running a couple encodes now to see the picture size, etc. of the two outputs.

Anyway, so in summary, I have a Handbrake that has working drive detection using the rev63 stuff, and a newer h.264 Enhanced Instant Handbrake that uses the newer controller and both levels of h.264 (if my tests work out okay, which i think they will).

So, what to do and where to go from here ???

It now occurs to me that the newer DriveDetector and ScanController was assimilated and worked for the public release of Instant Handbrake (everything post rev63). Handbrake never did work right with it so, titer distributed the rev63 binary of Handbrake.

Ultimately, it would really be nice if they could use the same controller and drivedetector. But, until then, right now, I have both of them working good for what they are.

Any ideas, opinions, thoughts ????
johnallen
Experienced
Posts: 95
Joined: Sat Sep 30, 2006 8:52 pm

Post by johnallen »

Check it in. We'll give it a workout.
mk2000
Posts: 28
Joined: Mon Dec 18, 2006 3:38 am

Post by mk2000 »

Perhaps I'm just talking out of my butt, but maybe we could create a separate branch in the svn?

This branch would either be for the current state of technolgy in rev 72 or perhaps in can be for your changes regarding regular HandBrake that integrate rev63 technology.

This will give you time to later merge the branches but most importantly it will allow you to upload your changes so that the whole group can see it, build and test.

Once again, I don't claim too much knowledge on svn but I could swear that a different branch is a standard suggestion for the issue above.

mk2000
dynaflash
Veteran User
Posts: 3820
Joined: Thu Nov 02, 2006 8:19 pm

Post by dynaflash »

mk200, I tend to agree. I am a bit new to the svn thing as well, but it seems we have one set of basecode which can create three binary apps. But, at least one of them does not work right when they are created from the same base code revision.

Ultimately, as you have said, they should all be compatible from the same svn rev. But, they are not, and unless someone else wants to dive in and fix HB, I probably wont have that figured out for who knows how long.

Anyway, meanwhile, we kind of have two branches going in the svn rev system.

One thing about everyone checking everything they do in all at once that concerns me in terms of keeping on track of where we are going. As rhester said previously. In svn you cannot take back a checkin. You can only check another rev in to correct the one you checked in. This is why I am cautious about just checking in any and everything.

So, where does that leave us ? What do you guys think?
rhester
Veteran User
Posts: 2888
Joined: Tue Apr 18, 2006 10:24 pm

Post by rhester »

If it should be desirable, I'm happy to create a completely separate SVN trunk (HandBrake-NG?) for 'bleeding edge' stuff that would later be backported in, though it may ultimately prove to be more trouble than it's worth (to the developers trying to keep everything synchronized). Just another option on the table, not a suggestion.

Rodney
johnallen
Experienced
Posts: 95
Joined: Sat Sep 30, 2006 8:52 pm

Post by johnallen »

Guys,

I do have quite a bit of experience using svn. Branching is a good idea. However, getting to far out of date can causes difficulties when merging branches back into the trunk.

In the past, here are a few tips I have found that keeps merging less painful...

-Start your branch from the current version in the trunk.
-Update your branch with any new changes from the trunk on a regular basis. (daily)
-Try to keep the changes in the branch somewhat 'low impact'.
-Keep the goals of the branch simple, so that you can complete the changes and get them merged back into the trunk in reasonable time period. (say 2 to 3 weeks).
rhester
Veteran User
Posts: 2888
Joined: Tue Apr 18, 2006 10:24 pm

Post by rhester »

All good information.

As someone not-so-experienced with svn (I can keep my head above water, that's about it), is there anything required on my end to create a branch, or is it basically just a subtree underneath the trunk?

Rodney
Post Reply