Skip to content

Fallback to best_effort_timestamp in case of invalid PTS #2916

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

Fallback to best_effort_timestamp in case of invalid PTS #2916

wants to merge 2 commits into from

Conversation

mthrok
Copy link
Collaborator

@mthrok mthrok commented Dec 12, 2022 •
edited
Loading

If the input video has invalid PTS, the current precise seek fails except when seeking into t=0.

This commit updates the discard mechanism to fallback to best_effort_timestamp in such cases.

best_effort_timestamp is just the number of frames went through decoder starting from the beginning of the file.

This means if the input file is very long, but seeking towards the end of the file, the StreamReader still decodes all the frames.

For videos with valid PTS, best_effort_timestamp should be same as pts. [src]

@mthrok mthrok requested a review from jdsgomes December 12, 2022 22:17
@jdsgomes
Copy link
Contributor

Thanks for making the change. it looks good to me, happy to approve once its ready

@mthrok
Copy link
Collaborator Author

mthrok commented Dec 13, 2022

@jdsgomes Do you know where the video "RATRACE_wave_f_nm_np1_fr_goo_37.avi" come from?
Looking at pytorch/vision#1303, it seems from one of HMDB-51, UCF-101 and Kinetics-400, but I cannot tell which and I need to check the license.

@jdsgomes
Copy link
Contributor

@jdsgomes Do you know where the video "RATRACE_wave_f_nm_np1_fr_goo_37.avi" come from? Looking at pytorch/vision#1303, it seems from one of HMDB-51, UCF-101 and Kinetics-400, but I cannot tell which and I need to check the license.

@bjuncek do you know?

@YosuaMichael
Copy link

YosuaMichael commented Dec 20, 2022 •
edited
Loading

@mthrok You can see where it comes from in this readme: https://github.com/pytorch/vision/blob/main/test/assets/videos/README

and seems like the video RATRACE_wave_f_nm_np1_fr_goo_37.avi comes from hmdb51

@mthrok mthrok marked this pull request as ready for review December 20, 2022 15:30
@facebook-github-bot
Copy link
Contributor

@mthrok has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@mthrok
Fallback to best_effort_timestamp in case of invalid PTS
dff90eb 
If the input video has invalid PTS, the current precise seek fails
except when seeking into t=0.

This commit updates the discard mechanism to fallback to
`best_effort_timestamp` in such cases.

`best_effort_timestamp` is just the number of frames went through
decoder starting from the beginning of the file.

This means if the input file is very long, but seeking towards the
end of the file, the StreamReader still decodes all the frames.

For videos with valid PTS, `best_effort_timestamp` should be same as
`pts`.
@facebook-github-bot
Copy link
Contributor

@mthrok has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@mthrok has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@mthrok merged this pull request in c6bc65f.

@github-actions
Copy link

Hey @mthrok.
You merged this PR, but labels were not properly added. Please add a primary and secondary label (See https://github.com/pytorch/audio/blob/main/.github/process_commit.py)

@mthrok mthrok deleted the update-seek branch December 21, 2022 01:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jdsgomes jdsgomes Awaiting requested review from jdsgomes

Assignees
No one assigned
Labels
CLA Signed improvement Merged module: IO
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mthrok @jdsgomes @YosuaMichael @facebook-github-bot