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.
Let me know what you prefer.
Thanks,
Marton
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
_______________________________________________
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".
