Re: [FFmpeg-devel] [PATCH] avcodec/jpeg2000dec: error check when processing tlm marker
On Wed, Mar 25, 2020 at 08:15:11AM +0530, Gautam Ramakrishnan wrote: > That was an extremely careless error. Apologies for that. Shall fix > that immediately. However, is changing the return type of get_tlm() > the right way? its probably a good idea to change the return type to int thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Those who are best at talking, realize last or never when they are wrong. signature.asc Description: PGP signature ___ 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".
Re: [FFmpeg-devel] [PATCH] avcodec/jpeg2000dec: error check when processing tlm marker
That was an extremely careless error. Apologies for that. Shall fix that immediately. However, is changing the return type of get_tlm() the right way? On Wed, Mar 25, 2020 at 6:06 AM Michael Niedermayer wrote: > > On Wed, Mar 25, 2020 at 01:06:15AM +0530, gautamr...@gmail.com wrote: > > From: Gautam Ramakrishnan > > > > Validate the value of ST field in the TLM marker of JPEG2000. > > Throw an error when ST takes value of 0x11. > > --- > > libavcodec/jpeg2000dec.c | 6 +- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/libavcodec/jpeg2000dec.c b/libavcodec/jpeg2000dec.c > > index 019dc81f56..74d70b686f 100644 > > --- a/libavcodec/jpeg2000dec.c > > +++ b/libavcodec/jpeg2000dec.c > > @@ -803,7 +803,11 @@ static uint8_t get_tlm(Jpeg2000DecoderContext *s, int > > n) > > > > // too complex ? ST = ((Stlm >> 4) & 0x01) + ((Stlm >> 4) & 0x02); > > ST = (Stlm >> 4) & 0x03; > > -// TODO: Manage case of ST = 0b11 --> raise error > > +if (ST == 0x11) { > > +av_log(s, AV_LOG_ERROR, "TLM marker contains invalid ST value.\n"); > > > +return AVERROR_INVALIDDATA; > > in addition to what paul said, > > the return type of get_tlm() is unsigned 8bit the error code doesnt fit in > 8bit > > > [...] > > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > The day soldiers stop bringing you their problems is the day you have stopped > leading them. They have either lost confidence that you can help or concluded > you do not care. Either case is a failure of leadership. - Colin Powell > ___ > 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". -- - 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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avcodec/jpeg2000dec: error check when processing tlm marker
On Wed, Mar 25, 2020 at 01:06:15AM +0530, gautamr...@gmail.com wrote: > From: Gautam Ramakrishnan > > Validate the value of ST field in the TLM marker of JPEG2000. > Throw an error when ST takes value of 0x11. > --- > libavcodec/jpeg2000dec.c | 6 +- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/libavcodec/jpeg2000dec.c b/libavcodec/jpeg2000dec.c > index 019dc81f56..74d70b686f 100644 > --- a/libavcodec/jpeg2000dec.c > +++ b/libavcodec/jpeg2000dec.c > @@ -803,7 +803,11 @@ static uint8_t get_tlm(Jpeg2000DecoderContext *s, int n) > > // too complex ? ST = ((Stlm >> 4) & 0x01) + ((Stlm >> 4) & 0x02); > ST = (Stlm >> 4) & 0x03; > -// TODO: Manage case of ST = 0b11 --> raise error > +if (ST == 0x11) { > +av_log(s, AV_LOG_ERROR, "TLM marker contains invalid ST value.\n"); > +return AVERROR_INVALIDDATA; in addition to what paul said, the return type of get_tlm() is unsigned 8bit the error code doesnt fit in 8bit [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The day soldiers stop bringing you their problems is the day you have stopped leading them. They have either lost confidence that you can help or concluded you do not care. Either case is a failure of leadership. - Colin Powell signature.asc Description: PGP signature ___ 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".
Re: [FFmpeg-devel] [PATCH] avcodec/jpeg2000dec: error check when processing tlm marker
On 3/24/20, gautamr...@gmail.com wrote: > From: Gautam Ramakrishnan > > Validate the value of ST field in the TLM marker of JPEG2000. > Throw an error when ST takes value of 0x11. > --- > libavcodec/jpeg2000dec.c | 6 +- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/libavcodec/jpeg2000dec.c b/libavcodec/jpeg2000dec.c > index 019dc81f56..74d70b686f 100644 > --- a/libavcodec/jpeg2000dec.c > +++ b/libavcodec/jpeg2000dec.c > @@ -803,7 +803,11 @@ static uint8_t get_tlm(Jpeg2000DecoderContext *s, int > n) > > // too complex ? ST = ((Stlm >> 4) & 0x01) + ((Stlm >> 4) & 0x02); > ST = (Stlm >> 4) & 0x03; > -// TODO: Manage case of ST = 0b11 --> raise error > +if (ST == 0x11) { 0x11 is hex representation of 17 decimal number above is &0x03 which is 3 in decimal, this ST can never be 0x11. So patch in pointless. IF you check for 3 instead does it break any existing files? > +av_log(s, AV_LOG_ERROR, "TLM marker contains invalid ST value.\n"); > +return AVERROR_INVALIDDATA; > +} > + > SP = (Stlm >> 6) & 0x01; > tile_tlm = (n - 4) / ((SP + 1) * 2 + ST); > for (i = 0; i < tile_tlm; i++) { > -- > 2.17.1 > > ___ > 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". ___ 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".