Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Nov 28, 2025

GetTempFileName creates files with .tmp extension, but MCI relies on file extension to identify media type. Compressed AVI files fail to play on some systems when the extension is not .avi. Additionally, MCIWndOpen/MCIWndPlay errors were silently ignored.

Changes

  • Rename temp file to .avi: After GetTempFileName, rename from .tmp to .avi using MoveFile. Falls back to original path if rename fails.
  • Add MCI error checking: Check return values of MCIWndOpen and MCIWndPlay, hide window on failure.
  • Add LogMCIError helper: Uses mciGetErrorString to log diagnostic info via OutputDebugString.
// MCI relies on file extension to determine media type; rename .tmp to .avi
LPTSTR pszDot = _tcsrchr(m_szTempFile, TEXT('.'));
if (pszDot != NULL && pszDot + 5 <= m_szTempFile + _countof(m_szTempFile))
{
    TCHAR szOldPath[MAX_PATH];
    _tcscpy_s(szOldPath, m_szTempFile);
    _tcscpy_s(pszDot, 5, TEXT(".avi"));
    if (!MoveFile(szOldPath, m_szTempFile))
        _tcscpy_s(m_szTempFile, szOldPath);  // revert on failure
}
Original prompt

Problem:
MBmpView::SetMedia creates a temporary file using GetTempFileName with prefix "avi", but GetTempFileName creates files with a .tmp extension (e.g., aviXXXX.tmp). MCI (MCIWndOpen/MCIWndPlay) may fail to recognize the file as an AVI if the extension is not .avi, preventing playback of compressed AVI files on some systems. The current code also doesn't check the result of MCIWndOpen/MCIWndPlay for errors, making it hard to diagnose issues.

Goal:
Create a pull request that fixes MBmpView so that temporary media files are saved with a proper .avi extension and that MCI open/play errors are checked and logged. The change should be minimal and maintain backward compatibility.

Proposed changes:

  1. In src/MBmpView.hpp, after obtaining a temp filename with GetTempFileName, replace the file extension with ".avi" (e.g., find the last '.' and replace with ".avi"). Ensure buffer bounds are handled safely (use _tcsrchr/_tcscpy_s or similar).
  2. When opening the file with MCIWndOpen and starting playback with MCIWndPlay, check return values (MCIWndOpen returns an HWND or nonzero error?) and use mciGetErrorString via mciSendCommand or appropriate MCI error retrieval to get diagnostic text. If MCI fails to open or play, log the MCI error string (use OutputDebugString and/or MessageBox for visibility) and hide the MCI window.
  3. Add minimal helper code (inline) for obtaining MCI error string if necessary.

Files to modify:

  • src/MBmpView.hpp: update SetMedia to ensure .avi extension and check/report MCI errors; update DeleteTempFile if necessary to account for renamed temp files.

Notes:

  • Keep changes small and focused so it's easy to review and backport.
  • No new third-party dependencies.
  • If helpful, include a small comment explaining why .avi extension is required for MCI on some systems.

Please create the PR in the katahiromz/RisohEditor repository with the above changes, include a concise description in the PR body, and ensure the commit modifies src/MBmpView.hpp accordingly.

This pull request was created as a result of the following prompt from Copilot chat.

Problem:
MBmpView::SetMedia creates a temporary file using GetTempFileName with prefix "avi", but GetTempFileName creates files with a .tmp extension (e.g., aviXXXX.tmp). MCI (MCIWndOpen/MCIWndPlay) may fail to recognize the file as an AVI if the extension is not .avi, preventing playback of compressed AVI files on some systems. The current code also doesn't check the result of MCIWndOpen/MCIWndPlay for errors, making it hard to diagnose issues.

Goal:
Create a pull request that fixes MBmpView so that temporary media files are saved with a proper .avi extension and that MCI open/play errors are checked and logged. The change should be minimal and maintain backward compatibility.

Proposed changes:

  1. In src/MBmpView.hpp, after obtaining a temp filename with GetTempFileName, replace the file extension with ".avi" (e.g., find the last '.' and replace with ".avi"). Ensure buffer bounds are handled safely (use _tcsrchr/_tcscpy_s or similar).
  2. When opening the file with MCIWndOpen and starting playback with MCIWndPlay, check return values (MCIWndOpen returns an HWND or nonzero error?) and use mciGetErrorString via mciSendCommand or appropriate MCI error retrieval to get diagnostic text. If MCI fails to open or play, log the MCI error string (use OutputDebugString and/or MessageBox for visibility) and hide the MCI window.
  3. Add minimal helper code (inline) for obtaining MCI error string if necessary.

Files to modify:

  • src/MBmpView.hpp: update SetMedia to ensure .avi extension and check/report MCI errors; update DeleteTempFile if necessary to account for renamed temp files.

Notes:

  • Keep changes small and focused so it's easy to review and backport.
  • No new third-party dependencies.
  • If helpful, include a small comment explaining why .avi extension is required for MCI on some systems.

Please create the PR in the katahiromz/RisohEditor repository with the above changes, include a concise description in the PR body, and ensure the commit modifies src/MBmpView.hpp accordingly.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

…ogging

Co-authored-by: katahiromz <2107452+katahiromz@users.noreply.github.com>
@katahiromz katahiromz marked this pull request as ready for review November 28, 2025 12:00
…dling

Co-authored-by: katahiromz <2107452+katahiromz@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix temporary file extension for AVI playback issues Fix MBmpView temp file extension for MCI playback and add error logging Nov 28, 2025
Copilot AI requested a review from katahiromz November 28, 2025 12:02
@katahiromz katahiromz merged commit 11dc680 into master Nov 28, 2025
1 check passed
@katahiromz katahiromz deleted the copilot/fix-temp-file-extension-error branch November 28, 2025 12:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants