GNOME Bugzilla – Bug 752807
h265parse: Fix sticky event mishandling when stream does not have VPS
Last modified: 2015-08-16 13:40:43 UTC
I get a sticky event mishandling error, for a file which does not have VPS in its stream. gst-discoverer-1.0 for the file times out because of this. In the commit http://cgit.freedesktop.org/gstreamer/gst-plugins-bad/commit/gst/videoparsers/gsth265parse.c?id=cd8c0227c8e5b4322052097131da20c04dbf68d0 the changes were made to set caps only when vps/sps/pps are set But it is mentioned that /* It is not mandatory to have VPS in the stream. But it might * be needed for other extensions like svc */ so i guess we need not check VPS condition.
Created attachment 308049 [details] [review] update caps even if VPS is not present, as it is not mandatory
Patch looks OK but I'm not sure VPS is indeed optional. Can someone confirm? I don't have access to the standard. As a minor nit, if it's not mandatory it might make sense to rephrase the commit message stating so right upfront: "avoid checking for non mandatory VPS NAL" and then add your event mishandling rationale in the following lines.
Created attachment 308421 [details] [review] avoid checking VPS NAL which is optional changed the commit message. The reason i feel VPS is optional is In the code it is commented stating case GST_H265_NAL_VPS: /* It is not mandatory to have VPS in the stream. But it might * be needed for other extensions like svc */ and there is one other check in code, where they check for presence of only SPS and PPS if (nalu.type == GST_H265_NAL_VPS || nalu.type == GST_H265_NAL_SPS || nalu.type == GST_H265_NAL_PPS || (h265parse->have_sps && h265parse->have_pps)) And i did some googling and found few places where they were discussing about it being optional. http://forum.doom9.org/archive/index.php/t-167081.html "1) VPS is optional - agreed"
commit 09992f32dc95be6087fb304d5b577e86f062e5b0 Author: Vineeth TM <vineeth.tm@samsung.com> Date: Thu Jul 30 08:58:48 2015 +0900 h265parse: Avoid checking for Non Mandatory VPS NAL VPS is not mandatory, and need not check for its presence before setting the caps. Because of the check, in streams which don't have VPS, sticky event mishandling happens. https://bugzilla.gnome.org/show_bug.cgi?id=752807