GNOME Bugzilla – Bug 762352
decoder: hevc: Fill dependent slice segment headers while parsing
Last modified: 2016-02-23 09:04:33 UTC
In GstVaapiDecoderH265, only a single independent slice segment header is passing from parsing into decoding (via `prev_independent_slice_pi`) for populating dependent slice segment headers. If the picture has multiple slices with dependent slice segments then the dependent segments from all slices will get populated with the fields from the last independent slice segment header instead of its corresponding independent slice segment header.
Created attachment 321703 [details] [review] [PATCH] decoder: hevc: Fill dependent slice segment headers while parsing
(In reply to Scott D Phillips from comment #0) If the picture has multiple > slices with dependent slice segments then the dependent segments from all > slices will get populated with the fields from the last independent slice > segment header instead of its corresponding independent slice segment header. Is it written like that in the spec? I am not sure whether the commit message here is correct, Missing syntax elements of the dependent slice segment headers should be always inferred from the previous independent slice segment. In that way the current code is right IMO... Also a "slice" includes CTUs of independent slice segments and all subsequent dependent slice segments that precedes the next independent slice segment. But there is a different issue which should be fixing by your patch: We are not supposed to use the previous independent slice header for populating all the dependent header fields. For eg; in a tiled video , entry point offsets might be present in dependent slice segment which shouldn't be override with the previous independent slice header values. Do you think I am missing something ? :) BTW, Could you please share a sample video ??
(In reply to sreerenj from comment #2) > (In reply to Scott D Phillips from comment #0) > If the picture has multiple > > slices with dependent slice segments then the dependent > > segments from all slices will get populated with the fields > > from the last independent slice segment header instead of its > > corresponding independent slice segment header. > > Is it written like that in the spec? > I am not sure whether the commit message here is correct, Maybe I worded that poorly. The spec says: H.265 7.4.7.1: > dependent_slice_segment_flag equal to 1 specifies that the > value of each slice segment header syntax element that is not > present is inferred to be equal to the value of the > corresponding slice segment header syntax element in the slice > header. So this bug and patch all deal with that "corresponding slice segment header" part. H.265 3.4.6 says "the values of some syntax elements of the slice segment header are inferred from the values for the preceding independent slice segment in decoding order." So what I mean to say in the commit message is that before my patch we're using the wrong slice segment header as the "corresponding" one. Before my patch we always use the AU's *last* independent slice segment header instead of correctly using the preceding independent header. > Missing syntax elements of the dependent slice segment headers > should be always inferred from the previous independent slice > segment. In that way the current code is right IMO... The current code has a parsing phase followed by a decoding phase. We keep the 'previous independent' reference up to date through the parsing phase but it is actually dereferenced during the decoding phase. So we're not pulling the values from the "corresponding" header as we go, but rather we currently pull the values from the *last* header after all parsing is done. In that way we wind up populating all dependent slice segment headers with the values from the last independent slice segment header in decoding order. > BTW, Could you please share a sample video ?? I'm testing against a licensed compliance test stream that I won't be able to share publicly. I can mail it to you of course.
Great ! I got confused with the commit message "In the case where the picture contains multiple slices with dependent segments the dependent segments of all slices would be filled with the syntax elements of the last independent slice segment in decoding order instead of the preceding independent slice segment in decoding order." Would be great if you can rephrase the commit message... Otherwise LGTM...
Created attachment 321899 [details] [review] decoder: hevc: Fill dependent slice segment headers while parsing Reworded commit message, hopefully it's clearer now.
commit ffd5028a38da45ed7e67c377590078534aca3078 Author: Scott D Phillips <scott.d.phillips@intel.com> Date: Tue Feb 23 10:55:02 2016 +0200 decoder: hevc: Fill dependent slice segment headers while parsing Copy the data into the dependent slice segment header from the corresponding independent slice segment header during parsing. Previously the reference to the "previous" independent header was held through the parsing phase and then dereferenced during the decoding phase. This caused all dependent headers to be populated with the data of the AU's last independent header instead of the proper corresponding header. https://bugzilla.gnome.org/show_bug.cgi?id=762352 Changes since v1: - Reworded commit message