Skip to content

Use CEF pts for audio#104

Closed
aiden-jeffrey wants to merge 2 commits intocentricular:masterfrom
aiden-jeffrey:aiden/use-cef-pts-audio
Closed

Use CEF pts for audio#104
aiden-jeffrey wants to merge 2 commits intocentricular:masterfrom
aiden-jeffrey:aiden/use-cef-pts-audio

Conversation

@aiden-jeffrey
Copy link
Copy Markdown
Contributor

In this change we use the CEF pts (which is in unix epoch ms) to seed our own internal frame time counter. If we identify a drift greater than 1ms (which we need to allow because of integer rounding), then we reset our counter to the CEF pts.

Note I have had to add some latency to the audio "cefGstOffset" (the value we add to the CEF pts to get to gst-land) in order for there to not to be underflow on the audio branch of the pipeline. The value here (4*audio buffer duration) was arrived at by trial and error.

@MathieuDuponchelle
Copy link
Copy Markdown
Collaborator

Nice :) I think the final solution might prove to be slightly more involved, have you tried to determine if there is indeed drift between the system clock that is used by default and the CEF clock?

I think https://gitlab.freedesktop.org/gstreamer/gstreamer/-/blob/main/subprojects/gst-plugins-bad/sys/aja/gstajasrc.cpp?ref_type=heads#L2751 and below could be useful as a reference, @sdroege can you perhaps offer guidance here? :)

@aiden-jeffrey
Copy link
Copy Markdown
Contributor Author

I think https://gitlab.freedesktop.org/gstreamer/gstreamer/-/blob/main/subprojects/gst-plugins-bad/sys/aja/gstajasrc.cpp?ref_type=heads#L2751 and below could be useful as a reference, @sdroege can you perhaps offer guidance here? :)

Since this improves things, can we merge as-is?

@MathieuDuponchelle
Copy link
Copy Markdown
Collaborator

Since this improves things, can we merge as-is?

I'm afraid I'd rather hold for now until we've had time to properly review. As discussed in matrix you shouldn't be worried about conflicts, you can consider you have an exclusive lock over the codebase until then :)

@aiden-jeffrey
Copy link
Copy Markdown
Contributor Author

Closing in favour of #107

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants