[FFmpeg-devel] [PATCH 1/2] h2645_parse: Propagate NAL header parsing errors
Derek Buitenhuis
derek.buitenhuis at gmail.com
Mon Mar 18 21:17:08 EET 2019
On 18/03/2019 18:38, James Almer wrote:
> 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.
Yes, this is correct, and arguably also not a wrong solution, just 'less good'
for broken files.
> 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).
Yes, very obviously so, but shouldn't explode the parser/decoder.
> The following change fixes this for me by preventing the decoder from
> trying to decode more than one "first" slice for the same frame:
Fixes it for me, too, and makes sense.
>> 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.
Not deadlocking is more ideal than not. I'm not particularily concerned with making
broken files look as best as possible, myself, but I know this sentiment is not
shared around here.
I'm content with it as-is, unless someone can offer a better one.
> 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.
Agree this would be a poor solution...
> Someone else that knows hevc and/or threading might want to look at this
> and fix this in a better way.
Does anyone? I'm not sure anyone still around could be considered an expert on
the HEVC decoder...
- Derel
More information about the ffmpeg-devel
mailing list