Re: [FFmpeg-devel] [PATCH] avcodec/encode: don't return immediately on failure

2017-10-03 Thread James Almer 
On 10/3/2017 5:23 AM, wm4 wrote:
> On Tue,  3 Oct 2017 01:55:44 -0300
> James Almer  wrote:
> 
>> Fixes memleaks introduced by skipping cleanup at the end of the
>> functions.
>>
>> Regression since a22c6a4796ca1f2cbee6784262515da876fbec22.
>>
>> Signed-off-by: James Almer 
>> ---
>>  libavcodec/encode.c | 16 
>>  1 file changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/libavcodec/encode.c b/libavcodec/encode.c
>> index c152228c92..841a185738 100644
>> --- a/libavcodec/encode.c
>> +++ b/libavcodec/encode.c
>> @@ -225,10 +225,10 @@ int attribute_align_arg 
>> avcodec_encode_audio2(AVCodecContext *avctx,
>>  } else if (!avpkt->buf) {
>>  AVPacket tmp = { 0 };
>>  ret = av_packet_ref(, avpkt);
>> -if (ret < 0)
>> -return ret;
>> -av_packet_unref(avpkt);
>> -*avpkt = tmp;
>> +if (!ret) {
>> +av_packet_unref(avpkt);
>> +*avpkt = tmp;
>> +}
>>  }
>>  }
>>  
>> @@ -324,10 +324,10 @@ int attribute_align_arg 
>> avcodec_encode_video2(AVCodecContext *avctx,
>>  } else if (!avpkt->buf) {
>>  AVPacket tmp = { 0 };
>>  ret = av_packet_ref(, avpkt);
>> -if (ret < 0)
>> -return ret;
>> -av_packet_unref(avpkt);
>> -*avpkt = tmp;
>> +if (!ret) {
>> +av_packet_unref(avpkt);
>> +*avpkt = tmp;
>> +}
>>  }
>>  }
>>  
> 
> Seems ok if ret is actually checked in the code below. Something like
> "goto cleanup;" on error would be less confusing though.

Did that instead and pushed. Thanks.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/encode: don't return immediately on failure

2017-10-03 Thread wm4 
On Tue,  3 Oct 2017 01:55:44 -0300
James Almer  wrote:

> Fixes memleaks introduced by skipping cleanup at the end of the
> functions.
> 
> Regression since a22c6a4796ca1f2cbee6784262515da876fbec22.
> 
> Signed-off-by: James Almer 
> ---
>  libavcodec/encode.c | 16 
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/libavcodec/encode.c b/libavcodec/encode.c
> index c152228c92..841a185738 100644
> --- a/libavcodec/encode.c
> +++ b/libavcodec/encode.c
> @@ -225,10 +225,10 @@ int attribute_align_arg 
> avcodec_encode_audio2(AVCodecContext *avctx,
>  } else if (!avpkt->buf) {
>  AVPacket tmp = { 0 };
>  ret = av_packet_ref(, avpkt);
> -if (ret < 0)
> -return ret;
> -av_packet_unref(avpkt);
> -*avpkt = tmp;
> +if (!ret) {
> +av_packet_unref(avpkt);
> +*avpkt = tmp;
> +}
>  }
>  }
>  
> @@ -324,10 +324,10 @@ int attribute_align_arg 
> avcodec_encode_video2(AVCodecContext *avctx,
>  } else if (!avpkt->buf) {
>  AVPacket tmp = { 0 };
>  ret = av_packet_ref(, avpkt);
> -if (ret < 0)
> -return ret;
> -av_packet_unref(avpkt);
> -*avpkt = tmp;
> +if (!ret) {
> +av_packet_unref(avpkt);
> +*avpkt = tmp;
> +}
>  }
>  }
>  

Seems ok if ret is actually checked in the code below. Something like
"goto cleanup;" on error would be less confusing though.

Also why did you essentially change "!(ret<0)" to "!ret"? In theory, all
positive return values indicate success.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] avcodec/encode: don't return immediately on failure

2017-10-02 Thread James Almer 
Fixes memleaks introduced by skipping cleanup at the end of the
functions.

Regression since a22c6a4796ca1f2cbee6784262515da876fbec22.

Signed-off-by: James Almer 
---
 libavcodec/encode.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/libavcodec/encode.c b/libavcodec/encode.c
index c152228c92..841a185738 100644
--- a/libavcodec/encode.c
+++ b/libavcodec/encode.c
@@ -225,10 +225,10 @@ int attribute_align_arg 
avcodec_encode_audio2(AVCodecContext *avctx,
 } else if (!avpkt->buf) {
 AVPacket tmp = { 0 };
 ret = av_packet_ref(, avpkt);
-if (ret < 0)
-return ret;
-av_packet_unref(avpkt);
-*avpkt = tmp;
+if (!ret) {
+av_packet_unref(avpkt);
+*avpkt = tmp;
+}
 }
 }
 
@@ -324,10 +324,10 @@ int attribute_align_arg 
avcodec_encode_video2(AVCodecContext *avctx,
 } else if (!avpkt->buf) {
 AVPacket tmp = { 0 };
 ret = av_packet_ref(, avpkt);
-if (ret < 0)
-return ret;
-av_packet_unref(avpkt);
-*avpkt = tmp;
+if (!ret) {
+av_packet_unref(avpkt);
+*avpkt = tmp;
+}
 }
 }
 
-- 
2.14.1

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