Re: [FFmpeg-devel] [PATCH] avcodec/jpeg2000dec: error check when processing tlm marker

2020-03-25 Thread Michael Niedermayer
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

2020-03-24 Thread Gautam Ramakrishnan
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

2020-03-24 Thread Michael Niedermayer
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

2020-03-24 Thread Paul B Mahol
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".