GNOME Bugzilla – Bug 732267
h264parse: extract base stream from MVC or SVC encoded streams
Last modified: 2018-11-03 13:24:25 UTC
If downstream decoder, or an intermediate capsfilter, requests caps with only Annex.A profiles, then h264parse should discard any MVC (Annex.H) specific NAL units from the output stream.
It is also possible to cover base layers from SVC encoded streams. That's a detail of implementation. I foresee the following patch: (i) if downstream caps only report Annex.A profiles then set "base_only" variable to TRUE; (ii) if base_only is set to TRUE, discard any NAL unit that does not pertain to the Annex.A set of nal_unit_type. Filtering out SEI messages is optional, but desired too. This will be tracked in another bug because this is non-essential to the decoding process, and if we keep those, the decoding process won't be perturbed.
The same could also be applied to Annex.I (MVCD). So, making this bug report generic enough.
This subsequently depends on bug #732203 as the resulting access unit buffer should really drop the non-Annex.A NAL units.
Created attachment 356212 [details] [review] h264parse: extract base view from MVC or SVC encoded streams This patch was tested with the ITU-T h264 test vectors from https://www.itu.int/net/itu-t/sigdb/spevideo/VideoForm-s.aspx?val=102002641. Specifically, all the *.264 files from "ITU-T_H.264.1(2016-02)_SVC_bitstreams_part{1,2,3,4}.zip" and "ITU-T_H.264.1(2016-02)_MVC_bitstreams.zip". The changes were only tested with the vaapih264dec decoder. This patch won't make avdec_h264 work with the base views of MVC / SVC bitstreams because it doesn't report supported profiles in it's caps. There are also some problems with the *-L0.264 files from the SVC bitstreams because there aren't any subset sps units there: base_only is not set to TRUE and the negotiated profile is not an SVC one. A workaround for both of these issues is to init base_only to TRUE and set it to FALSE only when there is a subset sps unit in the bitstream and the downstream element supports it's profile. If this method is preferred the change should be trivial.
(In reply to Orestis Floros from comment #4) > The changes were only tested with the vaapih264dec decoder. This patch won't > make avdec_h264 work with the base views of MVC / SVC bitstreams because it > doesn't report supported profiles in it's caps. There are also some problems > with the *-L0.264 files from the SVC bitstreams because there aren't any > subset sps units there: base_only is not set to TRUE and the negotiated > profile is not an SVC one. > > A workaround for both of these issues is to init base_only to TRUE and set > it to FALSE only when there is a subset sps unit in the bitstream and the > downstream element supports it's profile. If this method is preferred the > change should be trivial. Correction: avdec_h264's issue can be worked around if we set base_only to FALSE when the downstream element's caps have the "profile" field and the appropriate profile is included in it.
Review of attachment 356212 [details] [review]: ::: gst/videoparsers/gsth264parse.c @@ +225,3 @@ h264parse->align = GST_H264_PARSE_ALIGN_NONE; h264parse->format = GST_H264_PARSE_FORMAT_NONE; + h264parse->base_only = FALSE; as 0 is the default value, there's not much need to add this assignation @@ +721,3 @@ + caps = gst_caps_new_empty_simple ("video/x-h264"); + gst_caps_set_simple (caps, "profile", G_TYPE_STRING, profile_str, NULL); + return ensure_caps_profile (h264parse, caps, sps); wouldn't be simpler just call gst_caps_can_intersect() here?
(In reply to Víctor Manuel Jáquez Leal from comment #6) > Review of attachment 356212 [details] [review] [review]: > > ::: gst/videoparsers/gsth264parse.c > @@ +225,3 @@ > h264parse->align = GST_H264_PARSE_ALIGN_NONE; > h264parse->format = GST_H264_PARSE_FORMAT_NONE; > + h264parse->base_only = FALSE; > > as 0 is the default value, there's not much need to add this assignation but everything else is initialized. > > @@ +721,3 @@ > + caps = gst_caps_new_empty_simple ("video/x-h264"); > + gst_caps_set_simple (caps, "profile", G_TYPE_STRING, profile_str, NULL); > + return ensure_caps_profile (h264parse, caps, sps); > > wouldn't be simpler just call gst_caps_can_intersect() here? I call ensure_caps_profile() to also check for the compatible profiles. I can simplifiy with a single call to gst_caps_new_simple() though.
Created attachment 358002 [details] [review] h264parse: extract base view from MVC or SVC streams Should call gst_h264_parser_identify_nalu in gst_h264_parse_find_next_nal
Created attachment 358042 [details] [review] h264parse: skip prefix NALs for AU detection
h264parse should be able to convert byte-stream to avc and vice-versa. Could you please check the other possible combinations like: ... ! h264parse ! video/x-h264, stream-format=bytestream, alignment=nal ! ... ... ! h264parse ! video/x-h264, stream-format=byte-stream, alignment=au ! ... .... ! h264parse ! video/x-2h64, stream-format=avc, alighment=au ! ...
(In reply to sreerenj from comment #10) > h264parse should be able to convert byte-stream to avc and vice-versa. > Could you please check the other possible combinations like: > > ... ! h264parse ! video/x-h264, stream-format=byte-stream, alignment=nal ! ... This doesn't work (at least with vaapih264dec) but it doesn't seem to work without my patches either. > ... ! h264parse ! video/x-h264, stream-format=byte-stream, alignment=au ! ... > > ... ! h264parse ! video/x-h264, stream-format=avc, alighment=au ! ... These seem to work.
(In reply to Orestis Floros from comment #11) > (In reply to sreerenj from comment #10) > > h264parse should be able to convert byte-stream to avc and vice-versa. > > Could you please check the other possible combinations like: > > > > ... ! h264parse ! video/x-h264, stream-format=byte-stream, alignment=nal ! ... > > This doesn't work (at least with vaapih264dec) but it doesn't seem to work > without my patches either. Actually, with my patches from bug 732266 it works...
(In reply to Orestis Floros from comment #12) > (In reply to Orestis Floros from comment #11) > > (In reply to sreerenj from comment #10) > > > h264parse should be able to convert byte-stream to avc and vice-versa. > > > Could you please check the other possible combinations like: > > > > > > ... ! h264parse ! video/x-h264, stream-format=byte-stream, alignment=nal ! ... > > > > This doesn't work (at least with vaapih264dec) but it doesn't seem to work > > without my patches either. > > Actually, with my patches from bug 732266 it works... ! h264parse ! video/x-h264, stream-format=byte-stream, alignment=nal ! vaapih264dec ! vaapisink seems to be failing even for non mvc/svc use cases too. right? Please file a separate bug against gstreamer-vaapi for this. A path to fix the issue would be nice :)
Review of attachment 358002 [details] [review]: ::: gst/videoparsers/gsth264parse.c @@ +721,3 @@ + caps = gst_caps_new_simple ("video/x-h264", "profile", G_TYPE_STRING, + profile_str, NULL); +} mem leak, caps not freed @@ +764,3 @@ + return TRUE; + } + h264parse->base_only = TRUE; Why resetting base_only to "FALSE" in each iteration? @@ +964,3 @@ + } +} + I think this is going to affect the performance of generic (non-mvc/non-svc) use cases. Before(with out this code) we were only invoking the gst_h264_parser_identify_nalu_unchecked() in gst_h264_parse_collect_nal(), because we only needed to know the nex nal type and header. With your code, all most all nal units required to invoke the gst_h264_parser_identify_nalu() which involves two start_code scanning and it is unnecessary.
Created attachment 358539 [details] [review] h264parse: extract base view from MVC or SVC streams (In reply to sreerenj from comment #14) > Review of attachment 358002 [details] [review] [review]: > > ::: gst/videoparsers/gsth264parse.c > @@ +721,3 @@ > + caps = gst_caps_new_simple ("video/x-h264", "profile", G_TYPE_STRING, > + profile_str, NULL); > +} > > mem leak, caps not freed > > @@ +764,3 @@ > + return TRUE; > + } > + h264parse->base_only = TRUE; > > Why resetting base_only to "FALSE" in each iteration? Fixed. > @@ +964,3 @@ > + } > +} > + > > I think this is going to affect the performance of generic (non-mvc/non-svc) > use cases. > > Before(with out this code) we were only invoking the > gst_h264_parser_identify_nalu_unchecked() in gst_h264_parse_collect_nal(), > because we only needed to know the nex nal type and header. > > With your code, all most all nal units required to invoke the > gst_h264_parser_identify_nalu() which involves two start_code scanning and > it is unnecessary. Yes. I think with this patch the performance of generic cases shouldn't be affected and the extra cost for MVC / SVC streams is acceptable and necessary imo. Also, for the case of an MVC / SVC stream without base_only only prefix units have the extra cost and they are small.
Created attachment 358540 [details] [review] h264parse: skip prefix NALs for AU detection (In reply to Orestis Floros from comment #15) > Yes. I think with this patch the performance of generic cases shouldn't be > affected and the extra cost for MVC / SVC streams is acceptable and > necessary imo. Also, for the case of an MVC / SVC stream without base_only > only prefix units have the extra cost and they are small. I am sorry, squashed to the wrong commit.
Created attachment 358541 [details] [review] h264parse: extract base view from MVC or SVC streams ... sorry for the brain malfunction.
-- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/issues/156.