Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regression: Can't save any change of Song Info in MP3 file #1421

Closed
rosti-il opened this issue Jun 28, 2024 · 8 comments
Closed

Regression: Can't save any change of Song Info in MP3 file #1421

rosti-il opened this issue Jun 28, 2024 · 8 comments
Labels
Bug Something isn't working

Comments

@rosti-il
Copy link

This is a regression in the latest 4.4 version of Audacious in the functionality that worked properly in the previous 4.3.1 version of Audacious. When I try to save any change of the Song Info in a local MP3 file, an error appears, as shown in the attached screenshot.

image

My system is Windows 10 Pro 22H2 64-bit.
Audacious 4.4 was installed by running the audacious-4.4-win32.exe installer after the previously installed 4.3.1 version was uninstalled.

@rosti-il rosti-il added the Bug Something isn't working label Jun 28, 2024
@radioactiveman
Copy link
Member

I could reproduce the error with Audacious 4.4 on Windows. The log inspector reveals that calling tmpfile() fails with the message "Permission denied". It works when running Audacious as admin (this is not recommended though).

@jlindgren90: Can you reproduce this? Since the code has not changed for years, could it be caused by the change to 64-bit?

VFSImpl * vfs_tmpfile(String & error)
{
FILE * stream = tmpfile();
if (!stream)
{
int errsave = errno;
perror("(tmpfile)");
error = String(strerror(errsave));
return nullptr;
}
return new LocalFile("(tmpfile)", stream);
}

@jlindgren90
Copy link
Member

It probably has something to do with the newer 64-bit MinGW platform, yes.

Possibly we need to use a different location for temporary files on Windows. I don't know when I will have time to look into the issue further.

@rosti-il
Copy link
Author

This regression is likely related to the following bug in MinGW:
msys2/MINGW-packages#18878

@jlindgren90
Copy link
Member

We can probably use https://docs.gtk.org/glib/func.file_open_tmp.html instead.

@rosti-il
Copy link
Author

Maybe the best solution would be switching from MINGW64 into UCRT64 on the MSYS2 platform when building for Windows? At least the MSYS2 project itself recommends to use UCRT64 instead of MINGW64.

radioactiveman added a commit to radioactiveman/audacious that referenced this issue Aug 14, 2024
UCRT64 is meanwhile the recommended MSYS2 environment for Windows.
Visual Studio uses it by default since 2015. tmpfile() behaves here
also as expected and does not always return NULL.

See also:
- https://www.msys2.org/docs/environments/
- https://www.msys2.org/news/#2022-10-29-changing-the-default-environment-from-mingw64-to-ucrt6
- msys2/MINGW-packages#18878
radioactiveman added a commit to radioactiveman/audacious that referenced this issue Aug 14, 2024
UCRT64 is meanwhile the recommended MSYS2 environment for Windows.
Visual Studio uses it by default since 2015. tmpfile() behaves here
also as expected and does not always return NULL.

See also:
- https://www.msys2.org/docs/environments/
- https://www.msys2.org/news/#2022-10-29-changing-the-default-environment-from-mingw64-to-ucrt6
- msys2/MINGW-packages#18878
@radioactiveman
Copy link
Member

Maybe the best solution would be switching from MINGW64 into UCRT64 on the MSYS2 platform when building for Windows? At least the MSYS2 project itself recommends to use UCRT64 instead of MINGW64.

I agree and can confirm it indeed fixes this issue. See the referenced pull request.

@rosti-il
Copy link
Author

Nice!
Waiting for the next release with this fix. Is there any planned date?

@radioactiveman
Copy link
Member

@rosti-il: Not planned yet, but I guess we can prepare a new release in the next weeks.

@jlindgren90: Do you think we can release 4.4.1 from master? Strictly speaking we have not only bug fixes there, but the other changes are mostly straightforward or backports of existing features from Qt interfaces.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants