[x264-devel] [patch] minor code cleanup and fix for x264.c

[Limin Wang]

Hi guys,

Now I'm new to x264 and start to review the code now. 

Below is my patch to do code cleanup for x264.c by my review.
comments:
* if i_total_frame is 0, it should never enter the encode loop
* can't seek frame out of range
 

BTW, where is x264 code style? If not, can we create a style
like xvid's CodeStyle? It's better to use linux style instead
of window style.  It's buggy to prefix the variable with 
i_* etc.


Thanks,
Limin
-------------- next part --------------
Index: x264.c
===================================================================
--- x264.c	(revision 618)
+++ x264.c	(working copy)
@@ -762,6 +762,15 @@
     int     i_progress;
 
     i_frame_total = p_get_frame_total( opt->hin );
+    if( opt->i_seek > i_frame_total )
+    {
+        fprintf( stderr, "x264 [error]: seek(%d) > total frame(%d)\n",
+                 opt->i_seek, i_frame_total );
+        p_close_infile( opt->hin );
+        p_close_outfile( opt->hout );
+        return -1;
+    }
+    
     i_frame_total -= opt->i_seek;
     if( ( i_frame_total == 0 || param->i_frame_total < i_frame_total )
         && param->i_frame_total > 0 )
@@ -781,6 +790,7 @@
         fprintf( stderr, "x264 [error]: can't set outfile param\n" );
         p_close_infile( opt->hin );
         p_close_outfile( opt->hout );
+        x264_encoder_close( h );
         return -1;
     }
 
@@ -791,7 +801,7 @@
 
     /* Encode frames */
     for( i_frame = 0, i_file = 0, i_progress = 0;
-         b_ctrl_c == 0 && (i_frame < i_frame_total || i_frame_total == 0); )
+         b_ctrl_c == 0 && i_frame < i_frame_total; )
     {
         if( p_read_frame( &pic, opt->hin, i_frame + opt->i_seek ) )
             break;
@@ -813,21 +823,16 @@
 
         /* update status line (up to 1000 times per input file) */
         if( opt->b_progress && param->i_log_level < X264_LOG_DEBUG && 
-            ( i_frame_total ? i_frame * 1000 / i_frame_total > i_progress
-                            : i_frame % 10 == 0 ) )
+            ( i_frame * 1000 / i_frame_total > i_progress ) )
         {
             int64_t i_elapsed = x264_mdate() - i_start;
-            double fps = i_elapsed > 0 ? i_frame * 1000000. / i_elapsed : 0;
-            if( i_frame_total )
-            {
-                int eta = i_elapsed * (i_frame_total - i_frame) / ((int64_t)i_frame * 1000000);
-                i_progress = i_frame * 1000 / i_frame_total;
-                fprintf( stderr, "encoded frames: %d/%d (%.1f%%), %.2f fps, eta %d:%02d:%02d  \r",
-                         i_frame, i_frame_total, (float)i_progress / 10, fps,
-                         eta/3600, (eta/60)%60, eta%60 );
-            }
-            else
-                fprintf( stderr, "encoded frames: %d, %.2f fps   \r", i_frame, fps );
+            double fps = i_elapsed > 0 ? (double)i_frame * (double)1000000. / (double)i_elapsed : 0;
+            int eta = i_elapsed * (i_frame_total - i_frame) / ((int64_t)i_frame * 1000000);
+            
+            i_progress = i_frame * 1000 / i_frame_total;
+            fprintf( stderr, "encoded frames: %d/%d (%.1f%%), %.2f fps, eta %d:%02d:%02d  \r",
+                     i_frame, i_frame_total, (float)i_progress / 10, fps,
+                     eta/3600, (eta/60)%60, eta%60 );
             fflush( stderr ); // needed in windows
         }
     }
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 481 bytes
Desc: not available
Url : http://mailman.videolan.org/pipermail/x264-devel/attachments/20070115/779878bb/attachment.pgp