[FFmpeg-devel] [RFC/PATCH 5/8] tidsp: add init/close

[Reimar Döffinger]

On Tue, Sep 07, 2010 at 05:42:10PM +0300, Felipe Contreras wrote:
> On Tue, Sep 7, 2010 at 3:28 PM, Ronald S. Bultje <rsbultje at gmail.com> wrote:
> > On Tue, Sep 7, 2010 at 6:33 AM, Felipe Contreras
> > <felipe.contreras at gmail.com> wrote:
> >> On Tue, Sep 7, 2010 at 1:09 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> >>> On Mon, Sep 06, 2010 at 01:15:32AM +0300, Felipe Contreras wrote:
> >>>> Signed-off-by: Felipe Contreras <felipe.contreras at gmail.com>
> >>>> ---
> >>>> ?libavcodec/tidsp_mpeg4.c | ? 28 ++++++++++++++++++++++++++++
> >>>> ?1 files changed, 28 insertions(+), 0 deletions(-)
> >>>>
> >>>> diff --git a/libavcodec/tidsp_mpeg4.c b/libavcodec/tidsp_mpeg4.c
> >>>> index 3c3190d..1ed2e90 100644
> >>>> --- a/libavcodec/tidsp_mpeg4.c
> >>>> +++ b/libavcodec/tidsp_mpeg4.c
> >>>> @@ -8,8 +8,34 @@
> >>>> ? * in the packaging of this file.
> >>>> ? */
> >>>>
> >>>> +#include "tidsp/tidsp.h"
> >>>> +
> >>>> ?#include "avcodec.h"
> >>>>
> >>>> +struct td_av_context {
> >>>> + ? ? struct td_context *td_ctx;
> >>>> +};
> >>>
> >>> why this wraper struct?
> >>
> >> See patch 8/8: tidsp: process buffers.
> >>
> >> The code in the 'tidsp' directory (and thus struct td_context) is
> >> supposed to be independent of FFmpeg, and according to other people's
> >> comments I should strip out and make an independent library.
> >
> > So drop td_av_context and use td_context directly. :-). Saves you a
> > new extra struct.
> 
> If you see patch #8, you would see that I add an output_buffer there;
> that doesn't belong into 'struct td_context' -- it's specific to the
> design for FFmpeg.

If you need to refer to a different patch to explain something in
another one that usually is a strong sign you split the patches in
a bad way.
A separate patch should contain exactly that amount of changes
that you can reason about and argue for independently, not
more and not less.