[FFmpeg-devel] [PATCH 2/2] avfilter: add sidechaingate filter

Ganesh Ajjanagadde gajjanag at mit.edu
Fri Dec 4 19:49:53 CET 2015


On Fri, Dec 4, 2015 at 11:36 AM, Paul B Mahol <onemda at gmail.com> wrote:
> On 12/4/15, Ganesh Ajjanagadde <gajjanag at mit.edu> wrote:
>> On Wed, Dec 2, 2015 at 1:42 AM, Paul B Mahol <onemda at gmail.com> wrote:
>>> On 12/2/15, Paul B Mahol <onemda at gmail.com> wrote:
>>>> On 12/2/15, Ganesh Ajjanagadde <gajjanag at mit.edu> wrote:
>>>>> On Tue, Dec 1, 2015 at 2:14 PM, Paul B Mahol <onemda at gmail.com> wrote:
>>>>>> On 12/1/15, Nicolas George <george at nsup.org> wrote:
>>>>>>> Le primidi 11 frimaire, an CCXXIV, Paul B Mahol a ecrit :
>>>>>>>> Similar how its freed when no longer used.
>>>>>>>
>>>>>>> Please elaborate. I know the API, I do not see what you suggest.
>>>>>>>
>>>>>>> (Thanks for trimming.)
>>>>>>>
>>>>>>> Regards,
>>>>>>
>>>>>> After carefully looking at this functon, I see no leaking at all.
>>>>>
>>>>> Care to elaborate on this? My question is: what happens when e.g
>>>>> layouts get set and allocated/ref'ed correctly, but while trying to
>>>>> allocate formats, ENOMEM occurs? Or in other words, where does the
>>>>> formats get deallocated in such a case? And why does Coverity flag
>>>>> these things?
>>>>
>>>> Maybe it is for test program: libavfilter/filtfmts.c
>>>>
>>>
>>> Also I changed return value of query_formats to always be -1 and run
>>> program under 'valgrind --leak-check=full --show-leak-kinds=all'
>>> I see unrelated leaks from af_aformat and others, and no leaks from
>>> query_formats.
>>
>> You are being ambiguous here, and your testing was likely not thorough
>> enough.
>>
>> Please try the following:
>> diff --git a/libavfilter/af_agate.c b/libavfilter/af_agate.c
>> index 291e803..416b671 100644
>> --- a/libavfilter/af_agate.c
>> +++ b/libavfilter/af_agate.c
>> @@ -188,6 +188,7 @@ static int query_formats(AVFilterContext *ctx)
>>      layouts = ff_all_channel_counts();
>>      if (!layouts)
>>          return AVERROR(ENOMEM);
>> +    return AVERROR(ENOMEM);
>
> This is nonsense.

You have a very bad habit of calling something "nonsense". I can reply
in kind saying that your original "carefully looking at this function"
was nonsense, since the leak is demonstrated below. Your test was with
a return -1, which is also nonsensical.

I moreover did say it is not a proper illustration, and gave a better
one with instructions to reproduce below. Please stop your habit of
selectively replying and thus creating biased perspectives for a
casual observer of what I point out. Your code was broken wrt
memleaks, stop defending it, test the example I gave, and work towards
creating a constructive solution.

> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


More information about the ffmpeg-devel mailing list