Encoding: allow user-defined encoded sample rate #700
Conversation
…mple_rate_lezzzgo
…mple_rate_lezzzgo
facebook-github-bot
added
the
CLA Signed
…mple_rate_lezzzgo
…mple_rate_lezzzgo
src/torchcodec/_core/Encoder.h
Outdated
| // │ │ | ||
| // AVFrame from maybeFlushSwrBuffers() ───┘ │ | ||
| // Only if sample rate conversion was needed | ||
| // nullptr, to flush |
src/torchcodec/_core/Encoder.cpp
Outdated
| swrContext_.get(), avFrame->data, avFrame->nb_samples, NULL, 0); | ||
| avFrame->nb_samples = actualNumRemainingSamples; | ||
|
|
||
| encodeFrameThroughFifo(autoAVPacket, avFrame, /*andFlushFifo=*/true); |
There was a problem hiding this comment.
I had to think about what's going on here for a while, let me know if I'm correct: we sometimes need to encode a frame through a FIFO because of various conversion reasons. We also need to make sure we flush our buffers, sometimes, in order to get the final frames. And those frames also need to sometimes go through the FIFO for the same reasons as above. But when we do the final flushing encoding through a FIFO, we need to do some special logic to make sure we're not leaving a frame behind in the FIFO. Correct?
We have the excellent comment in the header, but I think saying something here and at the top of the while loop in encodeFrameThroughFifo() could help readers put all this together.
There was a problem hiding this comment.
Yes that's all correct. Something I may not have been made clear yet, and I will add comments to that end:
The use of the FIFO applies to all frames, or to none of them. i.e. a given Encoder instance will either:
- create and use the FIFO for every single frame
- not use the FIFO.
I hope I was able to clarify all this a bit in fa0856a (#700)
This PR allows the user to specify a desired sample rate for the encoded output.
Unlike other conversions (format, num_channels), this one is far from trivial. It involves, in some specific scenarios, the use of an intermediate FIFO to store samples before they get encoded. I tried to document all of this in the
Note: [Encoding loop, sample rate conversion and FIFO], so I recommend starting the review from there.