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

Add Airplay cover art to metadata #543

Merged
merged 2 commits into from
Feb 25, 2020
Merged

Conversation

cmfcmf
Copy link
Contributor

@cmfcmf cmfcmf commented Feb 2, 2020

This adds a new "COVER" attribute to the metadata, which is supported by Airplay and contains the base64 encoded cover.
To make it work, I also had to fix an issue with the control session handler which would sometimes produce 'Bad address' errors (particularly with large covers).
I have documented more details in the comments I added.

// operation. When the memory is no longer available, the buffer is said to have been invalidated.
// https://www.boost.org/doc/libs/1_72_0/doc/html/boost_asio/reference/buffer.html#boost_asio.reference.buffer.buffer_invalidation

string& message = messages_.front();
Copy link
Owner

Choose a reason for hiding this comment

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

this bug is a nasty one, great that you found it. But it's not related to the metadata, right? It looks like meta data is sent to the streaming clients, and not to the control clients.
Also I think, I would prefer adding the \r\n when adding the message to the queue:

void ControlSessionTcp::sendAsync(const std::string& message)
{
    strand_.post([ this, self = shared_from_this(), message ]() {
        messages_.emplace_back(message + "\r\n");

Copy link
Contributor Author

@cmfcmf cmfcmf Feb 25, 2020

Choose a reason for hiding this comment

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

Thank you for merging!

While the code may not be related to the metadata handling, the issue only surfaced when I sent big metadata cover images. I assume that the the big metadata strings resulted in a different/more rapidly changing/... stack/heap, causing the undefined behavior created by this bug to actually do something bad (crash the control server).

#endif

void pipeReadLine();
#ifdef HAS_EXPAT
int parse(std::string line);
void createParser();
void push();
void setMetaData(const std::string&, const std::string&);
Copy link
Owner

Choose a reason for hiding this comment

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

please add parameter names

@badaix badaix merged commit 2efe445 into badaix:develop Feb 25, 2020
@cmfcmf cmfcmf deleted the airplay-cover-art branch February 25, 2020 22:46
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