[FFmpeg-devel] [PATCH 1/5] avcodec/hevc_parse: check for parameter set decoding failure

[James Almer]

On 4/3/2017 7:00 AM, Michael Niedermayer wrote:
> On Sun, Apr 02, 2017 at 10:45:41PM -0300, James Almer wrote:
>> Signed-off-by: James Almer <jamrial at gmail.com>
>> ---
>>  libavcodec/hevc_parse.c | 32 +++++++++++++++++++++++++-------
>>  1 file changed, 25 insertions(+), 7 deletions(-)
>>
>> diff --git a/libavcodec/hevc_parse.c b/libavcodec/hevc_parse.c
>> index 6c1138e015..028ca5afe5 100644
>> --- a/libavcodec/hevc_parse.c
>> +++ b/libavcodec/hevc_parse.c
>> @@ -22,7 +22,8 @@
>>  #include "hevc_parse.h"
>>  
>>  static int hevc_decode_nal_units(const uint8_t *buf, int buf_size, HEVCParamSets *ps,
>> -                                 int is_nalff, int nal_length_size, void *logctx)
>> +                                 int is_nalff, int nal_length_size, int err_recognition,
>> +                                 void *logctx)
>>  {
>>      int i;
>>      int ret = 0;
>> @@ -38,9 +39,21 @@ static int hevc_decode_nal_units(const uint8_t *buf, int buf_size, HEVCParamSets
>>  
>>          /* ignore everything except parameter sets and VCL NALUs */
>>          switch (nal->type) {
>> -        case HEVC_NAL_VPS: ff_hevc_decode_nal_vps(&nal->gb, logctx, ps);    break;
>> -        case HEVC_NAL_SPS: ff_hevc_decode_nal_sps(&nal->gb, logctx, ps, 1); break;
>> -        case HEVC_NAL_PPS: ff_hevc_decode_nal_pps(&nal->gb, logctx, ps);    break;
>> +        case HEVC_NAL_VPS:
>> +            ret = ff_hevc_decode_nal_vps(&nal->gb, logctx, ps);
>> +            if (ret < 0)
>> +                goto done;
>> +            break;
>> +        case HEVC_NAL_SPS:
>> +            ret = ff_hevc_decode_nal_sps(&nal->gb, logctx, ps, 1);
>> +            if (ret < 0)
>> +                goto done;
>> +            break;
>> +        case HEVC_NAL_PPS:
>> +            ret = ff_hevc_decode_nal_pps(&nal->gb, logctx, ps);
>> +            if (ret < 0)
>> +                goto done;
>> +            break;
>>          case HEVC_NAL_TRAIL_R:
>>          case HEVC_NAL_TRAIL_N:
>>          case HEVC_NAL_TSA_N:
> 
> I didnt investigate how exactly this is used but from just the patch
> this seems not optimal
> For example, if you have 3 PPS NALs and the first fails to decode
> you might still be able to fully decode the other 2

I'm mimicking the behavior of decode_nal_unit() in hevcdec.c, which is
currently used during frame decoding and extradata decoding.
This patchset aims at removing duplicate code while keeping the hevc
decoder behaving the same as it was before. I could skip this change
if that's preferred, but if this behavior is not optimal then someone
who better understands the codec should look at it.