-
Notifications
You must be signed in to change notification settings - Fork 30
Broken media links when using offload media plugin #64
Comments
Thank you for these changes and the great write-up! The other scenario we've run into is if an audio file is not a child of the story post (happens most often when one person uploads the audio straight to the media library for someone else who's writing the post). The audio is still in an enclosure, but since we don't use the PowerPress plugin we don't get the duration. Would wp_read_audio_metadata work to get duration from enclosures instead of relying on PowerPress? |
Hmmm... I haven't used Powerpress either, so I'm not entirely sure. There might be a way to find the media attachment ID by querying I can work up a proof-of-concept tomorrow when I get back into the office. |
I forgot to mention an important piece: Delicious Brains wrote us a filter so that enclosures hold the actual URL instead of the GUID. I don't know if it's the ideal best practice, but it was our middle ground between changing GUIDs and breaking everything that uses enclosures (we do podcasting from our site). Using the enclosure_links filter in the tweaks plugin constructor: The function itself uses the WP Offload S3 library to do the standard URL replacement:
Edit: sorry, it wouldn't be wp_read_audio_metadata directly, because the enclosure has an URL, not a file path. I think under normal circumstances if you wrote something to get the path from the URL, wp_read_audio_metadata should work. As a disclaimer, however, I haven't actually done that: when we send audio to NPR One, we use a function that Delicious Brains wrote for us that function that copies the file down from S3 temporarily and then uses ID3 to get the duration from that local file. |
@jwcounts as I read the above, I see you have solutions for the issues you found. Are they related to plugins you use that others may not use? And if these issues apply to all, would you be interested in applying your fixes to a PR that could be reviewed for a merge? |
@davidmpurdy: Thanks for sending that filter over. Once the file is uploaded, Wordpress should save the audio file metadata to @eteare: The changes I made are related to a media offload plugin that @davidmpurdy and I are using, but the methods I'm using to accomplish this should be applicable to anyone using WP. I'm employing functions that are either built into Wordpress or normal PHP functions. Once I see if I can address @davidmpurdy 's concerns above, I would be more than happy to help with a PR for a merge. |
@jwcounts: That's a good point. I'm not sure why they downloaded it and ran ID3 directly (I have to admit that I kind of passed the buck to them make it "just work" as part of our evaluation process of their plugin in our workflow). |
Okay, I think I've got it figured out now. I've updated the file at the link provided above, but just in case: There is a function in WP called After that, I'm pulling the audio metadata from I tested this in a stock version of Wordpress, retested after adding Powerpress, and then again after adding WP Offload Media. So far so good. It also seems to be working on my test site, so I'm rolling it out to our production server. |
Resubmitted a bunch of articles from the last couple of weeks, both to the API and NPR One, and so far so good. Image and audio attachments are present and accounted for. @davidmpurdy, would you be willing to test out the changes on your end? |
It looks good on our staging site based on the NPRML being built (is there a sandbox version of StationConnect to test all the way through that I don't know about?). Thank you for doing all this @jwcounts! Edit: I will test it on the live site, too, as soon as we have a story that's ready to send (which could be a while because of people being out the holidays) |
You're welcome @davidmpurdy, and thanks for bringing it up on the Slack! We had been sending out non-working links and junky post data for a while, and I wouldn't have known to fix it if you hadn't sent up a red flag on your end. |
I just pushed my first couple of stories with the update nprml.php from our live site, and can confirm that they play in the app. Thanks again - looks like it's working perfectly for us. |
My colleague David Purdy at KTOO in Anchorage brought this up and I was able to replicate it in my setup.
We are both using the WP Offload Media plugin by Delicious Brains to push our media files to Amazon S3 for service. After uploading to S3, the plugin updates the attachment URL and other metadata, but leaves the attachment GUID untouched.
Any images or audio files in the body of the post are handled properly when sending to the API, since they are either untouched, or completely stripped (in the "text with HTML" and "text" versions respectively). However, the audio and image attachments created in the API all point to the attached media's GUID, which in our case returns a 404 error.
I made some changes to the NPRML.php file in my test and production environments that have sorted the problem out. Here is a link to a copy of the changed file (all changes are between lines 303 - 430):
NPRML.php
The main change is to switch
$audio->guid
towp_get_attachment_url( $audio->ID )
(or$image
when handling images). The benefit of this is that if no media offload plugin is being used, the GUID should be returned.Also, I made some modifications to how filenames are pulled from the image/audio URLs using
parse_url()
andpathinfo()
.Finally, I noticed an issue where we'd have 2 audio entries on a story in the API, because one was pulled from the post attachments and the other was pulled from an enclosure in
wp_postmeta
.When processing the post attachments, I created an array of filenames, and then added a check against that array when processing enclosures.
The text was updated successfully, but these errors were encountered: