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.

Sign up for GitHub

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

Show HLS Playlist for the VideoPress video url, instead of fragmented mp4 file #20839

Merged
merged 4 commits into from
Sep 1, 2021
Merged

Show HLS Playlist for the VideoPress video url, instead of fragmented mp4 file #20839

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3503670
show hls playlist as video url if it exists
jgcaruso Aug 26, 2021
61ecef6
added changelog file
jgcaruso Aug 26, 2021
c1d5f47
use an `isset` to check for `hls` key.
jgcaruso Aug 31, 2021
855cc0b
Merge remote-tracking branch 'origin/master' into update/videopress-v…
jeherve Sep 1, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions projects/plugins/jetpack/changelog/update-videopress-video-url-hls
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: minor
Type: compat

VideoPress file url field now displays the HLS playlist if one was generated. The fragmented mp4 file is not meant to be played on its own. MP4 file will only be displayed as the video url if it was created prior to the HLS update.
6 changes: 5 additions & 1 deletion projects/plugins/jetpack/modules/videopress/utility-functions.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -731,7 +731,11 @@ function videopress_get_attachment_url( $post_id ) {
return null;
}
} else {
$return = $meta['videopress']['file_url_base']['https'] . $meta['videopress']['files']['hd']['mp4'];
$return = $meta['videopress']['file_url_base']['https'] . (
$meta['videopress']['files']['hd']['hls']
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we have it pull the hd_1080p entry if it's available? I think maybe we discussed this on our call but can't remember what decision was made there 😄

Copy link
Contributor

Choose a reason for hiding this comment

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

Or maybe this is fine for now until we have streaming playlists and can just use those 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thats what I was thinking. I don't want to start hard-coding format lists in Jetpack again, and once adaptive is ready we'll just use that.

Copy link
Member

Choose a reason for hiding this comment

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

Should we check if $meta['videopress']['files']['hd']['hls'] is set before we check for it here, just like we do for $meta['videopress']['files']['hd']['mp4'] above?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I can change the condition here to be more explicit with an isset().

? $meta['videopress']['files']['hd']['hls']
: $meta['videopress']['files']['hd']['mp4']
);
}

// If the URL is a string, return it. Otherwise, we shouldn't to avoid errors downstream, so null.
Expand Down