Skip to content

Use our own index to seek more accurately when it is available #180

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

Merged
merged 8 commits into from
Aug 15, 2024

Conversation

ahmadsharif1
Copy link
Contributor

@ahmadsharif1 ahmadsharif1 commented Aug 14, 2024

Before this change when the user asks us to seek to some timestamp T, we pass that timestamp down to FFMPEG.
However, we have noticed that sometimes FFMEPG seeks past that timestamp for certain files:
#179
https://trac.ffmpeg.org/ticket/11137
After this change if we have our own index, we use that to seek to the last keyframe before the target frame. This avoids the buggy behavior and seeks are accurate even for the H265 encoded videos.

Benchmarked this change and didn't find changes above the noise level.

# Before
python ~/personal/torchcodec/benchmarks/decoders/benchmark_decoders2.py 
med = 48.34ms +- 7.39

# After
python ~/personal/torchcodec/benchmarks/decoders/benchmark_decoders2.py 
med = 44.17ms +- 6.23

# Benchmark code
$ cat ~/personal/torchcodec/benchmarks/decoders/benchmark_decoders2.py
from time import perf_counter_ns
from torchcodec.decoders import SimpleVideoDecoder
import torch
import os


def bench(f, *args, num_exp=100, warmup=10, **kwargs):
    video_path = os.path.join(os.path.dirname(__file__), "../../test/resources/nasa_13013.mp4")
    decoder = SimpleVideoDecoder(video_path)
    pts_list = [x * 0.5 for x in range(20)]
    for _ in range(warmup):
        frames = [decoder.get_frame_displayed_at(pts) for pts in pts_list]

    times = []
    for _ in range(num_exp):
        start = perf_counter_ns()
        frames = [decoder.get_frame_displayed_at(pts) for pts in pts_list]
        end = perf_counter_ns()
        times.append(end - start)
    return torch.tensor(times).float()

def report_stats(times, unit="ms"):
    mul = {
        "ns": 1,
        "µs": 1e-3,
        "ms": 1e-6,
        "s": 1e-9,
    }[unit]
    times = times * mul
    std = times.std().item()
    med = times.median().item()
    print(f"{med = :.2f}{unit} +- {std:.2f}")
    return med

if __name__ == "__main__":
    times = bench(lambda x: x, num_exp=1000, warmup=10)
    report_stats(times, unit="ms")

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Meta Open Source bot. label Aug 14, 2024
@ahmadsharif1 ahmadsharif1 marked this pull request as ready for review August 14, 2024 16:12
Copy link
Member

@NicolasHug NicolasHug left a comment

Choose a reason for hiding this comment

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

Thanks a ton for the fix and for the benchmark @ahmadsharif1.

Unrelated to this PR, but I'm surprised the stddev is so large even over 1000 runs!

@ahmadsharif1 ahmadsharif1 merged commit 0a06c3d into pytorch:main Aug 15, 2024
17 checks passed
@ahmadsharif1 ahmadsharif1 deleted the tsfix branch August 15, 2024 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Meta Open Source bot.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants