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

[Michael Niedermayer]

Hi

On Tue, Jun 26, 2007 at 09:42:13PM -0400, Marc Hoffman wrote:
> 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. 

yes, it should work in principle


> I cleaned it
> up 

there are tabs and cosmetics in it


> 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.

you always push, also the code is buggy it starts capture into the very
buffer which you return IIRC
and adding a function to the public interface is not something we do
quickly before the holiday, even less so if its just to avoid 2 lines of
code in grab.c

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

it is not once nor twice but times without number that the same ideas make
their appearance in the world. -- Aristotle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070627/4678ccf0/attachment.pgp>