Constant rate factor encoding in HandBrake's cli

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.

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

Post by dynaflash »

ABR = Average Bit Rate
CQP = Constant Quantizer (I forget what the P stands for)
CRF = Constant Rate Factor
Okay, so :

ABR should paralell to the same in the GUI version.
CRF should paralell to the Constant Quality Radio slider in the GUI
CQP should paralell to the Target Size in the GUI

I will vett this out, but thanks for the clarification. I am brushing up on using the cli version now so I can better understand when a developer discusses these terms,etc. Even though I use the GUI it should help with future development participation.
rhester
Veteran User
Posts: 2888
Joined: Tue Apr 18, 2006 10:24 pm

Post by rhester »

jbrjake,

Baseline 3.0 doesn't have a hard requirement on vbv_buffer. In fact, there's much debate about what an appropriate value should be (since the potential target devices vary so widely).

You were dead-on about the changes to ABR handling in x264 that led to the changes in the HandBrake core in revision 70.

I believe the CRF-default issue counts as a bug. :)

vbv_max_bitrate and vbv_bufsize were set to what at the time were thought to be sane values specifically for the iPod Video firmware 1.2. In hindsight, full compatibility with the iPV relies more on qcomp being 0 than setting vbv_max_bitrate (unless you artificially limit vbv_bufsize to something impossibly small). If you don't believe me that qcomp must be flat, try encoding any HBO DVD title and watch what happens to the 'snow' at the beginning. More correctly, watch what happens to the iPod. =)

I believe Chris may have interpreted the rate constant as a flag rather than a value, but I'm not sure why it's set to 1. It shouldn't be.

The single-pass vs. multipass ABR issue is also a bug - it was simply overlooked, as the vast majority of testing was in 2-pass mode (because we were originally crashing altogether on the start of the second pass).

Hope this bit of history helps.

Rodney
jbrjake
Veteran User
Posts: 4805
Joined: Wed Dec 13, 2006 1:38 am

Post by jbrjake »

dynaflash wrote: Okay, so :

ABR should paralell to the same in the GUI version.
CRF should paralell to the Constant Quality Radio slider in the GUI
CQP should paralell to the Target Size in the GUI
Almost.

ABR should be the same as in the GUI.
CRF shouldn't be used right now (following rhester's advice above to focus on folding in the new x264 before adding features).
CQP should be the same as the Constant Quality radio slider.

