Re: [FFmpeg-devel] h264: fix RTSP stream decoding

2018-01-03 Thread Michael Niedermayer
On Wed, Jan 03, 2018 at 12:54:03PM +0300, ser...@gavrushkin.com wrote:
> > The error code returned by decode_extradata_ps() is inconsistent after this
> > its not "if any failed" it is returning an error if the last failed 
> 
> Sorry, I don't get how it is supposed to work. I just found the previous 
> implementation and checked which commit broke it.

if a loop running multiple iterations each decoding a "packet"
then a failure of packet n should be treated the same as a failure
of packet n+1
The code after the patch would return failure if the last packet fails
but not if a earlier packet fails

Note this is seperate from continuing to decode all packets.
the code can continue after an error but still declare an error at the end
or it could never decalare an error.
Either way it should not treat the last iteration special


[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Good people do not need laws to tell them to act responsibly, while bad
people will find a way around the laws. -- Plato


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] h264: fix RTSP stream decoding

2018-01-03 Thread sergey
> The error code returned by decode_extradata_ps() is inconsistent after this
> its not "if any failed" it is returning an error if the last failed 

Sorry, I don't get how it is supposed to work. I just found the previous 
implementation and checked which commit broke it.

The other possible solution on upper level: 

---

From 9fcd003a095b19b9e2fb5f6af3cc57a9e131f308 Mon Sep 17 00:00:00 2001
From: Sergey Gavrushkin 
Date: Wed, 3 Jan 2018 12:51:15 +0300
Subject: [PATCH] libavcodec/h264: fix decoding

Fixes ticket #6422. It is a regression fix for an issue that was introduced in 
commit
98c97994c5b90bdae02accb155eeceeb5224b8ef. Variable err_recognition is
ignored while extradata is decoded and the whole decoding process is
failed due to timeout.

Signed-off-by: Sergey Gavrushkin 
---
 libavcodec/h264_parse.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/h264_parse.c b/libavcodec/h264_parse.c
index fee28d9..403fd39 100644
--- a/libavcodec/h264_parse.c
+++ b/libavcodec/h264_parse.c
@@ -487,7 +487,7 @@ int ff_h264_decode_extradata(const uint8_t *data, int size, 
H264ParamSets *ps,
 } else {
 *is_avc = 0;
 ret = decode_extradata_ps(data, size, ps, 0, logctx);
-if (ret < 0)
+if (ret < 0 && (err_recognition & AV_EF_EXPLODE))
 return ret;
 }
 return size;
--
2.6.4


Thank you, 
Sergey
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] h264: fix RTSP stream decoding

