Change one relevant assert to av_assert0 while here.

Fixes: out of array read
Fixes: 15409/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_H264_fuzzer-5758846959616000

Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpegSigned-off-by: Michael Niedermayer <michael@niedermayer.cc>

Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>

Fixes: nature_360-7501616eb5eafca5-1111.mp4
Reported-by: Thierry Foucu <tfoucu@google.com>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>

Bug-Id: 1036
CC: libav-stable@libav.org

The way videotoolbox hooks in as a hwaccel is pretty hacky. The VT decode
API is not invoked until end_frame(), so alloc_frame() returns a dummy
frame with a 1-byte buffer. When end_frame() is eventually called, the
dummy buffer is replaced with the actual decoded data from
VTDecompressionSessionDecodeFrame().

When the VT decoder fails, the frame returned to the h264 decoder from
alloc_frame() remains invalid and should not be used. Before
97472199, it was accidentally being
returned all the way up to the API user. After that commit, the dummy
frame was unref'd so the user received an error.

However, since that commit, VT hwaccel failures started causing random
segfaults in the h264 decoder. This happened more often on iOS where the
VT implementation is more likely to throw errors on bitstream anomolies.
A recent report of this issue can be see in
http://ffmpeg.org/pipermail/libav-user/2016-November/009831.html

The issue here is that the dummy frame is still referenced internally by the
h264 decoder, as part of the reflist and cur_pic_ptr. Deallocating the
frame causes assertions like this one to trip later on during decoding:

  Assertion h->cur_pic_ptr->f->buf[0] failed at src/libavcodec/h264_slice.c:1340

With this commit, we leave the dummy 1-byte frame intact, but avoid returning it
to the user.

This reverts commit 97472199.
Signed-off-by: wm4 <nfxjfg@googlemail.com>

Should fix "libavcodec/h264_refs.c:372:13: warning: variable 'i' is used uninitialized whenever switch default is taken"

Found-by: durandal_17
Suggested-by: jkqxz
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>

The code conflicts with moving the h264_init_ps() call point

Without this, ff_h264_parse_ref_count() fills ref and list count and
h264_init_ps() subsequently wipes them out on a "success" path.
Subsequently things crash as the wiped fields are used.
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>

Coverity fails to realize this
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>

This should make it more clear that this function does not need any
decoder-global state other than the parameter sets.

This will prevent conflicts e.g. in code that deals with both h264 and
hevc.

Move the NAL unit types into it. This will allow to stop including the
whole decoder-specific h264dec.h in some code that is unrelated to the
decoder and only needs some enum values.

This is more consistent with the naming of other decoders.

While the value of those variables will be constant for the whole frame,
they are only used in two functions called from slice header decoding.
Moving them to the per-slice context allows us to make the H264Context
passed to slice_header_parse() constant.

Do it right before the MMCOs are applied to the DPB. This will allow
moving the frame_start() call out of the slice header parsing, since
generating the implicit MMCOs needs to be done after frame_start().

They are stored in the slice header, so technically they are per-slice
(though they must be the same in every slice). This will simplify the
following commits.

The variable stores the number of mmco entries, so the current name is
misleading.

This will allow postponing the reference list construction (and by
consequence some other functions, like frame_start) until the whole
slice header has been parsed.

There is no real reason to call it separately.

Currently it's done in the code that initialises the ref list for
MBAFF, which is not a logical place for it. Move it to the function that
parses the pred table from the bitstream, which is analogous to what is
done for the implicit weight table as well.

Found-by: ubitux
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>

Found-by: ubitux
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>

Signed-off-by: Diego Biurrun <diego@biurrun.de>

This will allow decoupling the parser from the decoder.

Make the SPS/PPS parsing independent of the H264Context, to allow
decoupling the parser from the decoder. The change is modelled after the
one done earlier for HEVC.

Move the dequant buffers to the PPS to avoid complex checks whether they
changed and an expensive copy for frame threads.

This will allow decoupling the parser from the decoder.

This fixes a regression of the sample from Ticket 2371
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>

Before 741b494f, when the reference list
modification description was invalid, the code would substitute the
corresponding reference from the initial ("default") reference list.
After that commit, it will just return an error.

Since there are apparently invalid samples in the wild that used to play
fine with the old code, it is a good idea to re-add some sort of error
resilience here. So, when the reference list modification results in a
missing frame, substitute a previous reference frame for it. The
relevant sample again decodes fine with the same output as previously.

get_ue_golomb() cannot decode values larger than 8190 (the maximum
value that can be golomb encoded in 25 bits) and produces the error
"Invalid UE golomb code" if a larger value is encountered.  Use
get_ue_golomb_long() instead (which supports 63 bits, up to 4294967294)
when valid h264/hevc values can exceed 8190.

This updates decoding of the following values:   (maximum)
  first_mb_in_slice                                36863* for level 5.2
  abs_diff_pic_num_minus1                         131071
  difference_of_pic_nums_minus1                   131071
  idr_pic_id                                       65535
  recovery_frame_cnt                               65535
  frame_packing_arrangement_id                4294967294
  frame_packing_arrangement_repetition_period      16384
  display_orientation_repetition_period            16384

An alternative would be to modify get_ue_golomb() to handle encoded
values of up to 49 bits as was done for get_se_golomb() in a92816c4.
In that case get_ue_golomb() could continue to be used for all of
these except frame_packing_arrangement_id.
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>

Fixes out of array read
Fixes mozilla bug 1233606

Found-by: Tyson Smith
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>