shithub: opusfile

Download patch

ref: 24cb5eae37d11ad699b17b8add62573f156e96f6
parent: 34f945bb63b33e5b4069d99ea98b124c4cdde53d
author: Timothy B. Terriberry <[email protected]>
date: Mon Nov 5 10:58:14 EST 2018 

Fix seekability detection on win32.

The seeking functions on Windows internally dispatch to
 SetFilePointer(), whose behavior is undefined if you do not call
 it on "a file stored on a seeking device".
Check the type of file when it is opened and manually force seeks
 to fail if we do not have such a file.

Thanks to L W Anhonen for the report and Mark Harris for pointing
 out the solution used by opus-tools to avoid this problem.


--- a/src/opusfile.c
+++ b/src/opusfile.c
@@ -1531,7 +1531,9 @@
     ogg_sync_wrote(&_of->oy,(long)_initial_bytes);
   }
   /*Can we seek?
-    Stevens suggests the seek test is portable.*/
+    Stevens suggests the seek test is portable.
+    It's actually not for files on win32, but we address that by fixing it in
+     our callback implementation (see stream.c).*/
   seekable=_cb->seek!=NULL&&(*_cb->seek)(_stream,0,SEEK_CUR)!=-1;
   /*If seek is implemented, tell must also be implemented.*/
   if(seekable){
--- a/src/stream.c
+++ b/src/stream.c
@@ -227,6 +227,57 @@
   return dst;
 }
 
+/*fsetpos() internally dispatches to the win32 API call SetFilePointer().
+  According to SetFilePointer()'s documentation [0], the behavior is
+   undefined if you do not call it on "a file stored on a seeking device".
+  However, none of the MSVCRT seeking functions verify what kind of file is
+   being used before calling it (which I believe is a bug, since they are
+   supposed to fail and return an error, but it is a bug that has been there
+   for multiple decades now).
+  In practice, SetFilePointer() appears to succeed for things like stdin,
+   even when you are not just piping in a regular file, which prevents the use
+   of this API to determine whether it is possible to seek in a file at all.
+  Therefore, we take the approach recommended by the SetFilePointer()
+   documentation and confirm the type of file using GetFileType() first.
+  We do this once, when the file is opened, and return the corresponding
+   callback in order to avoid an extra win32 API call on every seek in the
+   common case.
+  Hopefully the return value of GetFileType() cannot actually change for the
+   lifetime of a file handle.
+  [0] https://docs.microsoft.com/en-us/windows/desktop/api/fileapi/nf-fileapi-setfilepointer
+*/
+static int op_fseek_fail(void *_stream,opus_int64 _offset,int _whence){
+  (void)_stream;
+  (void)_offset;
+  (void)_whence;
+  return -1;
+}
+
+static const OpusFileCallbacks OP_UNSEEKABLE_FILE_CALLBACKS={
+  op_fread,
+  op_fseek_fail,
+  op_ftell,
+  (op_close_func)fclose
+};
+
+# define WIN32_LEAN_AND_MEAN
+# define WIN32_EXTRA_LEAN
+# include <windows.h>
+
+static const OpusFileCallbacks *op_get_file_callbacks(FILE *_fp){
+  intptr_t h_file;
+  h_file=_get_osfhandle(_fileno(_fp));
+  if(h_file!=-1
+   &&(GetFileType((HANDLE)h_file)&~FILE_TYPE_REMOTE)==FILE_TYPE_DISK){
+    return &OP_FILE_CALLBACKS;
+  }
+  return &OP_UNSEEKABLE_FILE_CALLBACKS;
+}
+#else
+static const OpusFileCallbacks *op_get_file_callbacks(FILE *_fp){
+  (void)_fp;
+  return &OP_FILE_CALLBACKS;
+}
 #endif
 
 void *op_fopen(OpusFileCallbacks *_cb,const char *_path,const char *_mode){
@@ -247,7 +298,7 @@
     _ogg_free(wpath);
   }
 #endif
-  if(fp!=NULL)*_cb=*&OP_FILE_CALLBACKS;
+  if(fp!=NULL)*_cb=*op_get_file_callbacks(fp);
   return fp;
 }
 
@@ -254,7 +305,7 @@
 void *op_fdopen(OpusFileCallbacks *_cb,int _fd,const char *_mode){
   FILE *fp;
   fp=fdopen(_fd,_mode);
-  if(fp!=NULL)*_cb=*&OP_FILE_CALLBACKS;
+  if(fp!=NULL)*_cb=*op_get_file_callbacks(fp);
   return fp;
 }
 
@@ -277,7 +328,7 @@
     _ogg_free(wpath);
   }
 #endif
-  if(fp!=NULL)*_cb=*&OP_FILE_CALLBACKS;
+  if(fp!=NULL)*_cb=*op_get_file_callbacks(fp);
   return fp;
 }