[FFmpeg-devel] [RFC] libavfilter audio framework - split patches

[Stefano Sabatini]

On date Friday 2010-07-16 01:08:25 -0700, S.N. Hemanth Meenakshisundaram encoded:
>
>>>> On 07/15/2010 04:52 AM, S.N. Hemanth Meenakshisundaram wrote:
>>>>> On 07/14/2010 07:51 AM, Michael Niedermayer wrote:
>>>>>> [...]
>>>>>> to elaborate on this, we need patches that apply to svn.
>>>>>> you can send a patch series so that patch n depends on patches 0..n-1
>>>>>> to be applied before it.
>>>>>> but if patch x (x<n) is changed due to discussions all later patches
>>>>>> must be rebased on the new code. We dont apply bad patches and then
>>>>>> apply fixes on top.
>>>>>>
>>>>>>    
>>>>
>>>> [...]
>>>>
>>>> Am sending the series of patches again with the changes pointed out 
>>>> earlier. [...]
>
> This is audio framework formats.c changes.

Again, never split declaration and implementation, they need to stay
togheter.

> diff --git a/libavfilter/formats.c b/libavfilter/formats.c
> index bd1086f..0392a59 100644
> --- a/libavfilter/formats.c
> +++ b/libavfilter/formats.c
> @@ -75,7 +75,11 @@ AVFilterFormats *avfilter_make_format_list(const int *fmts)
>      AVFilterFormats *formats;
>      int count;
>  
> -    for (count = 0; fmts[count] != PIX_FMT_NONE; count++)
> +    /**
> +     * FIXME: Both PIX_FMT_NONE and SAMPLE_FMT_NONE now have value -1 so this works.
> +     * Need to handle the case where this may not be true.
> +     */
> +    for (count = 0; fmts[count] != -1; count++)
>          ;
>  
>      formats               = av_mallocz(sizeof(AVFilterFormats));
> @@ -115,6 +119,34 @@ AVFilterFormats *avfilter_all_colorspaces(void)
>      return ret;
>  }
>  
> +int avfilter_add_sampleformat(AVFilterFormats **avff, enum SampleFormat sample_fmt)
> +{
> +    enum SampleFormat *sample_fmts;
> +
> +    if (!(*avff) && !(*avff = av_mallocz(sizeof(AVFilterFormats))))
> +        return AVERROR(ENOMEM);
> +
> +    sample_fmts = av_realloc((*avff)->formats,
> +                          sizeof((*avff)->formats) * ((*avff)->format_count+1));
> +    if (!sample_fmts)
> +        return AVERROR(ENOMEM);
> +
> +    (*avff)->formats = sample_fmts;
> +    (*avff)->formats[(*avff)->format_count++] = sample_fmt;
> +    return 0;
> +}

We can avoit this duplication in two ways: either using an
AVFilterFormat struct instead of enum PixelFormat/enum SampleFormat,
or simply using an int for representing both formats.

The new function may be called
int avfilter_add_format(AVFilterFormats **avff, int fmt);
or
int avfilter_add_format(AVFilterFormats **avff, AVFilterFormat *avf);

> +
> +AVFilterFormats *avfilter_all_sampleformats(void)
> +{
> +    AVFilterFormats *ret = NULL;
> +    enum SampleFormat sample_fmt;
> +
> +    for (sample_fmt = 0; sample_fmt < SAMPLE_FMT_NB; sample_fmt++)
> +        avfilter_add_sampleformat(&ret, sample_fmt);
> +
> +    return ret;
> +}

This may be refactored like:
AVFilterFormats *avfilter_all_formats(enum AVMediaType type) ...

Regards.