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