Allow audio decoder to seek backwards #550
Conversation
facebook-github-bot
added
the
CLA Signed
| # indices. | ||
| # Ultimately, this test compares a "stateful decoder" which calls | ||
| # `get_frames_by_pts_in_range_audio()`` multiple times with a | ||
| # "stateless decoder" (the one here, treated as the reference) |
There was a problem hiding this comment.
I've convinced myself that we should actually keep this helper instead of doing the conversion. Thoughts?
There was a problem hiding this comment.
I think that's fine - then what we're testing is that decoder behaves the same when it seeks to a location "fresh" versus having to seek from some given location, including backwards. That seems reasonable.
| // of the stream. | ||
| // TODO-AUDIO: document why this is needed in a big comment. | ||
| setCursorPtsInSeconds(INT64_MIN); | ||
| } |
There was a problem hiding this comment.
Note that this is INT64_MIN and not 0, because some packets actually start before 0. In one of our assets the first packet is at -1024.
I noticed that passing an arbitrary low value like -999999 makes FFmpeg unhappy and raise and error, but INT64_MIN seems to be understood and correct (although I haven't found docs on this).
3 participants
Towards #549.
Changes are quite subtle, but I'm fairly confident they work as expected, since they pass our robust tests. I'll make sure to write some solid documentation around the mechanisms involved eventually.
Benchmarks results show no perf hit
Benchmark code is the same as in #538, I'm not benchmarking the "backwards seeking" logic.