Dependencies update

[mbattista]

Hi,

I am very sorry about the style of this pull request, since it has a lot of changes and should probably be 3 separated requests.

Changes:

• Update to go 1.19
• update to Node 18
• updated client dependencies to highest possible versions (vue3 support is missing from some packages), which removed some high and critical warnings
• removed go-events as dependency
• fixed a bug where max_fps=0 would lead to an invalid Pipeline
• added adaptive_framerate tag which now streams in the framerate you selected from the dropdown.
• added AV1 tag, metadata and pipeline, which does not work, since the encoding is way too slow

[m1k1o]

Hi, happy to see you contributing again! Don't worry about that.

I am glad that we are going away from go-events, because they had some issue that affected neko as well and they did not merge my fix until this day.

I am concerned about the samples channel, since there can be multiple users that need to receive copy of the sample, it cannot be done from a single channel. We would need to to channel multiplexing for listeners. Have you tried it with multiple users? I'll build it and test it myself, when I get the chance.

[mbattista]

I am concerned about the samples channel, since there can be multiple users that need to receive copy of the sample, it cannot be done from a single channel.

You are right. While it works with two viewers (thats what I tried), it should have a fan out for the samples. I will change that.

[mbattista]

While looking at the code, I realized that pion will duplicate the track for each listener. So no changes needed, since the samples are written to the track.

[m1k1o]

max_fps=0 was actually meant to use the FPS from the screen size. Therefore I would say, it should do what your implemented adaptiveFramerate is meant to do. Because having them both can lead to edge cases / confusion:

• max_fps=0 and adaptiveFramerate=true -> taken value from pipeline.
• max_fps=30 and adaptiveFramerate=true -> taken value from pipeline up to 30?
• max_fps=0 and adaptiveFramerate=false -> what FPS should be now used? default 25?
• max_fps=30 and adaptiveFramerate=false -> should fps be set to 30 regardless on whats on input?

With only max_fps it would be:

• max_fps=0 -> taken value from pipeline.
• max_fps=30 -> taken value from pipeline up to max specified value, now 30.

I agree that current implementation of max_fps is rather fps where current value is used regardless of what is in the pipeline.

TODO:

• [ ] Update versions in alternative dockerfiles (arm, intel).
• [ ] Since many new goroutines were created to replace go-events, need to check if they are not leaking and are closed properly. E.G.

go func() {
    for {
        val, ok := <-someChannel
        if !ok {
            // channel closed
            break
        }

        // do something with val
    }
}()

[m1k1o]

Do you have any idea why I cannot install the packages? Did you face similar issue?

[mbattista]

Do you have any idea why I cannot install the packages? Did you face similar issue?

No. It just went through.

[mbattista]

• I removed the adaptive framerate tag and max_fps = 0 now sets the adaptive framerate internal.
• I updated the other dockerfiles to node and golang versions
• I react to closed channels of channels that are in use. Should we remove the unused channels for now?

[m1k1o]

No. It just went through.

Interestingly it does not work on my ubuntu 20.04 but works on debian. Weird.

Should we remove the unused channels for now?

I'd say, some of them that are redundant should be removed. But some of them are actually already prepared for furute features.

[mbattista]

Removed the wait timer in the webrtc function and create the channel in the manager.

[m1k1o]

Those latest changes fail with panic when changing screen size, because GST closes that channel.

I have been thinking about how it could be scoped.

• Only manager that creates channel should write to it, others can read from it.
• Gst writes to the channel so it should continue to own it.
• Stream sink handles lifecycle of the pipeline, it knows when it starts and when it stops.
• Capture needs to read from this channel, but whenever pipeline stops it needs to stop reading from it and when pipeline starts, it nees to start reading from it again.

So maybe if we had channel in streamsink that notifies capture about when pipeline starts, it could create goroutine that starts reading from it until the pipeline channel is closed. E.G. additional GetPipelineChannel() chan struct{} that fires when pipeline is available.

Or something like channel that returns sample channel once it has been created. chan chan types.Sample though it might seem ugly, but thats what I logically concluded.

[mbattista]

Sorry, my testing were on the wrong commit.

It panics, because the channel is closed and it tries to write into a closed channel. The channel should never close after it is created, since it will always be needed. With this change the panic is gone.

The only thing that could be done, to be more futureproof, is to make it a map of channels on the manager, so e.g. different bandwidths can be done per different map entries.

[m1k1o]

I moved sample channel to AttachAppsink, because you can create pipelines (like broadcast) that do not need channel for samples. It is only added when we are attaching appsink.

[m1k1o]

It looks good from my side, this can be merged if you do not have any objections or pending changes.

[mbattista]

No objections from my side either.