[FFmpeg-devel] [PATCH][VAAPI][2/6] Add common data structures and helpers (take 3)

[Michael Niedermayer]

On Sun, Mar 08, 2009 at 09:55:28AM +0100, Gwenole Beauchesne wrote:
> Le 7 mars 09 ? 21:01, Michael Niedermayer a ?crit :
> 
> >>>> +    else {
> >>>> +        if (rds->slice_data_size + size > rds-
> >>>>> slice_data_size_max) {
> >>>> +            rds->slice_data_size_max += size;
> >>>> +            rds->slice_data = av_realloc(rds->slice_data, rds-
> >>>>> slice_data_size_max);
> >>>> +            if (!rds->slice_data)
> >>>> +                return -1;
> >>>> +        }
> >>>
> >>> memleak and possibly exploitable, at least the values are left in
> >>> inconsistant
> >>> state.
> >>
> >> Could you please explain the memleak part? If rds->slice_data ==
> >> NULL,  the memory is gone anyway.
> >
> > realloc() returning NULL means the original is still there just that
> > it failed re allocating it
> 
> 7.20.3.4 indeed confirms what you say but I couldn't find a single  
> code in lavc operating that way:
> new_buffer = av_*_realloc(buffer, new_size);
> if (!new_buffer) {
>      av_freep(&buffer);
>      // do whatever else/return -1
> }
> buffer = new_buffer;
> 
> would this be what you had in mind?

yes

also it might make sense to add a
av_realloc_and_free()
that does free the original in case of fail and replace all code that
expects these semantics rather ...


> 
> > also glibc memcpy() is shit, even more so for copying ito non system  
> > memory
> > you should maybe look at mplayer which has some memcpy written for  
> > that.
> 
> Hmmm, I think this statement no longer holds for some years now. ;-)  
> Even Agner's doesn't bring that much, if any performance gain.  
> Besides, system libcs generally provide the best memcpy() tuned for  
> the underlying processor and memory hierarchy (caches geometry et  
> al.). This is true for Apple's (commpage provided functions) and even  
> glibc, though depending on several factors (distributor, architecture).

glibc and "best" in the same paragraph makes me want to puke

anyway, actual numbers: (done 3x to show that they are stable)

k7 : cpu clocks=170059006 = 98361us  (1016.663fps)  1525.0MB/s
mmx: cpu clocks=293085663 = 169516us  (589.915fps)  884.9MB/s
sse: cpu clocks=170377116 = 98544us  (1014.775fps)  1522.2MB/s
c: cpu clocks=195054405 = 112817us  (886.391fps)  1329.6MB/s
k7 : cpu clocks=170553318 = 98645us  (1013.736fps)  1520.6MB/s
mmx: cpu clocks=291090475 = 168362us  (593.958fps)  890.9MB/s
sse: cpu clocks=173589832 = 100402us  (995.996fps)  1494.0MB/s
c: cpu clocks=193683165 = 112023us  (892.674fps)  1339.0MB/s
k7 : cpu clocks=171431091 = 99154us  (1008.532fps)  1512.8MB/s
mmx: cpu clocks=298011922 = 172365us  (580.164fps)  870.2MB/s
sse: cpu clocks=173702776 = 100468us  (995.342fps)  1493.0MB/s
c: cpu clocks=194631164 = 112571us  (888.328fps)  1332.5MB/s

(core 2 based cpu in case you wonder)

dirty patch needed to make above run: 
(patch/commit to fix it cleanly in mplayer svn is welcome especially
 by thouse who decided to #include config.h all over the place)

Index: libvo/aclib.c
===================================================================
--- libvo/aclib.c	(revision 28896)
+++ libvo/aclib.c	(working copy)
@@ -21,7 +21,6 @@
  * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
  */
 
-#include "config.h"
 #include <stddef.h>
 #include <string.h>
 #include "cpudetect.h"
Index: libvo/fastmemcpy.h
===================================================================
--- libvo/fastmemcpy.h	(revision 28896)
+++ libvo/fastmemcpy.h	(working copy)
@@ -19,7 +19,6 @@
 #ifndef MPLAYER_FASTMEMCPY_H
 #define MPLAYER_FASTMEMCPY_H
 
-#include "config.h"
 #include <inttypes.h>
 #include <string.h>
 
Index: TOOLS/fastmemcpybench.c
===================================================================
--- TOOLS/fastmemcpybench.c	(revision 28894)
+++ TOOLS/fastmemcpybench.c	(working copy)
@@ -19,9 +19,21 @@
 #include <sys/time.h>
 #include <inttypes.h>
 
+#define ARCH_X86 1
+#define ARCH_X86_64 1
+#define ASMALIGN(ZEROBITS) ".align 1<<" #ZEROBITS "\n\t"
+#define CONFIG_FASTMEMCPY 1
+
+#include "libvo/aclib.c"
+
+#ifdef HAVE_MMX
+#define memcpy fast_memcpy
+#endif
+
 //#define ARR_SIZE 100000
 #define ARR_SIZE (1024*768*2)
 
+
 #ifdef CONFIG_MGA
 
 #include "drivers/mga_vid.h"


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

I know you won't believe me, but the highest form of Human Excellence is
to question oneself and others. -- Socrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090308/9a1ce8a1/attachment.pgp>