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

KMS-CORE: SPS/PPS information not provided to clients during H264 video call #321

Closed
leetal opened this issue Dec 17, 2018 · 11 comments

Comments

3 participants
@leetal
Copy link

commented Dec 17, 2018

SPS/PPS information is not sent to clients when H264 has been negotiated and video is flowing. This will cause iOS (and perhaps other clients as well) to not regain decoding video if the application is for example backgrounded and then again maximized. This is due to the fact that the IDR frames does not embed PPS/SPS information if that parameter is not set.

KMS Version:

6.9.0

Other libraries versions:

ii gstreamer1.5-libav:amd64 1.8.2.1.xenial~20181023163456.96.493eee4 amd64 libav plugin for GStreamer
ii gstreamer1.5-nice:amd64 0.1.15-1kurento2 amd64 ICE library (GStreamer 1.5 plugin)
ii gstreamer1.5-plugins-bad:amd64 1.8.1.1.xenial~20181023161445.100.3db37b1 amd64 GStreamer plugins from the "bad" set
ii gstreamer1.5-plugins-base:amd64 1.8.1.1.xenial~20181023154838.55.7b19cfd amd64 GStreamer plugins from the "base" set
ii gstreamer1.5-plugins-good:amd64 1.8.1.1.xenial~20181023160032.112.9ee4248 amd64 GStreamer plugins from the "good" set
ii gstreamer1.5-plugins-ugly:amd64 1.8.1.1.xenial~20181023162927.89.2685b0f amd64 GStreamer plugins from the "ugly" set
ii gstreamer1.5-pulseaudio:amd64 1.8.1.1.xenial~20181023160032.112.9ee4248 amd64 GStreamer plugin for PulseAudio
ii gstreamer1.5-x:amd64 1.8.1.1.xenial~20181023154838.55.7b19cfd amd64 GStreamer plugins for X11 and Pango
ii kms-core 6.9.0.xenial.20181210200440.bee54d3 amd64 Kurento Core module
ii kms-elements 6.9.0.xenial.20181210201219.6d6f745 amd64 Kurento Elements module
ii kms-filters 6.9.0.xenial.20181210201843.08e687c amd64 Kurento Filters module
ii kms-jsonrpc 6.9.0.xenial.20181210195903.71b45ec amd64 Kurento JSON-RPC library
ii kmsjsoncpp 1.6.3.xenial.20181023152331.d78deb7 amd64 Kurento jsoncpp library
ii kurento-media-server 6.9.0.xenial.20181210202449.f940867 amd64 Kurento Media Server
ii libgstreamer-plugins-bad1.5-0:amd64 1.8.1.1.xenial~20181023161445.100.3db37b1 amd64 GStreamer development files for libraries from the "bad" set
ii libgstreamer-plugins-base1.5-0:amd64 1.8.1.1.xenial~20181023154838.55.7b19cfd amd64 GStreamer libraries from the "base" set
ii libgstreamer1.5-0:amd64 1.8.1.1.xenial~20181023153550.170.0d6031b amd64 Core GStreamer libraries and elements
ii libnice10:amd64 0.1.15-1kurento2 amd64 ICE library (shared library)
ii openh264-gst-plugins-bad-1.5:amd64 1.8.1.1.xenial~20181023161445.100.3db37b1 amd64 GStreamer plugins from openh264

Client libraries

  • Language: Java
  • Version: 6.8.0(?)

Browsers tested

  • Chrome: NO_TEST
  • Firefox: NO_TEST
  • Native: iOS, Android, Windows

System description:
Standard setup. Kurento-client on same machine as kurento-media-server.

What steps will reproduce the problem?

  1. On iOS, receive arbitrary H264 video.
  2. Minimize application so that decoder cannot run (not allowed in background).
  3. Maximize application again, video will be black.

What is the expected result?
Continue decode frames as nothing had happened.

What happens instead?
Video is black and decoding fails since the IDR NAL units does not contain format information.

Please provide any additional information below.
I have done some digging and found out that the "config-interval" gstreamer parameter is not set due to wrong type-checks. Newer versions of gstreamer have updated this parameter to support gint instead of the former guint. Thus this parmeter is not set since you have a check that verifies its type as G_TYPE_UINT (guint). My patch also modifies the configuration interval to "-1" instead of "1" which essentially means that whenever the clients ask for a H264 keyframe, also embed SPS/PPS information in the NAL unit IDR frames.
(gstreamer docs: HEAD ref: config-interval)

I have patched (and tested) a 6.9.0 build of KMS and can verify this indeed fixes the issue!
See attached patch file. I can provide a PR to kms-core if that is applicable as well.

@leetal leetal changed the title SPS/PPS information not provided to clients during H264 video call KMS-CORE: SPS/PPS information not provided to clients during H264 video call Dec 17, 2018