2017-12-30 Thread Michael Niedermayer
On Sat, Dec 30, 2017 at 12:48:19AM +0300, ser...@gavrushkin.com wrote:
> > Please add "Fixes ticket #6422" to the commit message.
> > 
> > And maybe remove "rtsp" from the commit title, the issue
> > is reproducible with files.
> 
> Done.
> 
> Please feel free to edit commit title/message as you wish for merge.
> 
> Thank you, 
> Sergey
> 
> -
> 
> 
> 
> From e90ef7b56d4147ff6555468f0154321b55596846 Mon Sep 17 00:00:00 2001
> From: Sergey Gavrushkin >
> Date: Fri, 29 Dec 2017 20:03:50 +0300
> Subject: [PATCH] h264: fix decoding
> 
> Fixes ticket #6422 . It is a regression fix for an issue that was introduced 
> in commit
> 98c97994c5b90bdae02accb155eeceeb5224b8ef. Variable err_recognition is
> ignored while extradata is decoded and the whole decoding process is
> failed due to timeout.
> ---
> libavcodec/h264_parse.c | 12 ++--
> 1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/libavcodec/h264_parse.c b/libavcodec/h264_parse.c
> index fee28d9..009d50c 100644
> --- a/libavcodec/h264_parse.c
> +++ b/libavcodec/h264_parse.c
> @@ -347,7 +347,7 @@ int ff_h264_init_poc(int pic_field_poc[2], int *pic_poc,
> }
> 
> static int decode_extradata_ps(const uint8_t *data, int size, H264ParamSets 
> *ps,
> -   int is_avc, void *logctx)
> +   int is_avc, int err_recognition, void *logctx)
> {
> H2645Packet pkt = { 0 };
> int i, ret = 0;
> @@ -363,13 +363,13 @@ static int decode_extradata_ps(const uint8_t *data, int 
> size, H264ParamSets *ps,
> switch (nal->type) {
> case H264_NAL_SPS:
> ret = ff_h264_decode_seq_parameter_set(>gb, logctx, ps, 0);
> -if (ret < 0)
> +if (ret < 0 && (err_recognition & AV_EF_EXPLODE))
> goto fail;
> break;
> case H264_NAL_PPS:
> ret = ff_h264_decode_picture_parameter_set(>gb, logctx, ps,
>nal->size_bits);
> -if (ret < 0)
> +if (ret < 0 && (err_recognition & AV_EF_EXPLODE))
> goto fail;
> break;
> default:

The error code returned by decode_extradata_ps() is inconsistent after this
its not "if any failed" it is returning an error if the last failed

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

When you are offended at any man's fault, turn to yourself and study your
own failings. Then you will forget your anger. -- Epictetus


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] h264: fix RTSP stream decoding

2017-12-29 Thread ser...@gavrushkin.com
> Please add "Fixes ticket #6422" to the commit message.
> 
> And maybe remove "rtsp" from the commit title, the issue
> is reproducible with files.

Done.

Please feel free to edit commit title/message as you wish for merge.

Thank you, 
Sergey

-



From e90ef7b56d4147ff6555468f0154321b55596846 Mon Sep 17 00:00:00 2001
From: Sergey Gavrushkin >
Date: Fri, 29 Dec 2017 20:03:50 +0300
Subject: [PATCH] h264: fix decoding

Fixes ticket #6422 . It is a regression fix for an issue that was introduced in 
commit
98c97994c5b90bdae02accb155eeceeb5224b8ef. Variable err_recognition is
ignored while extradata is decoded and the whole decoding process is
failed due to timeout.
---
libavcodec/h264_parse.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/libavcodec/h264_parse.c b/libavcodec/h264_parse.c
index fee28d9..009d50c 100644
--- a/libavcodec/h264_parse.c
+++ b/libavcodec/h264_parse.c
@@ -347,7 +347,7 @@ int ff_h264_init_poc(int pic_field_poc[2], int *pic_poc,
}

static int decode_extradata_ps(const uint8_t *data, int size, H264ParamSets *ps,
-   int is_avc, void *logctx)
+   int is_avc, int err_recognition, void *logctx)
{
H2645Packet pkt = { 0 };
int i, ret = 0;
@@ -363,13 +363,13 @@ static int decode_extradata_ps(const uint8_t *data, int 
size, H264ParamSets *ps,
switch (nal->type) {
case H264_NAL_SPS:
ret = ff_h264_decode_seq_parameter_set(>gb, logctx, ps, 0);
-if (ret < 0)
+if (ret < 0 && (err_recognition & AV_EF_EXPLODE))
goto fail;
break;
case H264_NAL_PPS:
ret = ff_h264_decode_picture_parameter_set(>gb, logctx, ps,
   nal->size_bits);
-if (ret < 0)
+if (ret < 0 && (err_recognition & AV_EF_EXPLODE))
goto fail;
break;
default:
@@ -393,7 +393,7 @@ static int decode_extradata_ps_mp4(const uint8_t *buf, int 
buf_size, H264ParamSe
{
int ret;

-ret = decode_extradata_ps(buf, buf_size, ps, 1, logctx);
+ret = decode_extradata_ps(buf, buf_size, ps, 1, err_recognition, logctx);
if (ret < 0 && !(err_recognition & AV_EF_EXPLODE)) {
GetByteContext gbc;
PutByteContext pbc;
@@ -425,7 +425,7 @@ static int decode_extradata_ps_mp4(const uint8_t *buf, int 
buf_size, H264ParamSe
escaped_buf_size = bytestream2_tell_p();
AV_WB16(escaped_buf, escaped_buf_size - 2);

-(void)decode_extradata_ps(escaped_buf, escaped_buf_size, ps, 1, 
logctx);
+(void)decode_extradata_ps(escaped_buf, escaped_buf_size, ps, 1, 
err_recognition, logctx);
// lorex.mp4 decodes ok even with extradata decoding failing
av_freep(_buf);
}
@@ -486,7 +486,7 @@ int ff_h264_decode_extradata(const uint8_t *data, int size, 
H264ParamSets *ps,
*nal_length_size = (data[4] & 0x03) + 1;
} else {
*is_avc = 0;
-ret = decode_extradata_ps(data, size, ps, 0, logctx);
+ret = decode_extradata_ps(data, size, ps, 0, err_recognition, logctx);
if (ret < 0)
return ret;
}
-- 
2.6.4
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] h264: fix RTSP stream decoding

2017-12-29 Thread Carl Eugen Hoyos
2017-12-29 18:43 GMT+01:00 ser...@gavrushkin.com :

> From e90ef7b56d4147ff6555468f0154321b55596846 Mon Sep 17 00:00:00 2001
> From: Sergey Gavrushkin 
> Date: Fri, 29 Dec 2017 20:03:50 +0300
> Subject: [PATCH] h264: fix RTSP stream decoding
>
> It is a regression fix for an issue that was introduced in commit
> 98c97994c5b90bdae02accb155eeceeb5224b8ef. Variable err_recognition is
> ignored while extradata is decoded and the whole decoding process is
> failed due to timeout.

Please add "Fixes ticket #6422" to the commit message.

And maybe remove "rtsp" from the commit title, the issue
is reproducible with files.

Thank you, Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] h264: fix RTSP stream decoding

2017-12-29 Thread Tristan Matthews
Hi,

On Fri, Dec 29, 2017 at 12:43 PM, ser...@gavrushkin.com
 wrote:
> Hello,
>
> FFmpeg stop working with the latest model of camera that I work now. I've 
> investigated it and found that "bad" commit is 
> 98c97994c5b90bdae02accb155eeceeb5224b8ef. There was additional check for ret 
> value and err_recognition, that was ignored while refactoring to h264_parse.c 
> function.
>
> Test RTSP streams are available here:
>
> rtsp://admin:123456@69.42.237.230:1569/mpeg4cif 
> 
> rtsp://admin:123456@69.42.237.230:1568/mpeg4cif

This seems to fix the reported issue here.

Best,
Tristan

>
> P.S. It is my first patch so please write any notes what I should provide for 
> accepting.
>
> Thank you,
> Sergey
>
>
> -
>
>
>
> From e90ef7b56d4147ff6555468f0154321b55596846 Mon Sep 17 00:00:00 2001
> From: Sergey Gavrushkin 
> Date: Fri, 29 Dec 2017 20:03:50 +0300
> Subject: [PATCH] h264: fix RTSP stream decoding
>
> It is a regression fix for an issue that was introduced in commit
> 98c97994c5b90bdae02accb155eeceeb5224b8ef. Variable err_recognition is
> ignored while extradata is decoded and the whole decoding process is
> failed due to timeout.
> ---
>  libavcodec/h264_parse.c | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/libavcodec/h264_parse.c b/libavcodec/h264_parse.c
> index fee28d9..009d50c 100644
> --- a/libavcodec/h264_parse.c
> +++ b/libavcodec/h264_parse.c
> @@ -347,7 +347,7 @@ int ff_h264_init_poc(int pic_field_poc[2], int *pic_poc,
>  }
>
>  static int decode_extradata_ps(const uint8_t *data, int size, H264ParamSets 
> *ps,
> -   int is_avc, void *logctx)
> +   int is_avc, int err_recognition, void *logctx)
>  {
>  H2645Packet pkt = { 0 };
>  int i, ret = 0;
> @@ -363,13 +363,13 @@ static int decode_extradata_ps(const uint8_t *data, int 
> size, H264ParamSets *ps,
>  switch (nal->type) {
>  case H264_NAL_SPS:
>  ret = ff_h264_decode_seq_parameter_set(>gb, logctx, ps, 0);
> -if (ret < 0)
> +if (ret < 0 && (err_recognition & AV_EF_EXPLODE))
>  goto fail;
>  break;
>  case H264_NAL_PPS:
>  ret = ff_h264_decode_picture_parameter_set(>gb, logctx, ps,
> nal->size_bits);
> -if (ret < 0)
> +if (ret < 0 && (err_recognition & AV_EF_EXPLODE))
>  goto fail;
>  break;
>  default:
> @@ -393,7 +393,7 @@ static int decode_extradata_ps_mp4(const uint8_t *buf, 
> int buf_size, H264ParamSe
>  {
>  int ret;
>
> -ret = decode_extradata_ps(buf, buf_size, ps, 1, logctx);
> +ret = decode_extradata_ps(buf, buf_size, ps, 1, err_recognition, logctx);
>  if (ret < 0 && !(err_recognition & AV_EF_EXPLODE)) {
>  GetByteContext gbc;
>  PutByteContext pbc;
> @@ -425,7 +425,7 @@ static int decode_extradata_ps_mp4(const uint8_t *buf, 
> int buf_size, H264ParamSe
>  escaped_buf_size = bytestream2_tell_p();
>  AV_WB16(escaped_buf, escaped_buf_size - 2);
>
> -(void)decode_extradata_ps(escaped_buf, escaped_buf_size, ps, 1, 
> logctx);
> +(void)decode_extradata_ps(escaped_buf, escaped_buf_size, ps, 1, 
> err_recognition, logctx);
>  // lorex.mp4 decodes ok even with extradata decoding failing
>  av_freep(_buf);
>  }
> @@ -486,7 +486,7 @@ int ff_h264_decode_extradata(const uint8_t *data, int 
> size, H264ParamSets *ps,
>  *nal_length_size = (data[4] & 0x03) + 1;
>  } else {
>  *is_avc = 0;
> -ret = decode_extradata_ps(data, size, ps, 0, logctx);
> +ret = decode_extradata_ps(data, size, ps, 0, err_recognition, 
> logctx);
>  if (ret < 0)
>  return ret;
>  }
> --
> 2.6.4
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] h264: fix RTSP stream decoding

2017-12-29 Thread ser...@gavrushkin.com
Hello,

FFmpeg stop working with the latest model of camera that I work now. I've 
investigated it and found that "bad" commit is 
98c97994c5b90bdae02accb155eeceeb5224b8ef. There was additional check for ret 
value and err_recognition, that was ignored while refactoring to h264_parse.c 
function.

Test RTSP streams are available here:

rtsp://admin:123456@69.42.237.230:1569/mpeg4cif 

rtsp://admin:123456@69.42.237.230:1568/mpeg4cif

P.S. It is my first patch so please write any notes what I should provide for 
accepting. 

Thank you,
Sergey


-



From e90ef7b56d4147ff6555468f0154321b55596846 Mon Sep 17 00:00:00 2001
From: Sergey Gavrushkin 
Date: Fri, 29 Dec 2017 20:03:50 +0300
Subject: [PATCH] h264: fix RTSP stream decoding

It is a regression fix for an issue that was introduced in commit
98c97994c5b90bdae02accb155eeceeb5224b8ef. Variable err_recognition is
ignored while extradata is decoded and the whole decoding process is
failed due to timeout.
---
 libavcodec/h264_parse.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/libavcodec/h264_parse.c b/libavcodec/h264_parse.c
index fee28d9..009d50c 100644
--- a/libavcodec/h264_parse.c
+++ b/libavcodec/h264_parse.c
@@ -347,7 +347,7 @@ int ff_h264_init_poc(int pic_field_poc[2], int *pic_poc,
 }
 
 static int decode_extradata_ps(const uint8_t *data, int size, H264ParamSets 
*ps,
-   int is_avc, void *logctx)
+   int is_avc, int err_recognition, void *logctx)
 {
 H2645Packet pkt = { 0 };
 int i, ret = 0;
@@ -363,13 +363,13 @@ static int decode_extradata_ps(const uint8_t *data, int 
size, H264ParamSets *ps,
 switch (nal->type) {
 case H264_NAL_SPS:
 ret = ff_h264_decode_seq_parameter_set(>gb, logctx, ps, 0);
-if (ret < 0)
+if (ret < 0 && (err_recognition & AV_EF_EXPLODE))
 goto fail;
 break;
 case H264_NAL_PPS:
 ret = ff_h264_decode_picture_parameter_set(>gb, logctx, ps,
nal->size_bits);
-if (ret < 0)
+if (ret < 0 && (err_recognition & AV_EF_EXPLODE))
 goto fail;
 break;
 default:
@@ -393,7 +393,7 @@ static int decode_extradata_ps_mp4(const uint8_t *buf, int 
buf_size, H264ParamSe
 {
 int ret;
 
-ret = decode_extradata_ps(buf, buf_size, ps, 1, logctx);
+ret = decode_extradata_ps(buf, buf_size, ps, 1, err_recognition, logctx);
 if (ret < 0 && !(err_recognition & AV_EF_EXPLODE)) {
 GetByteContext gbc;
 PutByteContext pbc;
@@ -425,7 +425,7 @@ static int decode_extradata_ps_mp4(const uint8_t *buf, int 
buf_size, H264ParamSe
 escaped_buf_size = bytestream2_tell_p();
 AV_WB16(escaped_buf, escaped_buf_size - 2);
 
-(void)decode_extradata_ps(escaped_buf, escaped_buf_size, ps, 1, 
logctx);
+(void)decode_extradata_ps(escaped_buf, escaped_buf_size, ps, 1, 
err_recognition, logctx);
 // lorex.mp4 decodes ok even with extradata decoding failing
 av_freep(_buf);
 }
@@ -486,7 +486,7 @@ int ff_h264_decode_extradata(const uint8_t *data, int size, 
H264ParamSets *ps,
 *nal_length_size = (data[4] & 0x03) + 1;
 } else {
 *is_avc = 0;
-ret = decode_extradata_ps(data, size, ps, 0, logctx);
+ret = decode_extradata_ps(data, size, ps, 0, err_recognition, logctx);
 if (ret < 0)
 return ret;
 }
-- 
2.6.4

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel