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

Michael Niedermayer michael at niedermayer.cc
Sun Aug 8 15:51:13 EEST 2021


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 <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...
> > 
> > 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20210808/a50a95ca/attachment.sig>


More information about the ffmpeg-devel mailing list