diff mbox series

[FFmpeg-devel,v4,2/2] libavcodec/jpeg2000dec.c: Handle non EOC streams

Message ID 20200328122810.25084-2-gautamramk@gmail.com
State Superseded
Headers show
Series [FFmpeg-devel,v4,1/2] libavcodec/jpeg2000dec.c: Add functions and modify structs for PPT marker support
Related show

Checks

Context Check Description
andriy/ffmpeg-patchwork pending
andriy/ffmpeg-patchwork success Applied patch
andriy/ffmpeg-patchwork success Configure finished
andriy/ffmpeg-patchwork success Make finished
andriy/ffmpeg-patchwork success Make fate finished

Commit Message

Gautam Ramakrishnan March 28, 2020, 12:28 p.m. UTC
From: Gautam Ramakrishnan <gautamramk@gmail.com>

This patch allows decoding of j2k streams which do
not have an EOC marker. OpenJPEG implements a similar
check.
---
 libavcodec/jpeg2000dec.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Comments

Carl Eugen Hoyos March 28, 2020, 12:52 p.m. UTC | #1
Am Sa., 28. März 2020 um 13:28 Uhr schrieb <gautamramk@gmail.com>:
>
> From: Gautam Ramakrishnan <gautamramk@gmail.com>
>
> This patch allows decoding of j2k streams which do
> not have an EOC marker. OpenJPEG implements a similar
> check.
> ---
>  libavcodec/jpeg2000dec.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/libavcodec/jpeg2000dec.c b/libavcodec/jpeg2000dec.c
> index 44b3e7e41b..eb877d499d 100644
> --- a/libavcodec/jpeg2000dec.c
> +++ b/libavcodec/jpeg2000dec.c
> @@ -2076,8 +2076,11 @@ static int jpeg2000_read_main_headers(Jpeg2000DecoderContext *s)
>
>          len = bytestream2_get_be16(&s->g);
>          if (len < 2 || bytestream2_get_bytes_left(&s->g) < len - 2) {
> -            av_log(s->avctx, AV_LOG_ERROR, "Invalid len %d left=%d\n", len, bytestream2_get_bytes_left(&s->g));
> -            return AVERROR_INVALIDDATA;
> +            if (s->avctx->err_recognition & AV_EF_EXPLODE) {

lol
Sorry, my mistake, please check for the following:
avctx->strict_std_compliance >= FF_COMPLIANCE_STRICT

> +                av_log(s->avctx, AV_LOG_ERROR, "Invalid len %d left=%d\n", len, bytestream2_get_bytes_left(&s->g));
> +                return AVERROR_INVALIDDATA;

A message should always be shown (after all, the stream is clearly
invalid), some
people argue that it should be WARNING for < STRICT and ERROR for >= STRICT.

> +            }
> +            continue;
>          }
>
>          switch (marker) {

I don't know if the continue is more correct.

Carl Eugen
Gautam Ramakrishnan March 28, 2020, 12:58 p.m. UTC | #2
On Sat, Mar 28, 2020 at 6:22 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
>
> Am Sa., 28. März 2020 um 13:28 Uhr schrieb <gautamramk@gmail.com>:
> >
> > From: Gautam Ramakrishnan <gautamramk@gmail.com>
> >
> > This patch allows decoding of j2k streams which do
> > not have an EOC marker. OpenJPEG implements a similar
> > check.
> > ---
> >  libavcodec/jpeg2000dec.c | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/libavcodec/jpeg2000dec.c b/libavcodec/jpeg2000dec.c
> > index 44b3e7e41b..eb877d499d 100644
> > --- a/libavcodec/jpeg2000dec.c
> > +++ b/libavcodec/jpeg2000dec.c
> > @@ -2076,8 +2076,11 @@ static int jpeg2000_read_main_headers(Jpeg2000DecoderContext *s)
> >
> >          len = bytestream2_get_be16(&s->g);
> >          if (len < 2 || bytestream2_get_bytes_left(&s->g) < len - 2) {
> > -            av_log(s->avctx, AV_LOG_ERROR, "Invalid len %d left=%d\n", len, bytestream2_get_bytes_left(&s->g));
> > -            return AVERROR_INVALIDDATA;
> > +            if (s->avctx->err_recognition & AV_EF_EXPLODE) {
>
> lol
> Sorry, my mistake, please check for the following:
> avctx->strict_std_compliance >= FF_COMPLIANCE_STRICT
>
oops, should not have copied it blindly!!
> > +                av_log(s->avctx, AV_LOG_ERROR, "Invalid len %d left=%d\n", len, bytestream2_get_bytes_left(&s->g));
> > +                return AVERROR_INVALIDDATA;
>
> A message should always be shown (after all, the stream is clearly
> invalid), some
> people argue that it should be WARNING for < STRICT and ERROR for >= STRICT.
>
> > +            }
> > +            continue;
> >          }
> >
> >          switch (marker) {
>
> I don't know if the continue is more correct.
The reasoning behind the "continue" is that it goes to the next
iteration and throws the
"Missing EOC" message. As the message is already there, I thought it
would be better
to reuse it rather than adding a new log message.
>
> Carl Eugen
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff mbox series

Patch

diff --git a/libavcodec/jpeg2000dec.c b/libavcodec/jpeg2000dec.c
index 44b3e7e41b..eb877d499d 100644
--- a/libavcodec/jpeg2000dec.c
+++ b/libavcodec/jpeg2000dec.c
@@ -2076,8 +2076,11 @@  static int jpeg2000_read_main_headers(Jpeg2000DecoderContext *s)
 
         len = bytestream2_get_be16(&s->g);
         if (len < 2 || bytestream2_get_bytes_left(&s->g) < len - 2) {
-            av_log(s->avctx, AV_LOG_ERROR, "Invalid len %d left=%d\n", len, bytestream2_get_bytes_left(&s->g));
-            return AVERROR_INVALIDDATA;
+            if (s->avctx->err_recognition & AV_EF_EXPLODE) {
+                av_log(s->avctx, AV_LOG_ERROR, "Invalid len %d left=%d\n", len, bytestream2_get_bytes_left(&s->g));
+                return AVERROR_INVALIDDATA;
+            }
+            continue;
         }
 
         switch (marker) {