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.

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

Read stream dimensions from stream packets #87

Merged
merged 3 commits into from
Apr 7, 2021

Conversation

luludotdev
Copy link
Contributor

@luludotdev luludotdev commented Feb 23, 2021

Closes #25

Caveats: First metadata report will use the fallback (wrong) values since keyframe data is likely not ready in time.
Is also worth in future refactoring PreviewGenerators to a more generic interface for video decoding.

@haydenmc
Copy link
Member

haydenmc commented Mar 6, 2021

Hi @lolPants , sorry for the delay on this! Glimesh.tv just launched and we've been scrambling to introduce stability fixes. I promise this change has not been forgotten!

@luludotdev luludotdev force-pushed the fix/read-video-size branch from a27e842 to 667001b Compare March 10, 2021 06:09
@luludotdev
Copy link
Contributor Author

Rebased onto latest master, I'm intending to do a bit of cleanup and refactor PreviewGenerator to VideoDecoder. Mostly just internal renames though.

@luludotdev luludotdev force-pushed the fix/read-video-size branch from a094a2a to 0d1a5b6 Compare March 15, 2021 21:02
@luludotdev
Copy link
Contributor Author

Rebased again to resolve merge conflicts.

@luludotdev
Copy link
Contributor Author

Any updates on this, it's been over a month now.

@haydenmc
Copy link
Member

haydenmc commented Apr 7, 2021

I think this looks good - I have two concerns, but neither of them are things I'd consider blocking:

1.) I don't think we need to decode an entire keyframe to get the width and height of the video - the SPS NAL should contain the width and height, so we could probably just pull that out without having to feed any data through libav at all.
2.) With this change the metadata report thread will now decode each keyframe twice - first to get the resolution, and then again to generate the preview image.

I don't think these are concerning enough to block the merge, so I'm just opening an issue to track here so we make improvements going forward: #120

@haydenmc haydenmc merged commit f477c76 into Glimesh:master Apr 7, 2021
@luludotdev luludotdev deleted the fix/read-video-size branch April 10, 2021 21:40
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.

Read video height/width from actual stream packets instead of ingest info
2 participants