[FFmpeg-devel] rough PATCH: reducing the memory copies

[Marc Hoffman]

On 6/26/07, Michael Niedermayer <michaelni at gmx.at> wrote:
>
> Hi
>
> On Tue, Jun 26, 2007 at 08:33:34PM -0400, Marc Hoffman wrote:
> > On 6/26/07, Marc Hoffman <mmhoffm at gmail.com> wrote:
> > >
> > >
> > >
> > > On 6/25/07, mmh <mmh at pleasantst.com> wrote:
> > > >
> > > > Michael Niedermayer writes:
> > > > > Hi
> > > > >
> > > > > On Mon, Jun 25, 2007 at 08:01:15PM -0400, mmh wrote:
> > > > > Content-Description: message body text
> > > > > >
> > > > > > This saves me about 50MIPS + reduce cache stress, by
> elliminating
> > > > this
> > > > > > memory copy, which I guess happens later anyways again.
> > > > > >
> > > > > > I know that I need to creat a new function in utils.c for this
> like
> > > > > > av_new_packet probably av_ref_packet which does the work in the
> if
> > > > variant.
> > > > > >
> > > > > > What are the problems with this and, other ideas that comes to
> mind?
> > > > >
> > > > > there probably wont be any problems, it should work fine though
> > > > > as you suspected it will sometimes tough not always cause a
> memcpy()
> > > > > to be done later ...
> > > > >
> > > > >
> > > > > [...]
> > > > > > -
> > > > > > +    PROF_BEGIN();
> > > > >
> > > > > this doesnt belong in here, the code is messy enough as is
> > > > >
> > > > >
> > > > > >      ptr = s->video_buf + s->gb_buffers.offsets[s->gb_frame];
> > > > > > -    memcpy(buf, ptr, s->frame_size);
> > > > > >
> > > > > > +    if (s->gb_buffers.frames > 1) {
> > > > > > +        av_free (pkt->data);
> > > > >
> > > > > alloc+free is not ok, dont malloc()
> > > > >
> > > > >
> > > > > > +        pkt->destruct= av_destruct_packet_nofree;
> > > > > > +        pkt->data = ptr;
> > > > > > +  pkt->size = s->frame_size;
> > > > >
> > > > > tab
> > > > >
> > > > >
> > > >
> > > > Trivial prototype correction.. Sorry for the confusion.
> > >
> > >
> > >
> > > Any objections to this patch being applied?
> > >
> >
> > I'm going to appy this tomorrow morning if there are no objections.
>
> please wait until someone reviewed it


I thought you did already call out that this would work fine. I cleaned it
up and made it modular.  Now the same thing touches a few other files but
its cleaner with respect to trying to maintain the internal API of
libavcodec.  I would like to get this out of my working tree before I take
holiday.  Sorry for pushing for closure.

BTW I appreciate your reviews, they are always so insightful.

Marc