[xiph-commits] r16452 - trunk/theora/lib

tterribe at svn.xiph.org tterribe at svn.xiph.org
Thu Aug 6 23:37:52 PDT 2009


Author: tterribe
Date: 2009-08-06 23:37:52 -0700 (Thu, 06 Aug 2009)
New Revision: 16452

Modified:
   trunk/theora/lib/encint.h
   trunk/theora/lib/rate.c
Log:
Code review of r16447.
Fix a 32-bit/64-bit bug.
Update the stat file version number.
Add back some saturation to prevent overflow.
Fix some other minor discrepancies.


Modified: trunk/theora/lib/encint.h
===================================================================
--- trunk/theora/lib/encint.h	2009-08-07 04:16:26 UTC (rev 16451)
+++ trunk/theora/lib/encint.h	2009-08-07 06:37:52 UTC (rev 16452)
@@ -188,9 +188,11 @@
 
 /*The 2-pass metrics associated with a single frame.*/
 struct oc_frame_metrics{
-  ogg_int64_t   log_scale; /* Q24; int64 to avoid casting each time we
-                              wish to convert back to Q57 */
+  /*The log base 2 of the scale factor for this frame in Q24 format.*/
+  ogg_int32_t   log_scale;
+  /*The number of application-requested duplicates of this frame.*/
   unsigned      dup_count:31;
+  /*The frame type from pass 1.*/
   unsigned      frame_type:1;
 };
 

Modified: trunk/theora/lib/rate.c
===================================================================
--- trunk/theora/lib/rate.c	2009-08-07 04:16:26 UTC (rev 16451)
+++ trunk/theora/lib/rate.c	2009-08-07 06:37:52 UTC (rev 16452)
@@ -88,7 +88,7 @@
   _f->y[1]=_f->y[0]=_f->x[1]=_f->x[0]=_value;
 }
 
