GNOME Bugzilla – Bug 572854
rtph264pay failed to incorporate PPS for sprop-parameters-set
Last modified: 2009-02-23 14:45:42 UTC
Please describe the problem: When a h264 PPS NAL is sent after a SPS NAL(both NALs are sent as individual discret buffers), the payloader failed to incorporate the PPS into the caps' sprop-parameters-set value. This is due to an extra loop inside gst_rtp_h264_pay_decode_nal (Looping is not necessary because this is already taken care of by handle_buffer()). The data being passed to gst_rtp_h264_pay_decode_nal is already one complete NAL unit. All that is necessary is to test the first byte for a NAL type 7 or 8 to identify SPS/PPS NALs. Steps to reproduce: 1. Send SPS, PPS and h264 coded slice NAL units as individual buffers 2. Observe GST_DEBUG traces. The PPS is never parsed. 3. Get the negotiated caps. the sprop-parameters-set does not contain the PPS part (no comma (,) separating two base64 strings. Only one base64 string that is the SPS). Actual results: Please refer to the GST_DEBUG traces emitted or do a gst_pad_get_negotiated_caps() on the payloader pad after sending both SPS and PPS. Expected results: The sprop-parameters-set value in the caps should contain 2 base64 strings, separated by a comma (,) Does this happen every time? Yes Other information: The patch for the bug is to replace gst_rtp_h264_pay_decode_nal() completely by the function below: static void gst_rtp_h264_pay_decode_nal (GstRtpH264Pay * payloader, guint8 * data, guint size, gboolean * updated) { guint8 *sps = NULL, *pps = NULL; guint sps_len = 0, pps_len = 0; guint8 header, type; guint len; /* default is no update */ *updated = FALSE; GST_DEBUG ("NAL payload len=%u", size); len = size; header = data[0]; type = header & 0x1f; /* keep sps & pps separately so that we can update either one * independently */ if (SPS_TYPE_ID == type) { /* encode the entire SPS NAL in base64 */ GST_DEBUG ("Found SPS %x %x %x Len=%u", (header >> 7), (header >> 5) & 3, type, len); sps = data; sps_len = len; } else if (PPS_TYPE_ID == type) { /* encoder the entire PPS NAL in base64 */ GST_DEBUG ("Found PPS %x %x %x Len = %u", (header >> 7), (header >> 5) & 3, type, len); pps = data; pps_len = len; } else { GST_DEBUG ("NAL: %x %x %x Len = %u", (header >> 7), (header >> 5) & 3, type, len); } /* If we encountered an SPS and/or a PPS, check if it's the * same as the one we have. If not, update our version and * set *updated to TRUE */ if (sps_len > 0) { if ((payloader->sps_len != sps_len) || !is_nal_equal (payloader->sps, sps, sps_len)) { payloader->profile = (sps[1] << 16) + (sps[2] << 8) + sps[3]; GST_DEBUG ("Profile level IDC = %06x", payloader->profile); if (payloader->sps_len) g_free (payloader->sps); payloader->sps = sps_len ? g_new (guint8, sps_len) : NULL; memcpy (payloader->sps, sps, sps_len); payloader->sps_len = sps_len; *updated = TRUE; } } if (pps_len > 0) { if ((payloader->pps_len != pps_len) || !is_nal_equal (payloader->pps, pps, pps_len)) { if (payloader->pps_len) g_free (payloader->pps); payloader->pps = pps_len ? g_new (guint8, pps_len) : NULL; memcpy (payloader->pps, pps, pps_len); payloader->pps_len = pps_len; *updated = TRUE; } } }
commit aeee52be051d7d73cd565e7eaf9b49b84a00c8a2 Author: Wai-Ming Ho <waiming at ailuropoda dot net> Date: Mon Feb 23 15:43:51 2009 +0100 Always add PPS to the sprop-parameters-set Rework the parsing code that under certain circumstances dropped the PPS from the sprop-parameters-set. Fixes #572854.