On Sat, Aug 07, 2021 at 09:21:36AM +0200, Marton Balint wrote:
>
>
> 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 <c...@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... do these cases really represent situations where the output might be correct ? for examlple any out of range coefficient cant be correct and a mismatching CRC well it "could" be. I think a big part would be, "do we actually have cases where it is not an error ?" if so i would argue that it should be a warning > > But let me know if you insisit and I will change it. And yes, it is indeed > nitpicking :) i dont insist but thats how the levels are defined /** * Something went wrong and cannot losslessly be recovered. * However, not all future data is affected. */ #define AV_LOG_ERROR 16 /** * Something somehow does not look correct. This may or may not * lead to problems. An example would be the use of '-vstrict -2'. */ #define AV_LOG_WARNING 24 [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB "I am not trying to be anyone's saviour, I'm trying to think about the future and not be sad" - Elon Musk
signature.asc
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
