[MPlayer-dev-eng] *SUGGESTED* libvo2 patch

[David Holm]

Arpi wrote:

>Hi,
>
>>Hi,
>>here is a suggested libvo2 patch, perhaps arpi should read it through first?
>>
>at first look it's messy!
>
>>--- main/libvo2/img_format.c	Tue Jul 31 02:29:22 2001
>>+++ main-dxr3/libvo2/img_format.c	Sun Nov 18 12:27:13 2001
>>@@ -38,6 +38,8 @@
>> 	case IMGFMT_CLJR: return("Packed CLJR");
>> 	case IMGFMT_YUVP: return("Packed YUVP");
>> 	case IMGFMT_UYVP: return("Packed UYVP");
>>+	case IMGFMT_MPEGPES: return("MPEG-PES");
>>+	case IMGFMT_MPEG12: return("MPEG-(1/2)");
>>
>what's the diff between mpegpes and mpeg12 ? i think they are the same.
>
you add headers to your mpegpes packets, and run mpeg2_decode_data on it 
in dec_video, this isn't necessary for dxr3 boards, and eat unneccessary 
cpu cycles... I guess disabling mpeg2_decode_data if vo = dxr3 would be 
enough...
But I remembered the last time I made alternations outside of libvo2, so 
this time I thought I'd stick to ./

>
>
>> 
>>-// return true if surface is in fast system ram, false if slow (video?) memory
>>-// USED for storing mpeg2 I/P frames only if it's fast enough
>>-#define VO2CTRL_QUERY_SURFACE_FAST 0x101
>>-
>>-// return true if surface is direct rendered, false if indirect (copied first)
>>-// Note: it's usually same as VO2CTRL_GET_SURFACE_SPEED, except for some
>>-// special cases, when video card does the copy from video ram (opengl...)
>>-// USED for deciding external double buffering mode (using 2 surfaces)
>>-#define VO2CTRL_QUERY_SURFACE_DIRECT 0x102
>>-
>>-// Get the upper hardware/Driver limitation (used for double buffering)
>>-#define VO2CTRL_GET_MAX_SURFACES 0x103
>>-
>>
>
>why did you removed these?
>they are used by my under-devel buffering engine...
>
I didn't remove them, I moved them to the bottom of the file, because 
there might be LOTS of VO2CTRL_ stuff, in which case developers would 
have to scroll a long way down to read the vo2_handle_t struct 
implementation etc

>
>
>> typedef struct vo2_info_s
>>@@ -43,7 +24,7 @@
>>     unsigned char* img[3];  // pointer to frame/planes
>>     int stride[3]; // strides (bytes per line) for frame/planes
>>     int format; // RGB / BGR / YUV_PACKED / YUV_PLANAR
>>-    int bpp;  // bits per pixel (15/16/24/32) or YUV fourcc
>>
>why?
>
isn't it pretty clear in format?
or is the bpp fussy in IMGFMT_RGB24 (to take one)

>
>
>>+    void* priv; /* Device private data, don't use globals! */
>>
>:))))))) (see my prev mail)
>
>>+// Save and load device options, if load returns NULL set to default
>>+int vo2_save_devoptions(void);
>>+int vo2_load_devoptions(void);
>>
>???
>
The device options struct is a bit complex to say the least, the benefit 
is it's completely versatile.
I though implementing a few handlers for it would make it easier for 
developers (the options struct is a sigsegv trap to say the least to an 
inexperienced programmer, if I implement handlers for it that can be 
avoided)

>
>
>>+//Query the overlay mode (used for osd and sub)
>>+#define VO2CTRL_QUERY_OVERLAY 0x141
>>
>i think you should get a better name for it (subpicture?)
>
well, it is an overlay, are you asking what a subpicture is or are you 
suggesting _SUBPICTURE for a name?

>
>
>>-static vo2_handle_t* new_handle(){
>>+static vo2_handle_t* new_handle( priv_t *handle )
>>
>see my prev-(N+1) mail
>
yes, I made a new post after reading that one where I pointed out that 
my new_handle modifications should be removed, perhaps my f*cking 
mailserver hasn't manage to deliver that one yet...

//David Holm