Conversation
zinigor
left a comment
There was a problem hiding this comment.
Tested, found one strange problem: when I upload a video, shortly after it's done, but before it's processed, I get false as the video URL. Is that related to this PR?
Plus there are two comments in the code.
| * An override for the attachment url, which returns back the WPCOM VideoPress processed url. | ||
| * | ||
| * TODO: Fix this so that it will return a VideoPress process url, to ensure that it is in MP4 format. | ||
| * This is most a action proxy to the videopress_get_attachment_url() utility function. |
There was a problem hiding this comment.
I think this sentence could use some rephrasing
| // Get the attached file and if there isn't one, then let's update it with the one from the server. | ||
| $file = get_attached_file( $id ); | ||
| if ( ! $file && is_string( $info['original'] ) ) { | ||
| videopress_download_video( $info['original'], $id ); |
There was a problem hiding this comment.
Do you think we can remove the videopress_download_video function after this? There will be no code calling it, would there?
There was a problem hiding this comment.
I left this for @georgestephanis, since he's a big proponent of downloaded videos. I'm not so sure about them, though. I'll delete as we can always pull this back out of source control later if needed.
ca7dc92 to
f2ff5ee
Compare
|
@zinigor I added a check and now use the original file when transcoding isn't completed and this condition occurs. |
zinigor
left a comment
There was a problem hiding this comment.
Thanks for the additional edits, I have tested again, works well! This time I didn't experience that problem with the false, but since it's not related to this PR, I'm going to test some more for the beta and create a separate issue if it occurs again.
|
@dbtlr This LGTM as-is but needs a rebase. Once that's done I'll give it a quick test and get it merged :-) |
9259dc2 to
4d1966d
Compare
jeherve
left a comment
There was a problem hiding this comment.
This works well in my tests! I rebased, and will merge as soon as Travis completes.
* Readme: remove old release and add skeleton for 4.8. * Changelog: add #6572 * Changelog: add #6567 * Changelog: add #6542 * Changelog: add #6527 * Changelog: add #6508 * Changelog: add #6478 * Changelog: add #6477 * Changelog: add #6249 * Update stable version and remove old version from readme. * Changelog: add 4.7.1 to changelog. * Readme: add new contributor. * Sync: update docblock @SInCE version. Related: #6053 * Changelog: add release post. * changelog: add #6053 * Changelog: add #6413 * Changelog: add #6482 * Changelog: add #6584 * Changelog add #6603 * Changelog: add #6606 * Changelog: add #6611 * Changelog: add #6635 * Changelog: add #6639 * Changelog: add #6684 * Changelog: add #6710 * Changelog: add #6711 * Changelog: add #5461 * Testing list: update Settings UI feedback prompt. Props @MichaelArestad * Changelog: add #6789 * Changelog: add #6778 * Changelog: add #6777 * Changelog: add #6775 * Changelog: add #6755 * Changelog: add #6731 * Changelog: add #6721 * Changelog: add #6705 * Changelog: add #6702 * Changelog: add #6671 * Changelog: add #6637 * Changelog: add #6582 * Changelog: add #6566 * Changelog: add #6555 * Changelog: add #6529 * Changelog: add #6344 * Changelog: add #5763 * Changelog: add #5503 * Changelog: update #6637 changelog. @see 40e115c#commitcomment-21523982 * Changelog: add #6699 * Changelog: add #6632 * Changelog: add #6769 * Changelog: add #6707 * Changelog: add #6590
This PR removes the use of locally downloaded (sideloaded) video files from the user's server and instead uses an attachment URL that points to the processed file for any video headers.
Fixes #6588
To test