-
Notifications
You must be signed in to change notification settings - Fork 26
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
feature/support-processing-m3u8-video-playlist-uris #188
feature/support-processing-m3u8-video-playlist-uris #188
Conversation
Codecov Report
@@ Coverage Diff @@
## main #188 +/- ##
==========================================
+ Coverage 94.55% 94.60% +0.04%
==========================================
Files 50 50
Lines 2607 2630 +23
==========================================
+ Hits 2465 2488 +23
Misses 142 142
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Appreciate!
I have two actions running on my dev infrastructure to test:
if both succeed I will link to their page on my dev infra website and then likely merge after approvals! 🎉 |
m3u8_To_MP4.download( | ||
uri, | ||
mp4_file_dir=dst.parent, | ||
mp4_file_name=save_name, | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This downloads to an mp4 file in local storage - could that be a problem on GitHub actions with limited storage?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We store the video on the disk regardless of m3u8 or native mp4. This slightly duplicates storage because it stores chunks and the converted but 🤷 we have never run into problems. Even when processing 8 events at a time
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the limit is 500mb. Are the temp files deleted after they have been hosted, that would help?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
500Mb wouldn't even be possible to store a docker image on.
I think it's 14GB: https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners#supported-runners-and-hardware-resources
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The 500MB must be for logs and artifact files then 😕
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that is likely
Both the normal event gather and the manual process special event succeeded https://jacksonmaxfield.github.io/cdp-dev/#/events/0913ecd21d43 @tohuynh you happy with this PR if so I think I can merge. 🎉 |
yes |
Link to Relevant Issue
CouncilDataProject/cdp-scrapers#96 (comment)
Description of Changes
Include a description of the proposed changes.
Allows processing of m3u8 URIs.
What will happen is during
resource_copy
we will download and convert the m3u8 playlist to an mp4. Then during determining the video host, we check the original session video uri and see if it ends with".m3u8"
and if so, we host the already converted and saved locally m3u8 file.