diff mbox series

[FFmpeg-devel,v3,3/3] libavcodec/jpeb2000dec.c: Handle non EOC streams

Message ID 20200328041919.3024-3-gautamramk@gmail.com
State New
Headers show
Series [FFmpeg-devel,v3,1/3] 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, 4:19 a.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 | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Gautam Ramakrishnan March 28, 2020, 4:24 a.m. UTC | #1
On Sat, Mar 28, 2020 at 9:49 AM <gautamramk@gmail.com> wrote:
>
> 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 | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/libavcodec/jpeg2000dec.c b/libavcodec/jpeg2000dec.c
> index 44b3e7e41b..99fcb2cf68 100644
> --- a/libavcodec/jpeg2000dec.c
> +++ b/libavcodec/jpeg2000dec.c
> @@ -2076,8 +2076,9 @@ 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;
> +            av_log(s->avctx, AV_LOG_WARNING, "Invalid len %d left=%d\n", len, bytestream2_get_bytes_left(&s->g));
> +            av_log(s->avctx, AV_LOG_WARNING, "Stream does not end with EOC.\n");
> +            return 0;
>          }
>
>          switch (marker) {
> --
> 2.17.1
>
Hi,

@Carl, on adding this patch series, looks like the frames of
http://samples.ffmpeg.org/jpeg2000/jpeg2000_mxf_first_10mb.mxf
also seems to be decoded correctly. Have sent this patch for your
comments.
Steven Liu March 28, 2020, 4:28 a.m. UTC | #2
> 2020年3月28日 下午12:24,Gautam Ramakrishnan <gautamramk@gmail.com> 写道:
> 
> On Sat, Mar 28, 2020 at 9:49 AM <gautamramk@gmail.com> wrote:
should the title be jpeg2000dec?, it maybe is a typo if I don’t misunderstand.
>> 
>> 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 | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>> 
>> diff --git a/libavcodec/jpeg2000dec.c b/libavcodec/jpeg2000dec.c
>> index 44b3e7e41b..99fcb2cf68 100644
>> --- a/libavcodec/jpeg2000dec.c
>> +++ b/libavcodec/jpeg2000dec.c
>> @@ -2076,8 +2076,9 @@ 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;
>> +            av_log(s->avctx, AV_LOG_WARNING, "Invalid len %d left=%d\n", len, bytestream2_get_bytes_left(&s->g));
>> +            av_log(s->avctx, AV_LOG_WARNING, "Stream does not end with EOC.\n");
>> +            return 0;
>>         }
>> 
>>         switch (marker) {
>> --
>> 2.17.1
>> 
> Hi,
> 
> @Carl, on adding this patch series, looks like the frames of
> http://samples.ffmpeg.org/jpeg2000/jpeg2000_mxf_first_10mb.mxf
> also seems to be decoded correctly. Have sent this patch for your
> comments.
> 
> -- 
> -------------
> Gautam |
> _______________________________________________
> 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".

Thanks

Steven Liu
Gautam Ramakrishnan March 28, 2020, 4:50 a.m. UTC | #3
On Sat, Mar 28, 2020 at 9:59 AM Steven Liu <lq@chinaffmpeg.org> wrote:
>
>
>
> > 2020年3月28日 下午12:24,Gautam Ramakrishnan <gautamramk@gmail.com> 写道:
> >
> > On Sat, Mar 28, 2020 at 9:49 AM <gautamramk@gmail.com> wrote:
> should the title be jpeg2000dec?, it maybe is a typo if I don’t misunderstand.
You are right, it is a typo. It must be jpeg2000dec.
> >>
> >> 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 | 5 +++--
> >> 1 file changed, 3 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/libavcodec/jpeg2000dec.c b/libavcodec/jpeg2000dec.c
> >> index 44b3e7e41b..99fcb2cf68 100644
> >> --- a/libavcodec/jpeg2000dec.c
> >> +++ b/libavcodec/jpeg2000dec.c
> >> @@ -2076,8 +2076,9 @@ 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;
> >> +            av_log(s->avctx, AV_LOG_WARNING, "Invalid len %d left=%d\n", len, bytestream2_get_bytes_left(&s->g));
> >> +            av_log(s->avctx, AV_LOG_WARNING, "Stream does not end with EOC.\n");
> >> +            return 0;
> >>         }
> >>
> >>         switch (marker) {
> >> --
> >> 2.17.1
> >>
> > Hi,
> >
> > @Carl, on adding this patch series, looks like the frames of
> > http://samples.ffmpeg.org/jpeg2000/jpeg2000_mxf_first_10mb.mxf
> > also seems to be decoded correctly. Have sent this patch for your
> > comments.
> >
> > --
> > -------------
> > Gautam |
> > _______________________________________________
> > 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".
>
> Thanks
>
> Steven Liu
>
>
>
> _______________________________________________
> 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".
Carl Eugen Hoyos March 28, 2020, 10:23 a.m. UTC | #4
Am Sa., 28. März 2020 um 05:19 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 | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/libavcodec/jpeg2000dec.c b/libavcodec/jpeg2000dec.c
> index 44b3e7e41b..99fcb2cf68 100644
> --- a/libavcodec/jpeg2000dec.c
> +++ b/libavcodec/jpeg2000dec.c
> @@ -2076,8 +2076,9 @@ 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;
> +            av_log(s->avctx, AV_LOG_WARNING, "Invalid len %d left=%d\n", len, bytestream2_get_bytes_left(&s->g));
> +            av_log(s->avctx, AV_LOG_WARNING, "Stream does not end with EOC.\n");
> +            return 0;

The return value should depend on the "strictness" requested by the user,
grep for "EXPLODE".

As said, the first two patches are (imo) not ok, either merge them or split
them differently so that the first does not add an unused function but
only splits ("uselessly" but making the second patch easier to read) an
existing function.

Carl Eugen
Gautam Ramakrishnan March 28, 2020, 11:13 a.m. UTC | #5
On Sat, Mar 28, 2020 at 3:54 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
>
> Am Sa., 28. März 2020 um 05:19 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 | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/libavcodec/jpeg2000dec.c b/libavcodec/jpeg2000dec.c
> > index 44b3e7e41b..99fcb2cf68 100644
> > --- a/libavcodec/jpeg2000dec.c
> > +++ b/libavcodec/jpeg2000dec.c
> > @@ -2076,8 +2076,9 @@ 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;
> > +            av_log(s->avctx, AV_LOG_WARNING, "Invalid len %d left=%d\n", len, bytestream2_get_bytes_left(&s->g));
> > +            av_log(s->avctx, AV_LOG_WARNING, "Stream does not end with EOC.\n");
> > +            return 0;
>
> The return value should depend on the "strictness" requested by the user,
> grep for "EXPLODE".
>
I did not understand this point too well. Could you point out some example I
could refer to?
> As said, the first two patches are (imo) not ok, either merge them or split
> them differently so that the first does not add an unused function but
> only splits ("uselessly" but making the second patch easier to read) an
> existing function.
>
Shall do that. What is your opinion on the third patch? Do you feel
the fix is right?

> 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".



--
-------------
Gautam |
Carl Eugen Hoyos March 28, 2020, 11:28 a.m. UTC | #6
Am Sa., 28. März 2020 um 12:13 Uhr schrieb Gautam Ramakrishnan
<gautamramk@gmail.com>:
>
> On Sat, Mar 28, 2020 at 3:54 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
> >
> > Am Sa., 28. März 2020 um 05:19 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 | 5 +++--
> > >  1 file changed, 3 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/libavcodec/jpeg2000dec.c b/libavcodec/jpeg2000dec.c
> > > index 44b3e7e41b..99fcb2cf68 100644
> > > --- a/libavcodec/jpeg2000dec.c
> > > +++ b/libavcodec/jpeg2000dec.c
> > > @@ -2076,8 +2076,9 @@ 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;
> > > +            av_log(s->avctx, AV_LOG_WARNING, "Invalid len %d left=%d\n", len, bytestream2_get_bytes_left(&s->g));
> > > +            av_log(s->avctx, AV_LOG_WARNING, "Stream does not end with EOC.\n");
> > > +            return 0;
> >
> > The return value should depend on the "strictness" requested by the user,
> > grep for "EXPLODE".
> >
> I did not understand this point too well. Could you point out some example I
> could refer to?

Did you grep for "EXPLODE"?

> > As said, the first two patches are (imo) not ok, either merge them or split
> > them differently so that the first does not add an unused function but
> > only splits ("uselessly" but making the second patch easier to read) an
> > existing function.
> >
> Shall do that. What is your opinion on the third patch? Do you feel
> the fix is right?

Yes, I believe the same is done for jpeg.

Carl Eugen
Gautam Ramakrishnan March 28, 2020, 11:35 a.m. UTC | #7
On Sat, Mar 28, 2020 at 4:59 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
>
> Am Sa., 28. März 2020 um 12:13 Uhr schrieb Gautam Ramakrishnan
> <gautamramk@gmail.com>:
> >
> > On Sat, Mar 28, 2020 at 3:54 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
> > >
> > > Am Sa., 28. März 2020 um 05:19 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 | 5 +++--
> > > >  1 file changed, 3 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/libavcodec/jpeg2000dec.c b/libavcodec/jpeg2000dec.c
> > > > index 44b3e7e41b..99fcb2cf68 100644
> > > > --- a/libavcodec/jpeg2000dec.c
> > > > +++ b/libavcodec/jpeg2000dec.c
> > > > @@ -2076,8 +2076,9 @@ 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;
> > > > +            av_log(s->avctx, AV_LOG_WARNING, "Invalid len %d left=%d\n", len, bytestream2_get_bytes_left(&s->g));
> > > > +            av_log(s->avctx, AV_LOG_WARNING, "Stream does not end with EOC.\n");
> > > > +            return 0;
> > >
> > > The return value should depend on the "strictness" requested by the user,
> > > grep for "EXPLODE".
> > >
> > I did not understand this point too well. Could you point out some example I
> > could refer to?
>
> Did you grep for "EXPLODE"?
>
Ran grep with -rnw accidently. Found it. Shall get back if I do not understand
> > > As said, the first two patches are (imo) not ok, either merge them or split
> > > them differently so that the first does not add an unused function but
> > > only splits ("uselessly" but making the second patch easier to read) an
> > > existing function.
> > >
> > Shall do that. What is your opinion on the third patch? Do you feel
> > the fix is right?
>
> Yes, I believe the same is done for jpeg.
In that case I'll make all the changes and resubmit a v4
>
> 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..99fcb2cf68 100644
--- a/libavcodec/jpeg2000dec.c
+++ b/libavcodec/jpeg2000dec.c
@@ -2076,8 +2076,9 @@  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;
+            av_log(s->avctx, AV_LOG_WARNING, "Invalid len %d left=%d\n", len, bytestream2_get_bytes_left(&s->g));
+            av_log(s->avctx, AV_LOG_WARNING, "Stream does not end with EOC.\n");
+            return 0;
         }
 
         switch (marker) {