rtph264pay: Change aggregate-mode default for RTSP at least

See !2 (comment 527142) for details.

In summary, aggregate-mode=zero-latency is breaking various proprietary NVRs while they worked in previous versions and with aggregate-mode=none. On the other hand Safari WebRTC doesn't work with no aggregation, which is why it was changed at all.

@ndufresne also says that aggregate-mode=none behaviour now matches live555.

added Blocker Regression labels

mentioned in merge request !2 (merged)

Apparently I wasn't right on 555, I received a pcap a while ago, which was suppose to be from that, most likely a modifier version. So you can find the minimalistic payloader here (live555 does not use a public version system like git, so picking from random repo):

https://github.com/cmberryau/live555/blob/master/liveMedia/src/H264or5VideoRTPSink.cpp#L184

So conclusion live555 do not produce any STAP, only FU.

changed the description

changed milestone to %1.17.90

@ndufresne What would you suggest then? Make the old behaviour the default again and document that people should enable zero-latency for WebRTC compatibility?

Not certain, I think rtsp server should explicitly configure it, whatever we pick here (for the RSTP case), and perhaps WebRTCBin have the ability to do the same. So basically we should not leave it to good will of default values in context we can control this.

As for the default, perhaps being conservative (old value) make sense. As I bet pretty old RAW RTP stuff will be affected ?

The payloaders are before webrtcbin in the pipeline - it has no control over them at the moment.

For this use case and also others (like automatic encoder bitrate control), we'll need to build something that makes it possible for WebRTCbin to affect elements outside the pipeline.

RTSP server also can't configure it directly, the payloaders are in the same position as with webrtcbin. In both cases it would have to be the job of the application.

changed the description

I am not sure what to do here.

If in doubt, return to the status quo ante and figure things out from there.

I'm sure we have done webrtc streaming to safari before those changes, is it something that changed recently in safari?

We could add some kind of CONFIGURATION_HINT query for elements to retrieve more info that could guide default values that are not negotiable via caps.

Or (something I've been wanting) a ConfigurationContext on pipelines/bins that elements can use to figure out more about the bigger use case / scenario they operate in.

I tried implementing something like that once, but it was awkward with the GstContext as it exists now. I think the best you can do is to supply a GstContext with a GstStructure that holds an object that elements can retrieve and call API on.

Might also make sense to add an auto enum value and default to that for now so we have more flexibility later once we figure out a mechanism.

We should go back to the original behaviour, and say that webrtc pipelines must configure that.

This type of query is always racy, a sticky upstream event would work better if that existed.

Yes, sticky upstream events would be useful, but that's flying car future for now.

It's not clear to me that this kind of query would always be racy, especially in an rtsp-server / webrtcbin context? Doing a (non-serialised) query on startup and in response to reconfigure events (which are triggered automatically on relink) should be pretty much functionally equivalent, even if convoluted?

The tricky bit with query is that you never know when to do them. Implementations I seen endup adding just enough random location so that it works for the tested pipeline (and add more when it breaks). It's far from elegant.

Let's move back to old behaviour for 1.18 now and if someone feels like it they can implement a query and a better solution. @ndufresne do you want to provide a MR for switching the default or should I?

Also this should be documented in the release notes and maybe in the webrtc docs too or what do you think?

I think it should be documented in webrtcbin but also in the payloader (basically explain that a mode can break webtrc while the other can break RTSP with specific NVR). I won't be able to help with this until August 10th.

Ok, I'll take care of that tomorrow then. Thanks!

assigned to @slomo

mentioned in commit slomo/gst-plugins-good@98852603

mentioned in merge request !692 (merged)

See !692 (merged)

Currently there is no codec/stream specific documentation at all in webrtcbin and I'm not sure where to put it exactly. Does anybody have any suggestions, or do we just keep it as-is?

mentioned in commit slomo/gst-examples@27c88c0d

mentioned in commit slomo/gst-examples@69c1069d

mentioned in merge request gst-examples!22 (merged)

See also gst-examples!22 (merged)

mentioned in commit slomo/gst-examples@bbed24d9

mentioned in commit slomo/gst-plugins-good@0867ede9

mentioned in commit slomo/gst-plugins-good@a9da9c83

mentioned in commit slomo/gst-plugins-good@e9a0307b

closed via commit e9a0307b

mentioned in commit StefanBruens/gst-plugins-good@82daf0b7