On Sat, Aug 07, 2021 at 08:32:35PM +0200, Marton Balint wrote:
>
>
> On Sat, 7 Aug 2021, Michael Niedermayer wrote:
>
> > 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
>
> Obviously it is hard to be sure. Among the samples we have, I found a few
> files affected:
>
> ffmpeg-bugs/trac/ticket2705/81391.mpg
> ffmpeg-bugs/trac/ticket2950/mpeg2_fuzz.mpg
> ffmpeg-bugs/roundup/issue1872/lol-mpeg2.mp4
> ffmpeg-bugs/roundup/issue1218/dvbt-ffmpeg-seg-fault.ts
>
> All are heavily corrupted files.
>
> I also had a private sample file having this issue, and no other error but
> the "ignoring extra picture following a frame-picture" warning, and it is
> also a corrupted bitstream, and causes missed frames when decoding.
> > So a patologcal bitstream can probably be created when the decoding is not > affected, but my guess is that for real-world files decoding is always > affected, therefore I think an error is more appropriate. i thought i saw one where it was just missing and no corruption but maybe not. Its certainly not worth the time spent on this to search further > > Let me know what you prefer. please use what you prefer thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB it is not once nor twice but times without number that the same ideas make their appearance in the world. -- Aristotle
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".
