| 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 | expand |
| Context | Check | Description |
|---|---|---|
| andriy/ffmpeg-patchwork | success | Make fate finished |
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.
> 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
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".
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
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 |
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
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 --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) {
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(-)