-static ogg_int64_t oc_iir_filter_update(oc_iir_filter *_f,int _x){
+static ogg_int64_t oc_iir_filter_update(oc_iir_filter *_f,ogg_int32_t _x){
   ogg_int64_t c0;
   ogg_int64_t c1;
   ogg_int64_t g;
@@ -112,6 +112,8 @@
   return ya;
 }
 
+
+
 /*Search for the quantizer that matches the target most closely.
   We don't assume a linear ordering, but when there are ties we pick the
    quantizer closest to the old one.*/
@@ -190,21 +192,45 @@
 
 /*Binary exponential of _log_scale with 24-bit fractional precision and
    saturation.
-  _log_scale: A binary logarithm in Q57 format.
-  Return: The binary exponential in Q24 format.*/
-static ogg_int64_t oc_bexp_q24(ogg_int64_t _log_scale){
-  return oc_bexp64(_log_scale+OC_Q57(24));
+  _log_scale: A binary logarithm in Q24 format.
+  Return: The binary exponential in Q24 format, saturated to 2**47-1 if
+   _log_scale was too large.*/
+static ogg_int64_t oc_bexp_q24(ogg_int32_t _log_scale){
+  if(_log_scale<(ogg_int32_t)23<<24){
+    ogg_int64_t ret;
+    ret=oc_bexp64(((ogg_int64_t)_log_scale<<33)+OC_Q57(24));
+    return ret<0x7FFFFFFFFFFFLL?ret:0x7FFFFFFFFFFFLL;
+  }
+  return 0x7FFFFFFFFFFFLL;
 }
 
 /*Convenience function converts Q57 value to a clamped 32-bit Q24 value
   _in: input in Q57 format.
   Return: same number in Q24 */
-static ogg_int32_t oc_q57_q24(ogg_int64_t _in){
+static ogg_int32_t oc_q57_to_q24(ogg_int64_t _in){
   ogg_int64_t ret;
   ret=_in+((ogg_int64_t)1<<32)>>33;
-  return (ret>2147483647?2147483647:(ret<-2147483648?-2147483648:ret));
+  /*0x80000000 is automatically converted to unsigned on 32-bit systems.
+    -0x7FFFFFFF-1 is needed to avoid "promoting" the whole expression to
+    unsigned.*/
+  return (ogg_int32_t)OC_CLAMPI(-0x7FFFFFFF-1,ret,0x7FFFFFFF);
 }
 
+/*Binary exponential of _log_scale with 24-bit fractional precision and
+   saturation.
+  _log_scale: A binary logarithm in Q57 format.
+  Return: The binary exponential in Q24 format, saturated to 2**31-1 if
+   _log_scale was too large.*/
+static ogg_int32_t oc_bexp64_q24(ogg_int64_t _log_scale){
+  if(_log_scale<OC_Q57(8)){
+    ogg_int64_t ret;
+    ret=oc_bexp64(_log_scale+OC_Q57(24));
+    return ret<0x7FFFFFFF?(ogg_int32_t)ret:0x7FFFFFFF;
+  }
+  return 0x7FFFFFFF;
+}
+
+
 static void oc_enc_rc_reset(oc_enc_ctx *_enc){
   ogg_int64_t npixels;
   ogg_int64_t ibpp;
@@ -263,13 +289,13 @@
   /*Set up second order followers, initialized according to corresponding
      time constants.*/
   oc_iir_filter_init(&_enc->rc.scalefilter[0],2,
-   oc_q57_q24(_enc->rc.log_scale[0]));
+   oc_q57_to_q24(_enc->rc.log_scale[0]));
   inter_delay=_enc->rc.twopass?
    OC_MAXI(_enc->keyframe_frequency_force,12):_enc->rc.buf_delay;
   oc_iir_filter_init(&_enc->rc.scalefilter[1],inter_delay>>1,
-   oc_q57_q24(_enc->rc.log_scale[1]));
+   oc_q57_to_q24(_enc->rc.log_scale[1]));
   oc_iir_filter_init(&_enc->rc.vfrfilter,2,
-   oc_bexp_q24(_enc->rc.log_drop_scale));
+   oc_bexp64_q24(_enc->rc.log_drop_scale));
 }
 
 void oc_rc_state_init(oc_rc_state *_rc,oc_enc_ctx *_enc){
@@ -318,7 +344,7 @@
     _enc->rc.target=(_enc->rc.max+1>>1)+(_enc->rc.bits_per_frame+2>>2)*
      OC_MINI(_enc->keyframe_frequency_force,_enc->rc.buf_delay);
     oc_iir_filter_init(&_enc->rc.scalefilter[1],_enc->rc.buf_delay>>1,
-     oc_q57_q24(_enc->rc.log_scale[1]));
+     oc_q57_to_q24(_enc->rc.log_scale[1]));
   }
   /*If we're in pass-2 mode, make sure the frame metrics array is big enough
      to hold frame statistics for the full buffer.*/
@@ -378,7 +404,7 @@
         qti=_enc->rc.cur_metrics.frame_type;
         _enc->rc.nframes[qti]++;
         _enc->rc.nframes[2]+=_enc->rc.cur_metrics.dup_count;
