-
Notifications
You must be signed in to change notification settings - Fork 295
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
refactor:calculate encoder pts based on decoder pts #348
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm so far, need to review the RecBuffer()
though
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
almost lgtm
src/AudioPipe.cpp
Outdated
//Get reference to first buffer in queue | ||
auto& frontAudioBuffer = queue.front(); | ||
|
||
if (frontAudioBuffer->GetTimestamp() > 2 * samples + prevPTS) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't be the timestamp threshold for detecting a discontinuity be a static constant and not depending on the samples?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mathematically, i think it's more accurate to use num of samples for discontinuity detection instead of use constant, for the following reasons:
- the discontinuity may happen between rateBlockBuffer and frontAudioBuffer. If constant is used, we allow more tolerance for this case.
- Not sure if it's valid case, is it possible that connection exists all the time, and user switch codecs in between? I think use num of samples is more safe for this case as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, I meant a timestamp threshold converted to the num samples based on the output clock rate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what I meant is that we are detecting the disconituity based on the leftovers of the rateBlockBuffer
which is variable
src/AudioPipe.cpp
Outdated
//Get reference to first buffer in queue | ||
auto& frontAudioBuffer = queue.front(); | ||
|
||
if (frontAudioBuffer->GetTimestamp() > 2 * samples + prevPTS) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what I meant is that we are detecting the disconituity based on the leftovers of the rateBlockBuffer
which is variable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
No description provided.