Skip to content

Streaming Video Fix#290

Merged
BryonLewis merged 3 commits intomasterfrom
video-fix
Aug 17, 2020
Merged

Streaming Video Fix#290
BryonLewis merged 3 commits intomasterfrom
video-fix

Conversation

@BryonLewis
Copy link
Copy Markdown
Collaborator

Changes the video file endpoint to the /file endpoint. This endpoint properly supports streaming of files using the correct range headers. This should fix the issue where it takes an insanely long time to load and playback large videos. The previous version would try to download the entire file up to that one location in the space.

Let me know if there is a better way or pattern to do this.

Fixes #273
Fixes the core of what we can reproduce for #171

item = Item().findOne({'folderId': folder['_id'], 'meta.codec': 'h264',})
if item:
video = Item().childFiles(item)[0]
if video:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Drop this condition. An item tagged with meta.codec should be an item with one file. If the child file is missing, that's effectively a database schema violation and a 500 error would be appropriate.

Alternatively, keep the if and add an else that throws a more verbose exception.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I originally didn't have it in because I assumed that it should have a single file child, I reviewed and didn't know if forever (in terms of future updates) it would have a single file. I'll drop it.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah that's sort of the trouble with girder overall. When you base your entire app on freeform metadata inside folders and items, your schemas get expressed throughout the whole project as code.

By putting an if here you say "I have a union type" and then you have to hope that's consistent with all the other code that manipulates this data.

It scales pretty poorly. That was a large part of my motivation for writing the postprocessing endpoint: I wanted to get all that data-affirming code in one place and let that function be the source of truth for schema.

I guess I should add some logic in that function to verify the results of the processing pipelines. Like, after image or video conversion runs, postprocessing should go through and make sure all the metadata was set.

@BryonLewis BryonLewis merged commit 7cf22d8 into master Aug 17, 2020
@BryonLewis BryonLewis deleted the video-fix branch August 17, 2020 14:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update Video Settings for Streaming

2 participants