GNOME Bugzilla – Bug 753228
rtph265: Sync with rtph264 changes
Last modified: 2015-08-18 12:27:52 UTC
See summary, this should happen before 1.6 release ideally to keep the behaviour of both consistent. Should also double-check if h265parse and h264parse are in sync. The h264 and h265 elements are almost the same except for some codec specific differences.
Assigning this to myself :)
And ideally should also check if the RTP elements are still up to date with the latest spec draft: https://tools.ietf.org/html/draft-ietf-payload-rtp-h265-13
commit 9379933607248b1ad3b1fd715405b64f5fa2b421 Author: Luis de Bethencourt <luis@debethencourt.com> Date: Wed Aug 12 14:59:53 2015 +0100 rtph265depay: prevent trying to get 0 bytes from adapter This causes an assertion and would lead to getting a NULL instead of a buffer. Without proper checking this would easily lead to a segfault. Related to rpth264depay: https://bugzilla.gnome.org/show_bug.cgi?id=737199
Notes: Original discussion of rtp elements for H.265 was in https://bugzilla.gnome.org/show_bug.cgi?id=731263
commit 2d3dc2caa6f12addd5492038b712bb5665fdc1ca Author: Luis de Bethencourt <luis@debethencourt.com> Date: Wed Aug 12 15:14:50 2015 +0100 rtph265depay: Use GST_BUFFER_PTS() instead of GST_BUFFER_TIMESTAMP() Switching to GST_BUFFER_TIMESTAMP() to be consistent with other rtp code.
commit fee3129d495b4355c2716f0f5fe053f9178280a4 Author: Luis de Bethencourt <luis@debethencourt.com> Date: Wed Aug 12 15:49:50 2015 +0100 rtph265depay: implement process_rtp_packet() vfunc For more optimised RTP packet handling: means we don't need to map the input buffer again but can just re-use the mapping the base class has already done. Based on: https://bugzilla.gnome.org/show_bug.cgi?id=750235 https://bugzilla.gnome.org/show_bug.cgi?id=753228
commit 021333a9fc38a67d4054302c55188598c325e98f Author: Luis de Bethencourt <luis@debethencourt.com> Date: Wed Aug 12 16:11:00 2015 +0100 rtph265depay: Insert SPS/PPS NALs into the stream rtph264depay does the same and this fixes decoding of some streams with 32 SPS (or 256 PPS). It is allowed to have SPS ID 0 to 31 (or PPS ID 0 to 255), but the field in the codec_data for the number of SPS or PPS is only 5 (or 8) bit. As such, 32 SPS (or 256 PPS) are interpreted as 0 everywhere. This looks like a mistake in the part of the spect about the codec_data.
commit 4edf2ac1c51011cce4a52306e7c16bae489b436a Author: Luis de Bethencourt <luis@debethencourt.com> Date: Wed Aug 12 16:43:48 2015 +0100 rtph265depay: PPS replaces old PPS if it has the same id https://bugzilla.gnome.org/show_bug.cgi?id=753228
commit ef9b7ef60a388dd0ff76b9543e7165ab299504cc Author: Luis de Bethencourt <luis@debethencourt.com> Date: Wed Aug 12 17:22:42 2015 +0100 rtph265depay: only update the srcpad caps if something else than the codec_data changed h264parse and gstrtph264depay do the same, let's keep the behaviour consistent. As we now include the codec_data inside the stream, this causes less caps renegotiation. https://bugzilla.gnome.org/show_bug.cgi?id=753228
Note from Sebastian: commit 021333a9fc38a67d4054302c55188598c325e98f might not be correct h265 also has a VPS in addition to SPS and PPS
Not updating the codec data in stream-format=avc is just wrong, having incorrect codec_data is worse than not having it at all! If you want the codec data in the stream but not in the caps, use stream-format=avc3. I would revert the effect of 8dda570e4 on stream-format=avc and just support stream-format=avc3 in rtph264depay and just not include the codec_data at all in this case!
commit 397b5d06ec7acac7f2818c0ed01743362fdf4d05 Author: Luis de Bethencourt <luis@debethencourt.com> Date: Wed Aug 12 17:54:52 2015 +0100 rtph265depay: checking if depay has sps/pps nals before insertion Related to: https://bugzilla.gnome.org/show_bug.cgi?id=753430 https://bugzilla.gnome.org/show_bug.cgi?id=753228
Olivier, I assume you are talking about [0] the commit in rtph264depay. I can open a new bug specific to this if you like. I'm worried it might get burried on this thread about making rtph265 in-sync with rtph264. Your point is a valid one. I would like to investigate that a bit further. [0] http://cgit.freedesktop.org/gstreamer/gst-plugins-good/commit/?id=8dda570e477e5848e4ffe80af2c86b93eec85ae1
It seems my objection was already ignored when this was inserted with bug #705333
Olivier, Looks like it :( I don't see a direct answer to your point. This should be revisited.
The problem here is that our definition of avc/avc3 is not the same as the one from the spec, as well as our definition of au is not the same. I don't think there's much we can change here at this point without breaking more things. Should clean that up for 2.0 :)
Are our differences from the spec documented somewhere?
No, but Jan was looking at that recently. Should be written down I guess
commit fd665514ba9f998d55508cd681f2a9c02f0f96a1 Author: Luis de Bethencourt <luis@debethencourt.com> Date: Fri Aug 14 11:49:51 2015 +0100 rtph265depay: copy metadata in the depayloader, but only the relevant ones The payloader didn't copy anything so far, the depayloader copied every possible meta. Let's make it consistent and just copy all metas without tags or with only the video tag. https://bugzilla.gnome.org/show_bug.cgi?id=751774
commit b3418759d93fc0c757cf29f2419f1955addef874 Author: Luis de Bethencourt <luis@debethencourt.com> Date: Fri Aug 14 15:08:08 2015 +0100 rtph265pay: fix buffer leak when using SPS/PPS Fixes a buffer leak that would occur if the pipeline was shutdown while a SPS/PPS header was being created. https://bugzilla.gnome.org/show_bug.cgi?id=741271
commit 585e042fca1536e8fe1c5f47672fdc01e946419d Author: Luis de Bethencourt <luis@debethencourt.com> Date: Sat Aug 15 11:30:36 2015 +0100 rtph265pay: fix potential crash when shutting down A race condition in the state change function may cause buffers to be unreffed while they are still used by the streaming thread in gst_rtp_h265_pay_send_vps_sps_pps() resulting in a crash. Chain up to the parent class first in the state change function to make sure streaming has stopped and only then free those buffers. https://bugzilla.gnome.org/show_bug.cgi?id=741381
commit 5b2ddfb90cadf5d634be1c1b4759a703f5f9e6dd Author: Luis de Bethencourt <luis@debethencourt.com> Date: Sat Aug 15 11:41:40 2015 +0100 rtph265pay: Use GST_WARNING_OBJECT() instead of GST_WARNING() https://bugzilla.gnome.org/show_bug.cgi?id=753228
commit 4075b1112cd00d7758b208abeab5afa5ef0bbcbe Author: Luis de Bethencourt <luis@debethencourt.com> Date: Sat Aug 15 14:45:34 2015 +0100 rtph265pay: Copy metadata in the payloader, but only the relevant ones The payloader didn't copy anything so far, the depayloader copied every possible meta. Let's make it consistent and just copy all metas without tags or with only the video tag. https://bugzilla.gnome.org/show_bug.cgi?id=751774