-        _enc->rc.scale_sum[qti]+=oc_bexp_q24(_enc->rc.cur_metrics.log_scale<<33);
+        _enc->rc.scale_sum[qti]+=oc_bexp_q24(_enc->rc.cur_metrics.log_scale);
         _enc->rc.scale_window_end+=_enc->rc.cur_metrics.dup_count+1;
         if(_enc->rc.scale_window_end-_enc->rc.scale_window0<buf_delay){
           /*We need more frame data.*/
@@ -492,7 +518,7 @@
               do{
                 qti=m->frame_type;
                 nframes[qti]--;
-                scale_sum[qti]-=oc_bexp_q24(m->log_scale<<33);
+                scale_sum[qti]-=oc_bexp_q24(m->log_scale);
                 buf_delay-=m->dup_count+1;
                 fmi++;
                 if(fmi>=_enc->rc.cframe_metrics)fmi=0;
@@ -511,7 +537,7 @@
       qti=_enc->rc.cur_metrics.frame_type;
       if(qti!=_qti){
         nframes[qti]--;
-        scale_sum[qti]-=oc_bexp_q24(_enc->rc.cur_metrics.log_scale<<33);
+        scale_sum[qti]-=oc_bexp_q24(_enc->rc.cur_metrics.log_scale);
       }
       /*Compute corrected log_scale estimates for each frame type from the
          pass-1 scales we measured in the current window.*/
@@ -564,10 +590,13 @@
       qti=_enc->rc.cur_metrics.frame_type;
       if(qti!=_qti){
         ogg_int64_t scale;
-        scale=oc_bexp_q24(_enc->rc.log_scale[_qti])*(ogg_int64_t)nframes[_qti];
+        scale=_enc->rc.log_scale[_qti]<OC_Q57(23)?
+         oc_bexp64(_enc->rc.log_scale[_qti]+OC_Q57(24)):0x7FFFFFFFFFFFLL;
+        scale*=nframes[_qti];
         nframes[_qti]++;
-        scale+=oc_bexp_q24((ogg_int64_t)_enc->rc.scalefilter[_qti].y[0]<<33);
-        _enc->rc.log_scale[_qti]=oc_blog64(scale)-OC_Q57(24);
+        scale+=oc_bexp_q24(_enc->rc.scalefilter[_qti].y[0]);
+        _enc->rc.log_scale[_qti]=oc_blog64(scale)
+         -oc_blog64(nframes[qti])-OC_Q57(24);
       }
       /*Add the padding from above.
         This basically reverts to 1-pass estimations in the last keyframe
@@ -581,12 +610,11 @@
         nextra_frames=oc_rc_scale_drop(&_enc->rc,buf_pad);
         /*And blend in the low-pass filtered scale according to how many frames
            we added.*/
-        scale=oc_bexp_q24(_enc->rc.log_scale[1])*(ogg_int64_t)nframes[1]
-          +oc_bexp_q24((ogg_int64_t)_enc->rc.scalefilter[1].y[0]<<33)*
-          (ogg_int64_t)nextra_frames;
+        scale=
+         oc_bexp64(_enc->rc.log_scale[1]+OC_Q57(24))*(ogg_int64_t)nframes[1]
+         +oc_bexp_q24(_enc->rc.scalefilter[1].y[0])*(ogg_int64_t)nextra_frames;
         nframes[1]+=nextra_frames;
-        _enc->rc.log_scale[1]=
-         oc_blog64((scale+(nframes[1]>>1))/nframes[1])-OC_Q57(24);
+        _enc->rc.log_scale[1]=oc_blog64(scale)-oc_blog64(nframes[1])-OC_Q57(24);
       }
     }break;
   }
@@ -702,7 +730,9 @@
   ogg_int64_t log_scale;
   int         dropped;
   dropped=0;
