[FFmpeg-devel] [PATCH] Support HDR10+ metadata for HEVC.

[Hendrik Leppkes]

On Sat, Dec 5, 2020 at 11:28 PM James Almer <jamrial at gmail.com> wrote:
>
> On 12/1/2020 3:09 PM, Mohammad Izadi wrote:
> > Ian, can you please take a look into it? And if it's fine to push it.
> >
> > Thanks,
> > Mohammad
>
> I removed the packet side data addition since it was not used by any
> module. It can be added when a demuxer needs to propagate it.
>
> Also removed the entry for dynamic_hdr10_plus.h in the installed
> libraries list as it's internal, simplified the test (cut the sample
> into a single frame, 70kb vs 1mb sample) and split the patch in four
> commits before pushing.
>

I looked through the committed patch as I was curious how it compares
to the code i've been using for a bit, and I noticed some concerns
with the GetBitContext handling.

- All SEI parsing uses one shared "gb", not only for one SEI message,
but the entire SEI NALU (this is fine, but important to know for the
other points)
- ff_parse_itu_t_t35_to_dynamic_hdr10_plus will always consume the
*full* GetBitContext with a "skip_bits(gb, get_bits_left(gb));" at its
end, so it may skip over a SEI message following the HDR10+ data?
- The skip in decode_nal_sei_user_data_registered_itu_t_t35 was moved
out of any conditions, so in case of decoding HDR10+ data, it would
skip data despite it already being consumed by parsing?

It seems like the intention of the code was to pass a copy of the
GetBitContex (and not the original) to
ff_parse_itu_t_t35_to_dynamic_hdr10_plus, as it would avoid any of
these issues, but then I don't know what the skip at the end of the
hdr10+ function is for?

- Hendrik