CRF and CQP are two different ways of getting constant quality. The difference is CQP is sorta dumb (it doesn't look at the video it's encoding or adjust compression based on what's going on in the frame) and CRF is sorta smart (it does adjust how much it compresses, based on motion).

Average Bit Rate and Target Size are actually the exact same thing from two different perspectives. A target size is the same as multiplying the average bitrate (the number of kb used per second) by how long the video is in seconds. From the other size, average bitrate is the size of the video in kb divided by how many seconds long it is.

(The values won't be exactly the same, because video encoders think there are 1000 b in a kb and your hard drive knows there are really 1024 b in every kb.)

Oh yeah, and thanks, rhester. That history was helpful and made things a little clearer for me. And, lacking a video iPod, I'm totally ignorant of the issues with that, so the qcomp issue is an eye-opener.
dynaflash
Veteran User
Posts: 3820
Joined: Thu Nov 02, 2006 8:19 pm

Post by dynaflash »

jbrjake

Thanks for clearing that up for me. I really appreciate it.

Hopefully the new x264 will be able to be folded into the rev70 code okay. But, alas, I dont think I am the one to do it. I quite obviously am over my head at the moment.

The possiblities are awesome for this project and I hope we can keep it moving forward.
rhester
Veteran User
Posts: 2888
Joined: Tue Apr 18, 2006 10:24 pm

Post by rhester »

To help things along, I've put the Dec 9 snapshot (CVS internal build 604, external build 54) on my web server in ready-to-use form for editing the version and Jamfiles with. All that needs to be done beyond that is updating the x264 patchfile to match the new trunk - which should mostly apply as-is, but I expect a few things will fall out, and then a new patch file should be generated.

The path to build 604 is http://multics.dynalias.com/handbrake/c ... 604.tar.gz and it's been packed with the same structure as past builds, so it should be largely plug-and-play.

Hope this helps.

Rodney
jbrjake
Veteran User
Posts: 4805
Joined: Wed Dec 13, 2006 1:38 am

Post by jbrjake »

rhester wrote:To help things along, I've put the Dec 9 snapshot (CVS internal build 604, external build 54) on my web server in ready-to-use form for editing the version and Jamfiles with. All that needs to be done beyond that is updating the x264 patchfile to match the new trunk - which should mostly apply as-is, but I expect a few things will fall out, and then a new patch file should be generated.
Awesome! Thank you, rhester. I'll try editing the version file on a clean copy of the source tonight and see how it goes. With it in tar.gz format, I don't believe any Jamfile changes will be necessary. It should only take a slight modification to libhb/encx264.c to work.

Where did the x264 patch file come from, by the way? I was looking at that last night, trying to figure out what it does. Any chance a fix was merged into the x264 trunk? On the x264 trac site, I see titer made a slight change to x264/common/i386/i386inc.asm 3 weeks ago for MacIntels. It's different from the changes in the patch, though.

Tonight I'll put together a libhb/encx264.c patch file that only changes to the new x264 core without any of the crf stuff in the diffs I pasted above. I'll also try to do the busywork of revising the MacIntel patch (I think the only reason it'd fail to patch the new core would be line number changes) , though without an x86 box to test on, I won't be able to confirm it works.
rhester
Veteran User
Posts: 2888
Joined: Tue Apr 18, 2006 10:24 pm

Post by rhester »

The r69 x264 patches were by titer. The r70 x264 patches were upleveled and enhanced for x264 build 50 by Chris Long, but I believe were incomplete w.r.t. MacIntel.

Rodney
jbrjake
Veteran User
Posts: 4805
Joined: Wed Dec 13, 2006 1:38 am

new x264 patch for rev 70

Post by jbrjake »

Here we go...made one small patch to libhb/encx264.c, switched out the URL in the x264 version file, ./configured, and it jammed on the first try. Sooooo much smoother than changing the x264 by hand. Thanks again, rhester.

Both ABR and CQP encoding methods work correctly with the build.

Patch against libhb/encx264.c:

Code: Select all

--- HandBrake-r70-clean/libhb/encx264.c.bak.c	2006-12-14 19:04:29.000000000 -0500
+++ HandBrake-r70-clean/libhb/encx264.c	2006-12-14 19:10:27.000000000 -0500
@@ -78,33 +78,23 @@
     if( job->vquality >= 0.0 && job->vquality <= 1.0 )
     {
         /* Constant QP */
+		param.rc.i_rc_method = X264_RC_CQP;
         param.rc.i_qp_constant = 51 - job->vquality * 51;
         hb_log( "encx264: encoding at constant QP %d",
                 param.rc.i_qp_constant );
     }
     else
     {
-
 		/* Rate control */
-		/* no longer in x264 - see rc.i_rc_method in x264.h */
-        /* param.rc.b_cbr     = 1; */
-		
-		/* these were the only settings I could use to get accurate ending video bitrate */
-		param.rc.i_rc_method    =  X264_RC_CRF;
-		param.rc.i_vbv_max_bitrate = job->vbitrate;
-        param.rc.i_vbv_buffer_size = 224;
-		param.rc.i_rf_constant = 1;
-		
+		param.rc.i_rc_method    =  X264_RC_ABR;
         param.rc.i_bitrate = job->vbitrate;
         switch( job->pass )
         {
             case 1:
-				param.rc.i_rc_method    =  X264_RC_ABR;
                 param.rc.b_stat_write  = 1;
                 param.rc.psz_stat_out = pv->filename;
                 break;
             case 2:
-				param.rc.i_rc_method    =  X264_RC_ABR;
                 param.rc.b_stat_read = 1;
                 param.rc.psz_stat_in = pv->filename;
                 break;
And while I doubt this one line diff will save anyone time, here's what changed in the version file:

Code: Select all

--- version_x264.txt.bak	2006-12-14 19:18:05.000000000 -0500
+++ version_x264.txt	2006-12-14 19:18:18.000000000 -0500
@@ -1 +1 @@
-http://multics.dynalias.com/handbrake/contrib/x264-r568.tar.gz
+http://multics.dynalias.com/handbrake/contrib/x264-r604.tar.gz

Of course, this is all on a G5. Next up is revising that x264 MacIntel patch, I guess.
dynaflash
Veteran User
Posts: 3820
Joined: Thu Nov 02, 2006 8:19 pm

Post by dynaflash »

Worked Great for me as well, will try a couple encodes today with the gui and see who it works.

Thanks a bunch rhester for getting that new x264 merged, etc.

jbr, thanks again for all of the nuts and bolts pointers.
SndChsr
Posts: 20
Joined: Thu Nov 30, 2006 1:53 pm

Post by SndChsr »

Wow, I disappear for a few days and you guys really push forward. Great work!

I still haven't heard from Chris Long, but I'll send him a quick email reminder. When I get a response from him, I'll pass it on to you guys, since you really seem to have a better handle on the x264 encoding than I do. I'm learning alot by reading your posts, and still want to contribute where I can, but jbrjake seems like the one with the most understanding of the x264 issues.

I'm still happy to do any Intel testing that needs to be done as well. I'll check out the snapshot rhester put up and try that out on my machine. Again, great work so far.
jbrjake
Veteran User
Posts: 4805
Joined: Wed Dec 13, 2006 1:38 am

macIntel x264 patch

Post by jbrjake »

Took a look at the x264 macIntel patch just now...

It patches against two files: x264/common/i386inc.asm and x264/common/predict-a.asm. The second one works still, the first one is rejected.

You know how I said in a post above that titer made a minor change to x264 for MacIntels a couple of weeks back? In the old version, line 44 of x264/common/i386/i386inc.asm was:

Code: Select all

SECTION .text
And now it's:

Code: Select all

SECTION .text align=16
The patch is designed to comment out that line, forcing MacIntels to use:

Code: Select all

SECTION .rodata data align=16
By the way--the code's comments say "On OS X we cannot use .rodata because NASM is unable to compute address offsets outside of .text so we use the .text section instead until NASM is fixed." Yet using .rodata on OS X is exactly what the patch forces MacIntels into doing? Could titer's simple addition of "align=16" work instead?

I'm totally out of my league on this sort of stuff, so I'll assume the way the patch currently works is correct.

As such, the only change required to the patch for it to run on the new x264 is to add "align=16" to the end of line 8. Here's a paste of the revised patch:

Code: Select all

--- x264/common/i386/i386inc.asm	2006-09-23 14:01:05.000000000 -0400
+++ x264-patched/common/i386/i386inc.asm	2006-09-24 09:21:27.000000000 -0400
@@ -40,12 +40,12 @@
 ; is unable to compute address offsets outside of .text so we use the .text
 ; section instead until NASM is fixed.
 %macro SECTION_RODATA 0
-    %ifidn __OUTPUT_FORMAT__,macho
-        SECTION .text align=16
-        fakegot:
-    %else
+;    %ifidn __OUTPUT_FORMAT__,macho
+;        SECTION .text align=16
+;        fakegot:
+;    %else
         SECTION .rodata data align=16
-    %endif
+;    %endif
 %endmacro
 
 ; PIC support macros. All these macros are totally harmless when __PIC__ is
--- x264/common/i386/predict-a.asm	2006-09-23 14:01:05.000000000 -0400
+++ x264-patched/common/i386/predict-a.asm	2006-09-24 09:21:27.000000000 -0400
@@ -191,7 +191,6 @@
 %assign Y (Y-1)
     movq        [edx + Y*FDEC_STRIDE], mm1
 
-    picpop      ebx
     ret
 
 ;-----------------------------------------------------------------------------
@@ -228,7 +227,6 @@
 %assign Y (Y-1)
     movq        [edx + Y*FDEC_STRIDE], mm0
 
-    picpop      ebx
     ret
 
 ;-----------------------------------------------------------------------------
@@ -269,8 +267,7 @@
 %endrep
     movq        [edx +  Y   *FDEC_STRIDE], mm3
     movq        [edx + (Y+1)*FDEC_STRIDE], mm0
-
-    picpop      ebx
+    
     ret
 
 ;-----------------------------------------------------------------------------

While the patch successfully applies to the source, I have no Intel Macs and can't guarantee that it actually compiles, though I see no reason why it wouldn't.
dynaflash
Veteran User
Posts: 3820
Joined: Thu Nov 02, 2006 8:19 pm

Post by dynaflash »

Maybe if SndChsr is up to it, he can get up to speed with where we are and try it. Alas, I am on ppc as well so have applied it as have you but cannot test it.

PPC seems smooth in my testing so far.

Are we at a point where we submit a rev 71 to the trunk with the new x264 without the intel patch and then verify the intel patch for a rev 72 ? Or does it make sense to have someone verify your additions for Intel and if they fly check in the whole new x264 ppc and intel all in one rev 71 update.

I may be off on this, but does it reflect rhesters suggestion for revs on the trunk ?

I am psyched how the new x264 built out of the box for ppc. I did get a hang at the end of encoding on one test, but I think it may have had more to do with running about a million apps here at the same time as HB. Had work to do but was fired up to run a test encode. Dont have my iPod here so cant verify if the new revisions have broken the h.264 3.0 iPod encoding that worked before the new x264. But, be sure I will iPod test asap.

Now were cooking with gas! jbrjake, your input has really moved this along as well as well as rhester finding some time to help out as well.
SndChsr
Posts: 20
Joined: Thu Nov 30, 2006 1:53 pm

Post by SndChsr »

I'd be happy to compile and test anything you need on my Intel Mac. You may just need to help me through the process a bit. :-) Let me know what I can do to help.
rhester
Veteran User
Posts: 2888
Joined: Tue Apr 18, 2006 10:24 pm

Post by rhester »

Just my opinion, but I'd like to see all releases compile cleanly (if not execute cleanly) on all "supported" platforms. It's going to be confusing to people to hear "OK, revision 71 is for Linux and PPC, revision 72 is for MacIntel, but otherwise there are no differences"...or, worse, there _are_ functional changes as well.

If it were my call, I'd get what will ultimately be revision 71 at least cleanly compiling on Linux, PPC, and MacIntel before check-in and focus on functionality and bugfixes in revision 72.

Rodney
SndChsr
Posts: 20
Joined: Thu Nov 30, 2006 1:53 pm

Post by SndChsr »

What's the easiest way for me to get the potential r71 source, patches and all, and compile on my Intel machine? I've seen the patches scattered throughout posts on this board, but do we have a set I can download and test out somewhere? Thanks.

I agree that any code checked in should work on all supported platforms. Bugs may (and do) exist in certain platforms, but the code should at least compile cleanly and be able to be tested by people wishing to contribute.
Last edited by SndChsr on Fri Dec 15, 2006 5:40 pm, edited 1 time in total.
dynaflash
Veteran User
Posts: 3820
Joined: Thu Nov 02, 2006 8:19 pm

Post by dynaflash »

Okay, well, that makes sense, I misinterpreted your ideas originally on the way to handle the revs. Sorry.

SndChsr, on the page before this is the steps jbrjake and I took for an easy compile for ppc. I think you would want to follow those steps plus try the additional steps he states at the top of this page for Intel.

Correct me if I am wrong on this jbr or rhester. I did it by checking out a clean rev 70 from the trunk into a new directory and making the couple of changes jbr suggests. As he said, jammed perfectly for ppc.

Then try what he says above. And see if it flys.

Anyways, if i had an intel mac, thats what I would try.

No Linux box here as well so I cant test that either.
SndChsr
Posts: 20
Joined: Thu Nov 30, 2006 1:53 pm

Post by SndChsr »

Thanks dynaflash. I'll give that a try right now.
jbrjake
Veteran User
Posts: 4805
Joined: Wed Dec 13, 2006 1:38 am

Post by jbrjake »

SndChsr wrote:What's the easiest way for me to get the potential r71 source, patches and all, and compile on my Intel machine? I've seen the patches scattered throughout posts on this board, but do we have a set I can download and test out somewhere? Thanks.
Well, we don't have a patched source up yet on a server. Here's a quick step-by-step to get it up and running. It looks longer and more complicated than it is. Mostly just light cutting and pasting.

1. Download a clean copy of the source

Code: Select all

/opt/local/bin/svn co svn://multics.dynalias.com/HandBrake/trunk HandBrake-r70
2. Apply the libhb/encx264.c patch I pasted into this post:
(For any fellow n00bs following along at home, apply the patch by pasting it into a new document (call it, say, encx264.patch), saving it in the main source directory (HandBrake-r70 following my example svn command in step 1), and entering this command in that main directory:

Code: Select all

patch < encx264.patch
That should work fine, but if it says it can't find the file, just specify it as:

Code: Select all

libhb/encx264.c
Also, make sure all patch files end with a blank new line (a carriage return). )

3. Replace the contents of contrib/version_x264.txt with this web address:
4. Replace the contents of contrib/patch-x264-macintel.patch with the slight revision I pasted into this post:
EDIT oops I pasted the wrong link, here's the right one:
5. ./configure

6. jam
Last edited by jbrjake on Fri Dec 15, 2006 6:51 pm, edited 1 time in total.
dynaflash
Veteran User
Posts: 3820
Joined: Thu Nov 02, 2006 8:19 pm

Post by dynaflash »

jbr, thanks for summarizing what we did.

I cant speak for SndChsr, but I was not really sure how to apply the code you posted but figured out how to modify my files straight away to reflect your changes. So my files are actually changed to reflect your patches manually. Again, coming from a Perl world, I wasnt sure exactly how to apply them at first.

That was my only reason for suggesting a new already patched rev so someone like SndChsr could run a quick clean build to verify on Intel since neither of us has one.

SndChsr - Hopefully what jbrjake told you makes sense to you and you can patch and verify for Intel. So far, following Jbrs instructions has been very successful for me.
SndChsr
Posts: 20
Joined: Thu Nov 30, 2006 1:53 pm

Post by SndChsr »

OK. Got the build working, everything's great so far. I'm going to do some encoding tests and I'll report back with my results. Thanks for the help with the build process, the instructions were easy to follow.
dynaflash
Veteran User
Posts: 3820
Joined: Thu Nov 02, 2006 8:19 pm

Post by dynaflash »

Awesome! Hope it works out. Now I suppose we will need a Linux test.
rhester
Veteran User
Posts: 2888
Joined: Tue Apr 18, 2006 10:24 pm

Post by rhester »

jamming now on Linux :) Will post results later tonight, holiday party =)

Good job to everyone on this - if mine clean-compiles, who would like the honor of checking this in? Obviously, there are only three file changes from revision 70, so it's _really_ simple.

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

Post by rhester »

Good news on Linux:

Archive libhb/libhb.a
ar: creating libhb/libhb.a
Ranlib libhb/libhb.a
Cc test/test.o
Link HBTest
Chmod1 HBTest
...updated 57 target(s)...
rhester@debbie:~/HandBrake-r70$ ls -l HBTest
-rwxr-xr-x 1 rhester rhester 8382146 2006-12-15 16:49 HBTest
rhester@debbie:~/HandBrake-r70$ ./HBTest
Missing input device. Run ./HBTest --help for syntax.

So am I correct in my understanding that the release 71 candidate does indeed compile successfully on PPC, MacIntel and Linux? (Anyone have a BeOS install they want to roll the dice with? ;)

On a more serious note - I'm sort of thinking BeOS support should be discontinued, at least temporarily. I know it held/holds a soft spot in titer's heart, but in reality, I haven't been able to locate a single binary of any HandBrake release for BeOS, much less an actual user.

Rodney
SndChsr
Posts: 20
Joined: Thu Nov 30, 2006 1:53 pm

Post by SndChsr »

rhester, it sounds like you probably have the most knowledge of svn, so you should probably check it in.

My tests are looking ok so far, I did a couple small files on the intel system. Working on a full movie now, I'll post the results when its finished.
rhester
Veteran User
Posts: 2888
Joined: Tue Apr 18, 2006 10:24 pm

Post by rhester »

Modified Jamrules to change the version to 0.7.1a2.

dynaflash, it looks like some of the GUI files have versions as well:

macosx/.svn/text-base/HandBrake.plist.svn-base
macosx/HandBrake.xcodeproj/.svn/text-base/project.pbxproj.svn-base
macosx/HandBrake.xcodeproj/project.pbxproj
macosx/HandBrake.plist

Should those be updated to match?

Regarding checkin:

My personal feeling is that checkins should not be done until the latest tree compiles cleanly on all platforms - but I'm not as sure that we should wait for actual user testing (even if only by the developers) prior to checkin. From my perspective, the sooner you can get code out in front of other developers, the better - and all that's needed is a clean compile. Bugs will be present at runtime, that is to be expected, and those can and should be corrected in future releases, but I don't know that I'd like to see checkins held up waiting for user testing.

For instance, last time I looked, ffmpeg encodes with revision 70 failed outright, at least on Linux. But the code did compile, and rather than have me be the only one looking at the problem, it seemed like a better idea to get it checked in so that other knowledgable folks could see what I was seeing on their own local source trees and possibly contribute a solution.

Thoughts welcomed.

Rodney
Post Reply