-  if(!_enc->rc.drop_frames || _enc->rc.twopass)_droppable=0;
+  if(!_enc->rc.drop_frames||_enc->rc.twopass&&_enc->rc.frame_metrics==NULL){
+    _droppable=0;
+  }
   buf_delta=_enc->rc.bits_per_frame*(1+_enc->dup_count);
   if(_bits<=0){
     /*We didn't code any blocks in this frame.*/
@@ -722,7 +752,7 @@
   switch(_enc->rc.twopass){
     case 1:{
       /*Pass 1 mode: save the metrics for this frame.*/
-      _enc->rc.cur_metrics.log_scale=oc_q57_q24(log_scale);
+      _enc->rc.cur_metrics.log_scale=oc_q57_to_q24(log_scale);
       _enc->rc.cur_metrics.dup_count=_enc->dup_count;
       _enc->rc.cur_metrics.frame_type=_enc->state.frame_type;
       _enc->rc.twopass_buffer_bytes=0;
@@ -764,7 +794,7 @@
         _enc->rc.frames_left[2]-=_enc->rc.prev_metrics.dup_count;
         _enc->rc.nframes[qti]--;
         _enc->rc.nframes[2]-=_enc->rc.prev_metrics.dup_count;
-        _enc->rc.scale_sum[qti]-=oc_bexp_q24(_enc->rc.prev_metrics.log_scale<<33);
+        _enc->rc.scale_sum[qti]-=oc_bexp_q24(_enc->rc.prev_metrics.log_scale);
         _enc->rc.scale_window0=(int)next_frame_num;
         /*Free the corresponding entry in the circular buffer.*/
         if(_enc->rc.frame_metrics!=NULL){
@@ -792,7 +822,7 @@
       /*Otherwise update the low-pass scale filter for this frame type,
          regardless of whether or not we dropped this frame.*/
       _enc->rc.log_scale[_qti]=oc_iir_filter_update(
-        _enc->rc.scalefilter+_qti,oc_q57_q24(log_scale))<<33;
+       _enc->rc.scalefilter+_qti,oc_q57_to_q24(log_scale))<<33;
       /*If this frame busts our budget, it must be dropped.*/
       if(_droppable&&_enc->rc.fullness+buf_delta<_bits){
         _enc->rc.prev_drop_count+=1+_enc->dup_count;
@@ -836,6 +866,7 @@
   return dropped;
 }
 
+#define OC_RC_2PASS_VERSION   (1)
 #define OC_RC_2PASS_HDR_SZ    (38)
 #define OC_RC_2PASS_PACKET_SZ (8)
 
@@ -860,13 +891,13 @@
       _enc->rc.scale_sum[0]=_enc->rc.scale_sum[1]=0;
       /*Fill in dummy summary values.*/
       oc_rc_buffer_val(&_enc->rc,0x5032544F,4);
-      oc_rc_buffer_val(&_enc->rc,0,4);
+      oc_rc_buffer_val(&_enc->rc,OC_RC_2PASS_VERSION,4);
       oc_rc_buffer_val(&_enc->rc,0,OC_RC_2PASS_HDR_SZ-8);
     }
     else{
       int qti;
       qti=_enc->rc.cur_metrics.frame_type;
-      _enc->rc.scale_sum[qti]+=oc_bexp_q24(_enc->rc.cur_metrics.log_scale<<33);
+      _enc->rc.scale_sum[qti]+=oc_bexp_q24(_enc->rc.cur_metrics.log_scale);
       _enc->rc.frames_total[qti]++;
       _enc->rc.frames_total[2]+=_enc->rc.cur_metrics.dup_count;
       oc_rc_buffer_val(&_enc->rc,
@@ -878,7 +909,7 @@
    _enc->rc.twopass_buffer_bytes!=OC_RC_2PASS_HDR_SZ){
     _enc->rc.twopass_buffer_bytes=0;
     oc_rc_buffer_val(&_enc->rc,0x5032544F,4);
-    oc_rc_buffer_val(&_enc->rc,0,4);
+    oc_rc_buffer_val(&_enc->rc,OC_RC_2PASS_VERSION,4);
     oc_rc_buffer_val(&_enc->rc,_enc->rc.frames_total[0],4);
     oc_rc_buffer_val(&_enc->rc,_enc->rc.frames_total[1],4);
     oc_rc_buffer_val(&_enc->rc,_enc->rc.frames_total[2],4);
@@ -949,7 +980,7 @@
       /*Read the summary header data.*/
       /*Check the magic value and version number.*/
       if(oc_rc_unbuffer_val(&_enc->rc,4)!=0x5032544F||
-       oc_rc_unbuffer_val(&_enc->rc,4)!=0){
+       oc_rc_unbuffer_val(&_enc->rc,4)!=OC_RC_2PASS_VERSION){
         _enc->rc.twopass_buffer_bytes=0;
         return TH_ENOTFORMAT;
       }
@@ -1074,7 +1105,7 @@
             /*And accumulate the statistics over the window.*/
             _enc->rc.nframes[qti]++;
             _enc->rc.nframes[2]+=m->dup_count;
-            _enc->rc.scale_sum[qti]+=oc_bexp_q24(m->log_scale<<33);
+            _enc->rc.scale_sum[qti]+=oc_bexp_q24(m->log_scale);
             _enc->rc.scale_window_end+=m->dup_count+1;
             /*Compute an upper bound on the number of remaining packets needed
                for the current window.*/



More information about the commits mailing list