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

acendan: Add ReaScript API: AC_GetMediaFileCoverImage() #1662

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

acendan
Copy link

@acendan acendan commented Jul 18, 2022

This PR is largely identical to the original PR, with the only difference being the branch acendan:acendan, as opposed to acendan:master (per this comment from @nofishonfriday).

This PR includes the squashed commit with:

  1. acendan: Initial commit. Add ReaScript API: AC_GetMediaFileCoverImage()
  2. Removed #include <codecvt>, was previously relying on wstrings for file I/O, but no longer needed for character encoding conversions.
  3. @cfillion: Swap std::copy for snprintf. @nofishonfriday: Add newline to end of files.

Tested: Win10, OSX 10.14.6, Linux Mint

Was previously relying on wstrings for file I/O, but no longer needed for character encoding conversions.

acendan: Initial commit. Add ReaScript API: AC_GetMediaFileCoverImage()

Tested: Win10, OSX 10.14.6, Linux Mint

Swap std::copy for snprintf. Add newline to end of files.

reaper-oss#1658 (comment)
@acendan acendan changed the title acendan: Squashed commit. Add ReaScript API: AC_GetMediaFileCoverImage() acendan: Add ReaScript API: AC_GetMediaFileCoverImage() Jul 18, 2022
@acendan
Copy link
Author

acendan commented Jul 21, 2022

Bump! @cfillion @nofishonfriday

Just bringing visibility to the new PR.

@nofishonfriday
Copy link
Collaborator

@acendan Just in case you're wondering, SWS dev happens rather slowly usually (last official release was February 06, 2021, pre-releases hapen a bit more frequently), but your PR will be processed and merged eventually. :)

@acendan
Copy link
Author

acendan commented Jul 26, 2022

No worries! Crazy to think that last official was over 1.5 years ago at this point. Cheers

@cfillion cfillion mentioned this pull request Oct 11, 2022
25 tasks
// APE: https://www.monkeysaudio.com/developers.html
bool ExtractCoverAPE(TagLib::APE::Tag* tag, const std::string& target)
{
const TagLib::APE::ItemListMap& listMap = tag->itemListMap();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

..\acendan\cover.cpp(160): warning C4996: 'TagLib::MP4::ItemListMap': was declared deprecated
..\acendan\cover.cpp(160): warning C4996: 'TagLib::MP4::Tag::itemListMap': was declared deprecated

Suggested change
const TagLib::APE::ItemListMap& listMap = tag->itemListMap();
const TagLib::APE::ItemMap& listMap = tag->itemMap();

https://taglib.org/api/classTagLib_1_1MP4_1_1Tag.html#ab4ef785d36d566f5bd23d12ac3d8b804

std::string GetTempFilePath(const std::string& extension = ".jpg")
{
char buffer[L_tmpnam];
tmpnam(buffer);
Copy link
Member

@cfillion cfillion Oct 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/home/appveyor/projects/sws/acendan/cover.cpp:68:9: warning: ignoring return value of ‘char* tmpnam(char*)’, declared with attribute warn_unused_result [-Wunused-result]

Suggested change
tmpnam(buffer);
(void)!tmpnam(buffer);

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425#c34

In function `_GLOBAL__sub_I_acendan.cpp.lto_priv.1285':
:(.text.startup+0xf4): warning: the use of `tmpnam' is dangerous, better use `mkstemp'

...which is because it's technically not 100% impossible for another program to use that same temporary name for themselves between the call to tmpnam and when the file is actually created afterwards.

Perhaps this could instead use a known safe & writable path in the user's resource directory (eg. GetResourcePath() + WDL_DIRCHAR_STR "sws_tmpcover.jpg"), or maybe even letting the caller of the API provide a path themselves?

(Or just return the picture data, letting the caller deal with writing to a file/do any other processing they want? realloc_cmd_ptr on the output buffer to send a binary-safe string.)

Are cover pictures always JPEG?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are cover pictures always JPEG?

MP3 tag windows app allows to pick JPEG, JFIF, PNG and WEBP when adding a cover image. No sure if it is standard definition, but we can expect it to be at least JPEG / PNG.

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.

5 participants