@j1elo

This comment has been minimized.

Copy link
Contributor

commented Dec 19, 2018

Hi, this looks good but I'll need studying a bit about the topic to fully understand the issue.
For now keep in mind that Kurento uses a fork of GStreamer 1.8, so it doesn't matter if newer GStreamer versions updated the 'config-interval' data type. Or you meant that it was changed before 1.8?

(This is interesting nonetheless because in our mid-term roadmap there is upgrading to a more recent version of Gst)

@leetal

This comment has been minimized.

Copy link
Author

commented Dec 20, 2018

Yes, sorry for being unclear. They have probably changed that value type before 1.8. I have seen some more issues here that might relate to this fix as well. We have been experiencing lots of quirks with h264 over all platforms (not just iOS) due to that parameter not being set that, when patched just magically disappeared :)

@prlanzarin

This comment has been minimized.

Copy link

commented Jan 30, 2019

@leetal @j1elo Seems like I was chasing this issue too, but didn't skim for this in the bugtracker.
I had opened a pull request (Kurento/kms-core#15) with a patch very much like the one @leetal described before I've seen this. Nonetheless, I've confirmed that the forked version of gstreamer does depend on the G_INT type (and so does gst >= 1.8.x), so the patch is sounding.

@j1elo

This comment has been minimized.

Copy link
Contributor

commented Jan 30, 2019

Good, sorry for the delay, we're pretty busy porting everything to Ubuntu 18.04.
@leetal your patch seems more complete than Kurento/kms-core#15 especially by taking care of the actual values set to "config-interval" (-1 instead of 1), so please could you open a PR? I'll review and merge.

@prlanzarin

This comment has been minimized.

Copy link

commented Jan 30, 2019

My only issue with setting -1 instead of 1 is that gstreamer tends to insert buffered SPS/PPS for every IDR-slice when H.264 is in that format. That doesn't seem to happen with fragmentation unit based streams, but since legacy, pure RTP endpoints tend to rely on IDR slices, I'd be careful setting that value instead of 1 @j1elo.

Inserting SPS/PPS before every slice is clearly not ok and makes some decoders stutter while also increasing the stream size considerably. It's more of a gstreamer issue than Kurento.

@prlanzarin

This comment has been minimized.

Copy link

commented Jan 30, 2019

I'll provide a pcap asap with the aforementioned behaviour with -1 just so you guys can see what I'm trying to say.

@prlanzarin

This comment has been minimized.

Copy link

commented Jan 30, 2019

sps-pps-minus1
sps-pps-1

Two images in annex. It's a pure RTP endpoint receiving two streams from Kurento. The garbled one has config-interval set to -1, while the ok one has config-interval set to 1.

Here are the pcaps of the tests depicted in the images (cf-1: config-interval = 1; cf-minus-1: config-interval = -1). They've been filtered to show only the incoming streams from Kurento.
https://drive.google.com/drive/folders/1YbNP73lEShTfRLPfvM473bI5FNH0p8cQ?usp=sharing

@leetal Have you tested setting 1 on your setup? FIY, we've also been using 1 for our webrtc endpoints.

@leetal

This comment has been minimized.

Copy link
Author

commented Jan 30, 2019

@prlanzarin Yes we have tested both and the only difference we can see locally is the fact that it takes 1 sec for the stream to regain control (since setting 1 will cause SPS/PPS information to be sent once every second, whilst -1 sends SPS/PPS with every IDR-slice). I'll agree that -1 is probably too aggressive and can cause pure RTP to never fully regain decoding capabilities since it will constantly have to adapt to the SPS/PPS NALs. I'd agree that setting 1 instead of -1 will significantly reduce the total transfer sizes and is also probably better for everyone (legacy RTP as well as WebRTC-black-magic guys). Setting 1 is still far better than the functionality right now (totally broken).

@j1elo I'll open a PR with the config-interval set to 1 since that was the old behaviour as well.

@prlanzarin

This comment has been minimized.

Copy link

commented Jan 30, 2019

I've also updated my PR with the paytreebin modification of your patch, but feel free to close that one and open a new one with your patch.

@leetal

This comment has been minimized.

Copy link
Author

commented Jan 30, 2019

Nah, let's use yours then :)

@j1elo j1elo added this to To do in Release 6.10 via automation Mar 11, 2019

@j1elo j1elo moved this from To do to In progress in Release 6.10 Mar 11, 2019

@j1elo

This comment has been minimized.

Copy link
Contributor

commented Mar 11, 2019

Merged Kurento/kms-core#15, hopefully this solves the issue with H.264 SPS and PPS nal units

@j1elo j1elo closed this Mar 11, 2019

Release 6.10 automation moved this from In progress to Done Mar 11, 2019

@j1elo j1elo self-assigned this Apr 9, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.