Fix movie module master frame handling with title sequences (issue #8322)

[Esteban82]

Done by Claude Sonnet 5.
Explanation by Claude:

Fix movie module master frame handling with title sequences (issue #8322)

Summary

This PR fixes four critical bugs affecting the movie module when -M (master frame) is used together with -E (title sequence):

1. Wrong frame selection for -Ml (last frame) — frame number not adjusted for title sequence offset
2. Segfault when -Mf requests a title frame — stale FILE pointer read after close
3. Duplicate -N option to psconvert — buffer reuse bug causes invalid PostScript output
4. Path resolution failure on master frame reopen — relative paths invalid after chdir to temp directory

Bug Analysis and Fixes

Bug #1: -Ml (last frame) points to wrong frame number

Root cause:
When -E (title sequence) is active, all frame numbers in the final video are offset by Ctrl->E.duration (the number of frames in the title sequence). The code that calculated the master frame number for -Ml failed to account for this offset:

Ctrl->M.frame = (Ctrl->M.update == 2) ? n_frames - 1 : n_frames / 2;

This meant -Ml would return frame n_frames - 1 relative to frame 0, which is actually a frame inside (or before) the title sequence, not the last frame of the main animation.

Example:

# Movie: 4 frames of title + 10 frames of animation = 14 total frames
gmt movie main.sh -T10 -Etitle.sh+d1s -Ml,png  # -D4 (default)
# Expected: frame 13 (last of the animation, absolute)
# Actual (bug): frame 9 (which is still in the title!)

Fix:
Add the title sequence offset to the frame calculation for both -Ml and -Mm:

// Before:
Ctrl->M.frame = (Ctrl->M.update == 2) ? n_frames - 1 : n_frames / 2;

// After (line ~1833):
Ctrl->M.frame = Ctrl->E.duration + ((Ctrl->M.update == 2) ? n_frames - 1 : n_frames / 2);

This ensures all frame numbers consistently refer to absolute positions in the final video, regardless of whether a title is present.

---

Bug #2: Segfault when master frame falls inside title sequence

Root cause:
The title script file pointer (Ctrl->E.fp) follows this lifecycle:

1. Opened successfully during parsing (line ~1311)
2. Read completely while building the intro script (line ~2062)
3. Closed with fclose() (line ~2078)

Later, when -M requests a frame that falls inside the title sequence (the is_title branch, typical for -Mf with -E), the code attempted to reuse the same closed FILE pointer:

while (gmt_fgets (GMT, line, PATH_MAX, Ctrl->E.fp)) { ... }  // Read closed FILE*
rewind (Ctrl->E.fp);  // Rewind closed FILE*

This is undefined behavior in C. The segfault occurs in gmt_fgets → __fgets_chk when the runtime tries to access an invalid file descriptor.

Example:

gmt movie main.sh -Etitle.sh -Mf,png
# Caught signal 11 (SIGSEGV) in gmt_fgets
# mv: cannot stat 'frame_0.png': No such file or directory

Fix:
When the code needs to re-read the title script for a title master frame, reopen the file with a fresh fopen() call (line ~2378):

// Before:
while (gmt_fgets (GMT, line, PATH_MAX, Ctrl->E.fp)) { ... }
rewind (Ctrl->E.fp);

// After:
if ((Ctrl->E.fp = fopen (Ctrl->E.file, "r")) == NULL) {
    GMT_Report (API, GMT_MSG_ERROR, "Unable to reopen title script file %s\n", Ctrl->E.file);
    fclose (Ctrl->In.fp);
    error = GMT_ERROR_ON_FOPEN;
    goto out_of_here;
}
while (gmt_fgets (GMT, line, PATH_MAX, Ctrl->E.fp)) { ... }
fclose (Ctrl->E.fp);  // File is not needed again after this point

The reopen is now safe because (in conjunction with Bug #4 fix) Ctrl->E.file has been converted to an absolute path.

---

Bug #3: Duplicate -N option to psconvert with -G and title master frame

Root cause:
The extra buffer (used to build PSL prologue arguments passed to psconvert) is recycled across three different script construction phases:

• Title/intro script (line ~2049)
• Main animation script (line ~2103)
• Master frame script (line ~2327)

In the master frame is_title branch (when the requested frame falls inside the title sequence), the code only computed the fade level but never reinitialize extra with a fresh sprintf() call:

if (is_title) {
    // ... compute fade_level ...
    gmt_set_dvalue (fp, Ctrl->In.mode, "MOVIE_FADE", fade_level, 0);
    // BUG: extra is not reinitialized here!
}

This meant extra retained stale content from a previous phase—typically the intro script construction, which already included ,N+f<fadevar>. Then, when the -G (canvas color) block executed:

if (!Ctrl->K.active) strcat (extra, ",N");  // Adds ANOTHER ,N!

The result: a duplicate -N option passed to psconvert, which rejects it with "Option -N: Given more than once".

Example:

gmt movie main.sh -Etitle.sh -M0,png -Gred
# psconvert [ERROR]: Option -N: Given more than once (offending option is -N+gred)
# end [ERROR]: Failed to call psconvert
# mv: cannot stat 'frame_0.png': No such file or directory

Fix:
Always rebuild extra in the is_title branch (line ~2327), just like in the other phases, and guard the strcat(extra, ",N") call to skip it when either title fade or animation fade (Ctrl->K.active) is already present:

// Before:
if (is_title) {
    if (Ctrl->M.frame < Ctrl->E.fade[GMT_IN]) fade_level = ...
    else if (Ctrl->M.frame > (start = ...)) fade_level = ...
    else fade_level = 0.0;
    gmt_set_dvalue (fp, Ctrl->In.mode, "MOVIE_FADE", fade_level, 0);
}
else if (Ctrl->K.active) {
    sprintf (extra, "A+M+r,N+f%s", gmt_place_var (Ctrl->In.mode, "MOVIE_FADE"));
    ...
}
else
    sprintf (extra, "A+M+r");

if (Ctrl->G.active) {
    if (!Ctrl->K.active) strcat (extra, ",N");  // BUG: doesn't account for is_title!
    ...
}

// After:
if (is_title) {
    if (Ctrl->M.frame < Ctrl->E.fade[GMT_IN]) fade_level = ...
    else if (Ctrl->M.frame > (start = ...)) fade_level = ...
    else fade_level = 0.0;
    gmt_set_dvalue (fp, Ctrl->In.mode, "MOVIE_FADE", fade_level, 0);
    // FIX: rebuild extra here
    sprintf (extra, "A+M+r,N+f%s", gmt_place_var (Ctrl->In.mode, "MOVIE_FADE"));
    if (Ctrl->E.fill) { strcat (extra, "+k"); strcat (extra, Ctrl->E.fill); }
}
else if (Ctrl->K.active) {
    sprintf (extra, "A+M+r,N+f%s", gmt_place_var (Ctrl->In.mode, "MOVIE_FADE"));
    ...
}
else
    sprintf (extra, "A+M+r");

if (Ctrl->G.active) {
    // FIX: skip N if already present via title or animation fade
    if (!Ctrl->K.active && !is_title) strcat (extra, ",N");
    ...
}

---

Bug #4: "Unable to reopen title script file" when using relative paths

Root cause:
Ctrl->E.file (the title script filename) is parsed and stored as-is at parse time (line ~1311), when the working directory is still the user's original directory. Later, GMT_movie changes to a temporary workspace directory via chdir(). When Bug #2's fix attempts to reopen Ctrl->E.file for a title master frame, any relative path no longer resolves:

$ pwd
/home/user/myproject

$ gmt movie main.sh -Etitle.sh -M0,png
# Ctrl->E.file = "title.sh" (relative path, valid here)
# ... GMT_movie does chdir("GMT_job_1234") ...
# ... later, fopen("title.sh", "r") fails — no such file in /tmp/GMT_job_1234
# movie [ERROR]: Unable to reopen title script file title.sh

Fix:
Right after successfully opening Ctrl->E.file the first time (while still in the original directory), convert it to an absolute path using realpath() (POSIX) or _fullpath() (Windows), and store the absolute path (line ~1315):

// After successfully opening Ctrl->E.file for the first time:
if (n_errors == 0) {
    char abs_path[PATH_MAX];
#ifdef WIN32
    if (_fullpath (abs_path, Ctrl->E.file, PATH_MAX) != NULL) {
#else
    if (realpath (Ctrl->E.file, abs_path) != NULL) {
#endif
        gmt_M_str_free (Ctrl->E.file);
        Ctrl->E.file = strdup (abs_path);
    }
}

This conversion happens before any chdir(), so relative paths resolve correctly. Subsequent reopens (for title master frames) now work from any directory.

---

Impact and Testing

Before the fixes:

• -Ml with -E: Returns wrong frame number (inside title, not the actual last frame)
• -Mf with -E: Segfault in gmt_fgets
• -M0 with -E and -Gred: Duplicate -N option error from psconvert
• Relative title script paths: Reopen fails after chdir

After the fixes:

• All combinations of -M (f/m/l/number) work correctly with -E
• Master frames in the title sequence are built without segfault
• Canvas color and outline options (-G) work with title master frames
• Relative paths are transparently converted to absolute

Recommended test cases:

# Test 1: -Ml without title (no change)
gmt movie main.sh -T10 -Ml,png

# Test 2: -Ml with title (now returns frame 9, not 3)
gmt movie main.sh -T10 -Etitle.sh+d1s -Ml,png

# Test 3: -Mf with title (no segfault)
gmt movie main.sh -Etitle.sh -Mf,png

# Test 4: Master frame with canvas color and title
gmt movie main.sh -Etitle.sh -M0,png -Gred

# Test 5: Relative path resolution
cd /tmp && gmt movie ./main.sh -E./title.sh -M0,png

# Test 6: -Mm with title (now returns frame at midpoint of animation, not title)
gmt movie main.sh -T10 -Etitle.sh+d1s -Mm,png

---

Files Modified

• src/movie.c — Four distinct bug fixes as described above

Backward Compatibility

✓ Fully backward compatible. All existing command lines work unchanged. This PR corrects bugs; no API changes or new options in this version.

---

Related Issues

• Closes Get last frame (-Ml) when a title sequence is used (-E) in movie #8322

Test with:

cat << 'EOF' > title.sh
gmt begin
	echo "5 5 Title" | gmt text -R0/10/0/10 -Jx1c -F+f26p,Helvetica-Bold+jCB -X0 -Y0
gmt end
EOF
cat << 'EOF' > main.sh
gmt begin
	echo ${MOVIE_FRAME} | gmt text -R0/10/0/10 -Jx1c -F+f200p+cCM -B0 -X0 -Y0
gmt end
EOF
gmt movie main.sh -Nprueba_1 -T3 -C10cx10cx30 -Gred -D4 -Zs -M0,png -Etitle.sh+d1s -Vi -Fmp4

[joa-quim]

I don't think it is of any use these long reports generated by AI. Many sites require that messages be posted by humans and I think we should try to, at least, filter/summarize those reports. Actually the PR messages should be written by humans (even if locally we ask for assistance).

[Esteban82]

Okay, I won't copy them anymore. Sounds good to me. I thought it might be helpful. That said, I can confirm that the PR fixes the problem. I tested it with the script.

[joa-quim]

Trust your testing of it.

[Esteban82]

In the tests, I see that the pscoast_JE2.sh failed. Is this related to this PR? Can I merge it without any issues?

[joa-quim]

pscoast_JE2.sh failure is a different thing. Don't get why it started to fail on *nix (passes on Windows) from some weeks now.

[Esteban82]

Okay, so I'm going to merge it.