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

Fix seeking in review & trim step #1186

Merged
merged 1 commit into from
Sep 9, 2024

Conversation

Arnei
Copy link
Member

@Arnei Arnei commented Sep 5, 2024

Aims at fixing or at least helping with #517.

As suggested in #517, this introduces the webm-duration-fix package to Studio. This should fix the "Preview & Trim" step on Chrome (desktop), allowing for playback and seeking in the recorded video. Furthermore, the duration should now be stored correctly in the video metadata.

This should not change behaviour in Firefox, as it was already working in Firefox. I have not tested other browsers. I have also only tested this with short (~1 minute) recordings.

Note that recordings downloaded directly from Chrome may still not be playable by all and every media player. This is not something this PR can fix.

Aims at fixing or at least helping with elan-ev#517.

As suggested in elan-ev#517, this introduces the
webm-duration-fix package to Studio. This should
fix the "Preview & Trim" step on Chrome (desktop),
allowing for playback and seeking in the recorded
video. Furthermore, the duration should now
be stored correctly in the video metadata.

This should not change behaviour in Firefox, as
it was already working in Firefox. I have not tested
other browsers.

Note that recordings downloaded directly from
Chrome may still not be playable by all and every
media player. This is not something this PR can fix.
@Arnei Arnei added the type:bug Something isn't working label Sep 5, 2024
Copy link
Member

@LukasKalbertodt LukasKalbertodt left a comment

Choose a reason for hiding this comment

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

Hui, thats a nice and easy fix. Code looks good and I tested this: it does indeed fix the isuse. The duration is immediately visible when opening it in chrome or VLC and I can skip around.

Only thing that stops me from merging: I will test this next week with a recording in the ballpark of 1h, just to make sure. But I expect this to also work. So yeah nice, this long standing issue will finally be resolved.

@mtneug
Copy link

mtneug commented Sep 6, 2024

webm-duration-fix has not been updated in 2 years. Maybe fix-webm-duration is a better alternative (also seems to be more popular)?

@Arnei
Copy link
Member Author

Arnei commented Sep 6, 2024

From what I could tell, they both do pretty much the same thing, with fix-webm-duration being slightly more unwieldy to use (forces us to calculate the duration ourselves), but it also works. Giving the particularity of the issue, I don't think it matters much which package we use, so if people would prefer the newer package we can also use that.

@LukasKalbertodt
Copy link
Member

I would agree with Arne: for this kind of dependency an "active development" is not that important.

I just checked with a 1h recording (4k, 1.2GB) and everything worked. The "fixing" was also fast, I couldn't notice it anyway. So I think this can be merged!

@LukasKalbertodt LukasKalbertodt merged commit a85ca0f into elan-ev:master Sep 9, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants