search

NVIDIA / VideoProcessingFramework

Set of Python bindings to C++ libraries which provides full HW acceleration for video decoding, encoding and GPU-accelerated color space and pixel format conversions
Apache License 2.0
1.32k stars 233 forks source link

Add seek functionality by milliseconds. #491

Closed sotelo closed 1 year ago

sotelo commented 1 year ago

This PR addresses this issue: https://github.com/NVIDIA/VideoProcessingFramework/issues/490

Currently, we're limited to seek at integer second locations (i.e. 1 second, 2 seconds, ...). However, since we usually have more than 1 frame per second, there are situations in which we'd want to seek some intermediate location (i.e. 1.5 seconds). This PR solves that issue by adding a new SeekCriteria (BY_TIMESTAMP_MILLISECONDS) that changes the interpretation of seek_frame from seconds to milliseconds.

I think that there are other better ways of solving this issue (like having 2 different variables int: seek_frame and double: timestamp. However, this is a simple change that is backwards-compatible with the way things work right now.

RomanArzumanyan commented 1 year ago

@sotelo

This PR addresses this issue: https://github.com/NVIDIA/VideoProcessingFramework/issues/490

Don't get me wrong with this, but I'm not quite sure if it does. There's a buggy code which treats uint64_t as double and causes issue. IMHO the fix should address that. Imagine you seek to the 12s exactly using the existing API...

sotelo commented 1 year ago

Thanks for reviewing this @RomanArzumanyan.

It makes sense. I will close this PR.

RomanArzumanyan commented 1 year ago

@sotelo It would be great if you could just brush it up instead of closing. Basically you can extend the SeekContext structure with another member of type double, say timestamp_s. Then you just need to forward it all the way to ffmpeg demuxer class. And that would eradicate the bug.