[x264-devel] Fix issues relating to input/output files being pipes/FIFOs

[Tomas Janousek]

----- Forwarded message from Tomas Janousek <tomi at nomi.cz> -----

Date: Thu, 24 Dec 2009 11:11:33 +0100
From: Tomas Janousek <tomi at nomi.cz>
To: Steven Walters <kemuri9 at gmail.com>
Subject: Re: Fix issues relating to input/output files being pipes/FIFOs
User-Agent: Mutt/1.5.20 (2009-06-14)

Hello,

On Sun, Nov 08, 2009 at 01:07:28AM +0000, Steven Walters wrote:
> Fix issues relating to input/output files being pipes/FIFOs

This patch broke two-pass encoding on Samba shares on 32-bit machines. The
problem here is that _FILE_OFFSET_BITS in osdep.h is set too late and doesn't
have any effect, hence 32-bit fstat is used and fails, because CIFS has really
big inode numbers.

> @@ -63,6 +63,7 @@ do {\
>  /****************************************************************************
>   * Includes
>   ****************************************************************************/
> +#include <sys/stat.h>
>  #include "osdep.h"
>  #include <stdarg.h>
>  #include <stddef.h>

For example here, you include a system header _before_ _FILE_OFFSET_BITS are
set. This happens elsewhere in x264 as well, so I suggest that you just add
relevant -D options to CFLAGS. I can confirm that adding it to --extra-cflags
in configure does solve the issue for me.

> +static inline uint8_t x264_is_regular_file( FILE *filehandle )
> +{
> +    struct stat file_stat;
> +    if( fstat( fileno( filehandle ), &file_stat ) )
> +        return 0;
> +    return S_ISREG( file_stat.st_mode );
> +}

^^^ fstat returns -1, since the inode number can't be copied into a 32-bit
inode field in the stat structure.

And, well, this is the place that causes stats files not to be renamed:

> --- a/encoder/ratecontrol.c
> +++ b/encoder/ratecontrol.c
> @@ -880,11 +880,13 @@ void x264_ratecontrol_delete( x264_t *h )
>  {
>      x264_ratecontrol_t *rc = h->rc;
>      int i;
> +    int b_regular_file;
>  
>      if( rc->p_stat_file_out )
>      {
> +        b_regular_file = x264_is_regular_file( rc->p_stat_file_out );
>          fclose( rc->p_stat_file_out );
> -        if( h->i_frame >= rc->num_entries )
> +        if( h->i_frame >= rc->num_entries && b_regular_file )
>              if( rename( rc->psz_stat_file_tmpname, h->param.rc.psz_stat_out ) != 0 )
>              {
>                  x264_log( h, X264_LOG_ERROR, "failed to rename \"%s\" to \"%s\"\n",
> @@ -894,8 +896,9 @@ void x264_ratecontrol_delete( x264_t *h )
>      }
>      if( rc->p_mbtree_stat_file_out )
>      {
> +        b_regular_file = x264_is_regular_file( rc->p_mbtree_stat_file_out );
>          fclose( rc->p_mbtree_stat_file_out );
> -        if( h->i_frame >= rc->num_entries )
> +        if( h->i_frame >= rc->num_entries && b_regular_file )
>              if( rename( rc->psz_mbtree_stat_file_tmpname, rc->psz_mbtree_stat_file_name ) != 0 )
>              {
>                  x264_log( h, X264_LOG_ERROR, "failed to rename \"%s\" to \"%s\"\n",

You may as well consider treating a file as regular whenever fstat fails, but
I think that won't happen any more if you fix the _FILE_OFFSET_BITS defines.

Regards,
-- 
Tomáš Janoušek, a.k.a. Liskni_si, http://work.lisk.in/

----- End forwarded message -----