[FFmpeg-devel] [PATCH 1/2] h2645_parse: Propagate NAL header parsing errors

[James Almer]

On 3/18/2019 12:52 PM, Derek Buitenhuis wrote:
> On 18/03/2019 15:50, James Almer wrote:
>> This will abort the splitting process when it's meant to only discard
>> the faulty NAL. Even the log message stats it's skipping it.
> 
> The log message also claims to be AV_LOG_ERROR.
> 
> Also there are no comments indicating why this is right, or any commit I
> could find.
> 
> If you feel you can fix this in hevcdec.c, feel free... I couldn't figure
> out where it was blowing up, specifically.
> 
> - Derek

So, what i'm seeing here is two slice NALs in the same packet (which
means processed in the same decode_nal_units() loop in hevcdec.c)
reporting being the "first slice segment in the pic". And that's
seemingly making the threading logic shit itself.
In between them are two NALs, both with valid starting codes but invalid
data (first bit is 1 when it's a bitstream requirement for it to be 0),
but they ultimately have nothing to do with the issue in this file. Your
patch works around this simply because it aborts the NAL splitting
before it gets to the second slice NAL, and the whole packet gets discarded.

This is among other things a muxing mistake, since if you remux this
into raw hevc (ffmpeg -i nal_header_deadlock.mp4 -c:v copy -an
nal_header_deadlock.hevc) and try to decode that, the hevc parser will
split it into proper packets with one slice/pic NAL each and the
deadlock will not happen (see hevc_find_frame_end() in hevc_parser.c).

The following change fixes this for me by preventing the decoder from
trying to decode more than one "first" slice for the same frame:

> diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c
> index 967f8f1def..9492108c77 100644
> --- a/libavcodec/hevcdec.c
> +++ b/libavcodec/hevcdec.c
> @@ -2927,6 +2927,10 @@ static int decode_nal_unit(HEVCContext *s, const H2645NAL *nal)
>          }
> 
>          if (s->sh.first_slice_in_pic_flag) {
> +            if (s->ref) {
> +                av_log(s->avctx, AV_LOG_ERROR, "Two slices reporting being the first in the picture.\n");
> +                goto fail;
> +            }
>              if (s->max_ra == INT_MAX) {
>                  if (s->nal_unit_type == HEVC_NAL_CRA_NUT || IS_BLA(s)) {
>                      s->max_ra = s->poc;

But it will however discard the second slice, despite its only apparent
problem being showing up in the wrong packet, so it's probably still not
ideal.
Another solution would be to enable the parser for mp4 input, so the
packetization in the input will be ignored, but that'll slow down
demuxing for every single file.

Someone else that knows hevc and/or threading might want to look at this
and fix this in a better way.