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: Initial commit. Add ReaScript API: AC_GetMediaFileCoverImage() #1658

Closed
wants to merge 1 commit into from

Conversation

acendan
Copy link

@acendan acendan commented Jul 17, 2022

AC_GetMediaFileCoverImage

Description: This function uses TagLib to fetch the embedded image from an audio file. It then writes that image to a temp jpeg file using native OS-specific temp file implementation. The image can then be used via ReaScript API to, for example, display the embedded image for a selected media item. It could also be copied and saved locally as an image file, if desired.

Tested: Win10, OSX 10.14.6, Linux Mint

Win10
Win10 Screenshot

OSX 10.14.6
OSX 10 14 6 Screenshot

Linux Mint
Linux Mint Screenshot

const auto& coverPath = acendan::player.GetCoverPath();
if (!coverPath.empty())
{
std::copy(coverPath.c_str(), coverPath.c_str() + temppathOut_sz, temppathOut);
Copy link
Member

@cfillion cfillion Jul 18, 2022

Choose a reason for hiding this comment

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

This copies temppathOut_sz (typically 1024 bytes) even if the cover path is shorter (buffer over-read).

Suggested change
std::copy(coverPath.c_str(), coverPath.c_str() + temppathOut_sz, temppathOut);
snprintf(temppathOut, temppathOut_sz, "%s", coverPath.c_str());

(or limit to coverPath.size() + 1)

@nofishonfriday
Copy link
Collaborator

nofishonfriday commented Jul 18, 2022

Also some more general things:

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
Copy link
Author

acendan commented Jul 18, 2022

@nofishonfriday

Looking into this right now, so forgive my unfamiliarity with GitHub PRs. When editing the PR, I can use a new branch from my fork, called 'acendan'. However, I'm still only seeing the option to submit the request to SWS's master branch. How should I go about creating a new branch in sws?

Here's a draft of the new PR (not actually submitted for review yet): #1662. It's the same thing, but from a branch acendan:acendan, rather than acendan:master. That appears to be the extent of the branch-related flexibility on my end.

Refer to screenshot below; can't edit the base repository's target branch field.
image

@nofishonfriday
Copy link
Collaborator

nofishonfriday commented Jul 18, 2022

@acendan
That's exactly what I meant, creating a new branch locally (based on current SWS master branch) and submiting that as PR for merging into SWS master.
So everything fine with your new PR in this regard, thanks and sorry for being unclear. :)

@acendan acendan closed this Jul 18, 2022
@acendan
Copy link
Author

acendan commented Jul 18, 2022

@nofishonfriday No worries! Thanks for the explanation. Closed this PR and opened up the new one, at: #1662

Cheers 😄

@nofishonfriday
Copy link
Collaborator

Btw. not sure if you're familiar with it, if you want to make further changes to your PR without creating a new commit you can amend the last (most recent) commit.

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.

4 participants