Skip to content
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

gstosxaudiodeviceprovider: add device change monitoring. #12

Open
wants to merge 339 commits into
base: main
Choose a base branch
from

Conversation

bitflows
Copy link
Contributor

No description provided.

Stian Selnes and others added 30 commits September 27, 2023 17:46
The pad may be unparented while this macro is called which could result
in a segfault. The new macro protects against this. The parent may still
be disposed while the macro is called, but this will not result in a
crash (but the parent name may be garbage). Using gst_pad_get_parent ()
is undesirable since it takes the object lock.

The patch take advantage of compound expressions available as a C
extension in GCC and some other compilers.

https://bugzilla.gnome.org/show_bug.cgi?id=761916
This fixes a race where priv->passthrough is changed from TRUE to FALSE
while processing a buffer and we end up passing a non-writable buffer to
transform_ip(). More precicely if passthrough is changed just after
prepare_output_buffer() is finished.

Since priv->passthrough and other priv variables are accessed throughout
the chain function a lock is introduced and held while processing the
buffer, but released before pushing downstream. Since sub-classes may
call is_passthrough() and similar functions during for instance
transform_ip() a recursive lock is needed.

https://bugzilla.gnome.org/show_bug.cgi?id=773093
clipped from #gstreamer:

Lets start with some history:
Back in 2008 we needed a dynamic jitterbuffer.
After discussing back and forth with wtay, we came to the conclusion that
the proper way to do this was to:
1. Change the latency property on the jitterbuffer
2. The jb would then post a latency message on the bus
3. in the bus-handler (in the application), this msg would be picked up,
and wtay added a new method gst_bin_recalculate_latency that the
application could emit on the main pipeline to have the whole pipeline
query its latency again (each sink sending upstream latency queries etc).

Now this has worked ok for us ever since, in Pexip we terminate the
latency in the mixers, not in the sinks, but other then that it works
exactly the same way as before. But lately we have discovered that this
has some horrific side-effects, in that with many participants connecting,
and latency changing often, we in fact, using this method,
are recalculating EVERYONES latency when we only want to reconfigure that
of a single participant. or more specifically the jitterbuffers with the
same cname belonging to that participant.

Now, I know the idea of "groups of sinks" have been mentioned before,
but we are at a stage now where we really need to be able to re-calculate
latency only for the affected sinks / mixers, and this is where I have a
concrete suggestions: a downstream "latency-changed" event that in turn
can trigger an upstream latency query, either from a sink or from a mixer.

If this was emitted from the jitterbuffer in the same place the
latency-message is emitted, it would mean all affected downstream
mixers/sinks would know "directly" that latency had changed somewhere
upstream from them, and could make decisions based on this.

For us it would mean re-sending latency-queries upstream from the mixers
that received this event, and then using this new latency in the mix.
For this sink-case, it would be the same way, only that with using
this "mode" the sinks would get their new latency "independent" of the
pipeline latency, and the "grouping" would then happen from whichever
sources decided to send this event.

So in the case of rtpbin, you are already changing latency for all the
jitterbuffers with the same cname as one "group", and hence all of them
would also be sending the latency event, ensuring lipsync at the
mixers/sinks.

https://bugzilla.gnome.org/show_bug.cgi?id=764396
Enable feature with --enable-gst-debug-syslog (default is disabled). Use
it with setting environment variable GST_DEBUG_SYSLOG. The value of the
variable is used as ident for the log message.
Before this could lead to the trans->sinkpad and ->srcpad being
invalid when used.
Or else the next test will just hang waiting to clean up threads
from the test that failed.
This is needed to compile for iOS 64bit with XCode 11.3.1
We want a better more permanent patch here, but this is the smallest
conceivable fix for a dot-release.
- Don't crash if no fundamental type name is found for a GValue
- Print the fundamental type of each GValue in a GstStructure
- Fix type name comparison to GEnum
This is created when using the gst-gdb Python helper script.
Some python-bindings are doing this twice, so we need to be resilient.
We also leak during removal of a function
A very common pipeline is feeding an encoder inheriting from
GstAudioEncoder into a payloader inheriting from GstRtpBasePayload.

If this is the case, the rtp-timestamp will, if perfect_rtptime is TRUE,
reflect the amount of bytes in each packet, and not (like it should)
reflect the timestamp of the packets in the clock-rate domain.

Example:
20ms AAC packets with clock-rate of 90000, should have the rtp-timestamp
incrementing with 90000 * 20 / 1000 = 1800.

Currently, because perfect_rtptime is default TRUE, the AAC-packets
will be incrementing with the buffer-size of the packets, which is
typically 64000 * 20 / 1000 = 1280.

In the case that bitrate and clock-rate are the same (like with ALAW and
MULAW) it will work by chance, which is probably why it has not been
discovered earlier.

https://bugzilla.gnome.org/show_bug.cgi?id=761943
When doing DTX generation in decoders, they are indeed
producing buffers without having a corresponding input
buffer, so this code needs to be revisited, and perhaps
need a special method for pushing buffers like this?

https://bugzilla.gnome.org/show_bug.cgi?id=773106
…eo alignment requirements" [pexhack]

This reverts commit 8b96b52.
Depayloaders should be less strict about spec conformance on incoming
packets.
tbeloqui and others added 23 commits September 27, 2023 18:59
CreateMacOSSurface should only be called from the main thread.
In our dynamic build winsock2 symbols are missing.
gstvkwindow_cocoa: call close on the internal window

Fixes window leak.
And introduce "require-keyfram" property
When shutting down the element, there is a fairly big chance that the
loop is currently waiting for more buffers.

At the point when (de)activate pad is called as part of the state changes,
it will then deadlock because it want to aquire the stream-lock, and
that is held by the loop...

The fix (and this is the proper fix for ALL element that uses a GstTask)
is to implement activatemode_function on the srcpad, so that we have
a chance to stop/pause the loop when the pad is deactivated, to avoid
such deadlocks.
Instead of calling gst_pad_get_current_caps() twice for every buffer,
we simply cache the caps and pass that instead.
To test if the device is actually being hold by another process or not.
A new src that spawns a single thread when it needs to push something,
and then joins said thread and goes back to an idle state, not consuming
system resources.

Co-authored-by: Camilo Celis Guzman <[email protected]>
@bitflows bitflows requested a review from tbeloqui December 18, 2023 13:16
break;
}
}
return 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not return noErr rather than zero?

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.