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

jgcaruso · 2021-08-26T16:18:56Z

This PR exposes the HLS playlist as the video url for all videos that were uploaded and transcoded using the new transcode process (default for all videopress uploads, no options for not transcoding this way). This change is in response to some support requests where 3rd party video players were being used on a site, instead of the VideoPress video player (which isn't a scenario that we directly support, but seems to have been working fine until we started working on the video streaming improvements).

A user's process for configuring the 3rd party player was to copy the video url from the Media Library and provide it to the player. The result of this was the video file was not playing until the entire file was downloaded since it is a fragmented mp4 file (due to the new transcode process). The implication of this change is that the 3rd party player will now need to support HLS playlists in order to play the video.

Fixes 931-gh-Automattic/greenhouse

Changes proposed in this Pull Request:

If an HLS file is present in the VideoPress video's metadata, expose it as the "Url" in the media library instead of the mp4 file. The mp4 file is a fragmented file that won't play in the browser until it is fully downloaded.
If an HLS file is not present (for all older VideoPress videos) then the mp4 file will be exposed as the "Url" in the media library.

Jetpack product discussion

Not a Jetpack Product discussion, but a description of the issue and discussion with HEs 928-gh-Automattic/greenhouse
If this seems like something that needs a larger discussion, I'm happy to start a P2 post for it.

Does this pull request change what data or activity we track or use?

nope

Testing instructions:

On a Jetpack site with VideoPress enabled, visit the Media Library
Upload a new video (or open a recently uploaded video). The url property should be a url that ends in .master.m3u8 if it was uploaded recently (the last month or so depending on the type of user being tested with)
For older videos, a url ending in .mp4 should be displayed

github-actions · 2021-08-26T16:22:19Z

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

✅ Include a description of your PR changes.
✅ All commits were linted before commit.
✅ Add a "[Status]" label (In Progress, Needs Team Review, ...).
✅ Add testing instructions.
✅ Specify whether this PR includes any changes to data or privacy.
✅ Add changelog entries to affected projects

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖

The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available.

Once your PR is ready for review, check one last time that all required checks (other than "Required review") appearing at the bottom of this PR are passing or skipped.
Then, add the "[Status] Needs Team review" label and ask someone from your team review the code.
Once you’ve done so, switch to the "[Status] Needs Review" label; someone from Jetpack Crew will then review this PR and merge it to be included in the next Jetpack release.

Jetpack plugin:

Next scheduled release: September 7, 2021.
Scheduled code freeze: August 30, 2021.

roundhill · 2021-08-26T22:13:38Z

projects/plugins/jetpack/modules/videopress/utility-functions.php

@@ -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']


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 😄

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

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.

roundhill

Tested well for me.

jeherve · 2021-08-31T10:11:08Z

projects/plugins/jetpack/modules/videopress/utility-functions.php

@@ -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']


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?

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

…ideo-url-hls

* master: tools: Tool to report unreleased projects (#20874) Add max length for stacktrace in Slack notifications (#20913) Show HLS Playlist for the VideoPress video url, instead of fragmented mp4 file (#20839) VideoPress: Fix VideoPress Attachment Deletions on wp.com (#20832) Update various JS dependencies (#20890) Widget Visibility: add scaffolding for upcoming editor features. (#20910) Heartbeat: Fix typo (#20901) Social Icons: fix being able to remove icons (#20899) Update To-Test.md for Jetpack 10.1 (#20894) Update dependency size-limit to v5 (#20893) Release: start 10.2 cycle (#20896) Jetpack 10.1 Changelog (#20892) E2E tests: add report name option in Slack notification (#20887) Carousel: add more specific selectors for the carousel cursor (#20882) E2E Tests: Fix pre-connection test (#20888) Search: fix body scroll for overlay open/close (#20733) E2E Tests: Fix mailchimp test (#20855) E2E tests: improve error handling (#20884)

jgcaruso added 2 commits August 26, 2021 11:56

show hls playlist as video url if it exists

3503670

added changelog file

61ecef6

jgcaruso added the [Feature] VideoPress A feature to help you upload and insert videos on your site. label Aug 26, 2021

jgcaruso requested a review from roundhill August 26, 2021 16:18

jgcaruso self-assigned this Aug 26, 2021

github-actions bot added the [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ label Aug 26, 2021

github-actions bot added the [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! label Aug 26, 2021

jgcaruso added [Status] Needs Team Review and removed [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! labels Aug 26, 2021

roundhill reviewed Aug 26, 2021

View reviewed changes

roundhill previously approved these changes Aug 30, 2021

View reviewed changes

jgcaruso added [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. and removed [Status] Needs Team Review labels Aug 30, 2021

jeherve requested changes Aug 31, 2021

View reviewed changes

use an isset to check for hls key.

c1d5f47

jgcaruso dismissed roundhill’s stale review via c1d5f47 August 31, 2021 13:53

jgcaruso added [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. and removed [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! labels Aug 31, 2021

jgcaruso requested a review from jeherve August 31, 2021 13:54

jeherve approved these changes Sep 1, 2021

View reviewed changes

jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Sep 1, 2021

jeherve added this to the jetpack/10.2 milestone Sep 1, 2021

Merge remote-tracking branch 'origin/master' into update/videopress-v…

855cc0b

…ideo-url-hls

jgcaruso merged commit 4409f37 into master Sep 1, 2021

jgcaruso deleted the update/videopress-video-url-hls branch September 1, 2021 14:49

github-actions bot removed the [Status] Ready to Merge Go ahead, you can push that green button! label Sep 1, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

jgcaruso commented Aug 26, 2021 •

edited

Loading

github-actions bot commented Aug 26, 2021 •

edited

Loading

roundhill Aug 26, 2021

roundhill Aug 26, 2021

jgcaruso Aug 27, 2021

roundhill left a comment

jeherve Aug 31, 2021

jgcaruso Aug 31, 2021

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

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

Conversation

jgcaruso commented Aug 26, 2021 • edited Loading

Changes proposed in this Pull Request:

Jetpack product discussion

Does this pull request change what data or activity we track or use?

Testing instructions:

github-actions bot commented Aug 26, 2021 • edited Loading

roundhill Aug 26, 2021

Choose a reason for hiding this comment

roundhill Aug 26, 2021

Choose a reason for hiding this comment

jgcaruso Aug 27, 2021

Choose a reason for hiding this comment

roundhill left a comment

Choose a reason for hiding this comment

jeherve Aug 31, 2021

Choose a reason for hiding this comment

jgcaruso Aug 31, 2021

Choose a reason for hiding this comment

jgcaruso commented Aug 26, 2021 •

edited

Loading

github-actions bot commented Aug 26, 2021 •

edited

Loading