Fix the behaviour of FileGroup when seeking too far.

Previously, if you did a seek off the end of the file group,
the seek would return an error.

This is not what fseek() does; it returns no error, and preserves
the file pointer (returned by ftell()) as if the seek had been
successful.  fread()s after a too-far seek return no data, of
course.

Parsing some files (the example used to find the bug was a
H264 MP4) involves a seek which is to the byte after the end
of the mp4 file.  If this fails the whole header parsing fails
and DCP-o-matic refuses to use the file.

diff --git a/src/lib/file_group.cc b/src/lib/file_group.cc
index 3e8a7b79c43177a8d775889ab91d08855bb0e77e..f06ca300718acd727c74b39fa09981d1185ffb55 100644 (file)
--- a/src/lib/file_group.cc
+++ b/src/lib/file_group.cc
@@ -132,12 +132,18 @@ FileGroup::seek (int64_t pos, int whence) const
                if (sub_pos < int64_t (len)) {
                        break;
                }
-               sub_pos -= len;
                ++i;
+               if (i < _paths.size()) {
+                       /* If we've run out of files we need to seek off the end of the last file */
+                       sub_pos -= len;
+               }
        }
 
        if (i == _paths.size ()) {
-               return -1;
+               /* Seeking too far isn't an error; we'll seek too far in the last file which
+                * will "pass on" fseek()'s behaviour to our caller.
+                */
+               i--;
        }
 
        ensure_open_path (i);

diff --git a/test/file_group_test.cc b/test/file_group_test.cc
index cfcaacfc261f75159a89466a70aee4203a9e1725..05127828cb722c39e32834a45ecb30632e6c437b 100644 (file)
--- a/test/file_group_test.cc
+++ b/test/file_group_test.cc
@@ -92,8 +92,15 @@ BOOST_AUTO_TEST_CASE (file_group_test)
        BOOST_CHECK_EQUAL (fg.read (test, total_length * 3), total_length - pos);
        BOOST_CHECK_EQUAL (memcmp (data + pos, test, total_length - pos), 0);
 
-       /* Bad seek */
-       BOOST_CHECK_EQUAL (fg.seek (total_length * 2, SEEK_SET), -1);
+       /* Seeking off the end of the file should not give an error */
+       BOOST_CHECK_EQUAL (fg.seek (total_length * 2, SEEK_SET), total_length * 2);
+       /* and attempting to read should return nothing */
+       BOOST_CHECK_EQUAL (fg.read (test, 64), 0);
+       /* but the requested seek should be remembered, so if we now go back (relatively) */
+       BOOST_CHECK_EQUAL (fg.seek (-total_length * 2, SEEK_CUR), 0);
+       /* we should be at the start again */
+       BOOST_CHECK_EQUAL (fg.read (test, 64), 64);
+       BOOST_CHECK_EQUAL (memcmp (data, test, 64), 0);
 
        /* SEEK_SET */
        BOOST_CHECK_EQUAL (fg.seek (999, SEEK_SET), 999);