Re: [libav-devel] [PATCH] zerocodec: check if there is previous frame

2012-05-04 Thread Kostya Shishkov
On Fri, May 04, 2012 at 10:41:47AM -0400, Derek Buitenhuis wrote:
 From: Paul B Mahol one...@gmail.com
 
 Fixes crash in bug #1219.
 
 Signed-off-by: Paul B Mahol one...@gmail.com
 ---
  libavcodec/zerocodec.c |4 
  1 file changed, 4 insertions(+)
 
 diff --git a/libavcodec/zerocodec.c b/libavcodec/zerocodec.c
 index 6c57e05..54a7e06 100644
 --- a/libavcodec/zerocodec.c
 +++ b/libavcodec/zerocodec.c
 @@ -65,6 +65,10 @@ static int zerocodec_decode_frame(AVCodecContext *avctx, 
 void *data,
  pic-key_frame = 1;
  pic-pict_type = AV_PICTURE_TYPE_I;
  } else {
 +if (prev == NULL) {
 +av_log(avctx, AV_LOG_ERROR, No previous frame!\n);
 +return AVERROR_INVALIDDATA;
 +}
  pic-key_frame = 0;
  pic-pict_type = AV_PICTURE_TYPE_P;
  }
 -- 

nit: if (!prev) looks better to me
in any case patch looks OK
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH] zerocodec: check if there is previous frame

2012-05-04 Thread Derek Buitenhuis
On 04/05/2012 10:44 AM, Kostya Shishkov wrote:
 nit: if (!prev) looks better to me
 in any case patch looks OK

I agree, but I elected to leave the patch untouched.

- Derek
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH] zerocodec: check if there is previous frame

2012-05-04 Thread Måns Rullgård
Derek Buitenhuis derek.buitenh...@gmail.com writes:

 On 04/05/2012 10:44 AM, Kostya Shishkov wrote:
 nit: if (!prev) looks better to me
 in any case patch looks OK

 I agree, but I elected to leave the patch untouched.

Why?

-- 
Måns Rullgård
m...@mansr.com
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH] zerocodec: check if there is previous frame

2012-05-04 Thread Derek Buitenhuis
On 04/05/2012 10:48 AM, Måns Rullgård wrote:
 I agree, but I elected to leave the patch untouched.
 Why?

Modifying others'/existing patches for personal preferences
is silly, in my opinion, is all.

- Derek
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH] zerocodec: check if there is previous frame

2012-05-04 Thread Måns Rullgård
Derek Buitenhuis derek.buitenh...@gmail.com writes:

 On 04/05/2012 10:48 AM, Måns Rullgård wrote:
 I agree, but I elected to leave the patch untouched.
 Why?

 Modifying others'/existing patches for personal preferences
 is silly, in my opinion, is all.

We have style rules which should be adhered to, even if someone submits
a patch in violation.

-- 
Måns Rullgård
m...@mansr.com
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH] zerocodec: check if there is previous frame

2012-05-04 Thread Derek Buitenhuis
On 04/05/2012 11:19 AM, Måns Rullgård wrote:
 We have style rules which should be adhered to, even if someone submits
 a patch in violation.

There is no violation here.
Such a case is not documented by our style rules.

- Derek
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH] zerocodec: check if there is previous frame

2012-05-04 Thread Måns Rullgård
Derek Buitenhuis derek.buitenh...@gmail.com writes:

 On 04/05/2012 11:19 AM, Måns Rullgård wrote:
 We have style rules which should be adhered to, even if someone submits
 a patch in violation.

 There is no violation here.
 Such a case is not documented by our style rules.

You were asked to change it.  Please comply.

-- 
Måns Rullgård
m...@mansr.com
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH] zerocodec: check if there is previous frame

2012-05-04 Thread Kostya Shishkov
On Fri, May 04, 2012 at 04:29:21PM +0100, Måns Rullgård wrote:
 Derek Buitenhuis derek.buitenh...@gmail.com writes:
 
  On 04/05/2012 11:19 AM, Måns Rullgård wrote:
  We have style rules which should be adhered to, even if someone submits
  a patch in violation.
 
  There is no violation here.
  Such a case is not documented by our style rules.

There's nothing bad with making patch slightly more beautiful even if it's not
yours. So, Derek, next time think of better reasons why the patch should be
left as is.

 
 You were asked to change it.  Please comply.

It was merely a nit.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH] zerocodec: check if there is previous frame

2012-05-04 Thread Derek Buitenhuis
On 04/05/2012 11:59 AM, Kostya Shishkov wrote:
   There is no violation here.
   Such a case is not documented by our style rules.
 There's nothing bad with making patch slightly more beautiful even if it's not
 yours. So, Derek, next time think of better reasons why the patch should be
 left as is.

My point was, in this case, it's entirely opinion. It's neither documented nor
standarduzed. I'll change it, but this seems to be a case of X's opinion on
how this should look is more valid than Y's.

  You were asked to change it.  Please comply.
 It was merely a nit.

Exactly.

- Derek
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH] zerocodec: check if there is previous frame

2012-05-04 Thread Diego Biurrun
On Fri, May 04, 2012 at 10:41:47AM -0400, Derek Buitenhuis wrote:
 Fixes crash in bug #1219.

No such bug number.

Please take care of such things when porting patches.

Diego
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH] zerocodec: check if there is previous frame

2012-05-04 Thread Derek Buitenhuis
I realized that as soon as I sent it. It's been fixed locally.

- Derek

On 04/05/2012 12:21 PM, Diego Biurrun wrote:
 No such bug number.
 
 Please take care of such things when porting patches.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH] zerocodec: check if there is previous frame

2012-05-04 Thread Diego Biurrun
On Fri, May 04, 2012 at 11:21:45AM -0400, Derek Buitenhuis wrote:
 On 04/05/2012 11:19 AM, Måns Rullgård wrote:
  We have style rules which should be adhered to, even if someone submits
  a patch in violation.
 
 There is no violation here.
 Such a case is not documented by our style rules.

We do have style rules, traditions might be the more appropriate term,
which are not fixed in writing, but passed down orally through the
generations.  Patches welcome :)

Derek
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel