[FFmpeg-devel] [PATCH v2] avcodec/mpeg12dec: report error when picture type is unknown and err_detect is EXPLODE

[Marton Balint]

On Fri, 6 Aug 2021, Michael Niedermayer wrote:

> On Mon, Aug 02, 2021 at 08:50:05PM +0200, Marton Balint wrote:
>> Also split error message to error and warning.
>>
>> Signed-off-by: Marton Balint <cus at passwd.hu>
>> ---
>>  libavcodec/mpeg12dec.c | 14 ++++++++++----
>>  1 file changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
>> index 858dca660c..49d865853b 100644
>> --- a/libavcodec/mpeg12dec.c
>> +++ b/libavcodec/mpeg12dec.c
>> @@ -1529,7 +1529,7 @@ static void mpeg_decode_quant_matrix_extension(MpegEncContext *s)
>>          load_matrix(s, s->chroma_inter_matrix, NULL, 0);
>>  }
>>
>> -static void mpeg_decode_picture_coding_extension(Mpeg1Context *s1)
>> +static int mpeg_decode_picture_coding_extension(Mpeg1Context *s1)
>>  {
>>      MpegEncContext *s = &s1->mpeg_enc_ctx;
>>
>> @@ -1539,8 +1539,10 @@ static void mpeg_decode_picture_coding_extension(Mpeg1Context *s1)
>>      s->mpeg_f_code[1][0] = get_bits(&s->gb, 4);
>>      s->mpeg_f_code[1][1] = get_bits(&s->gb, 4);
>>      if (!s->pict_type && s1->mpeg_enc_ctx_allocated) {
>> -        av_log(s->avctx, AV_LOG_ERROR,
>> -               "Missing picture start code, guessing missing values\n");
>> +        av_log(s->avctx, AV_LOG_ERROR, "Missing picture start code\n");
>> +        if (s->avctx->err_recognition & AV_EF_EXPLODE)
>> +            return AVERROR_INVALIDDATA;
>> +        av_log(s->avctx, AV_LOG_WARNING, "Guessing pict_type from mpeg_f_code\n");
>
> If we are nitpicking then this is not ideal
> "Missing picture start code" is only an error when AV_EF_EXPLODE is set
> because it only in that case results in wrong output, when AV_EF_EXPLODE
> is not set the output from the decoder might be correct si it too should
> be a warning in that case

I understand your logic, but there are arguments for keeping the error 
message as well. I think in practice, most times, this error is caused by 
bitstream errors and it _will_ affect decoding. Also there are tons of 
cases in the code which log an error, then return INVALIDDATA if EXPLODE 
is used, so why handle it here differently...

But let me know if you insisit and I will change it. And yes, it is indeed 
nitpicking :)

Regards,
Marton