[MPlayer-dev-eng] [PATCH] vdpau fixes and timing enhancements
The Wanderer
inverseparadox at comcast.net
Mon Apr 20 12:44:02 CEST 2009
Dan Oscarsson wrote:
> On 2009-04-18 at 16:34 +0200 Diego Biurrun wrote:
>
>> > The patches include:
>>
>> ... a lot of things ..
>>
>> Please split the patch into manageable pieces. Also, please avoid tabs
>> and trailing whitespace.
>
> I did that before but got nowhere. It is a lot of work to split it into
> parts, though the mplayer.c patches are fairly easy to split out. That I
> did before.
>
> There should not be any tabs or tailing whitespace unless my Linux tools
> add them when I copy between windows.
At a quick scan, I see trailing whitespace in only one place, in the
fifth hunk of the mplayer.c patch:
> +if (match_vsync_speed && vo_vsync_interval > 0) {
> + if (!vsynced_speed) {
> + float fps_interval, vo_interval;
> +
> + fps_interval = 1000000.0f/vo_fps;
> + vo_interval = vo_vsync_interval/1000.0f;
> + if (fps_interval*0.95 < vo_interval && vo_interval < fps_interval*1.05) {
> + matched_playback_speed = fps_interval/vo_interval;
> + mp_msg(MSGT_CPLAYER,MSGL_INFO,"Matching speed to vsync %3.10f\n",matched_playback_speed);
> + } else if (fps_interval/2*0.95 < vo_interval && vo_interval < fps_interval/2*1.05) {
on the last line above.
The only tabs I see (except on unchanged lines) are in the PLAY VIDEO
section, which (now that I look closer) actually seems to be likewise
unchanged - just moved. I don't recall whether standard practice is to
remove tabs on lines which are being moved but otherwise not modified,
but it would at the least seem like a cosmetic change to me...
>>> @@ -288,26 +352,21 @@
>>> if (output_surface_width < vo_dwidth || output_surface_height < vo_dheight) {
>>> if (output_surface_width < vo_dwidth) {
>>> output_surface_width += output_surface_width >> 1;
>>> - output_surface_width = FFMAX(output_surface_width, vo_dwidth);
>>> + output_surface_width = FFMIN(vo_screenwidth,FFMAX(output_surface_width, vo_dwidth));
>>> }
>>> if (output_surface_height < vo_dheight) {
>>> output_surface_height += output_surface_height >> 1;
>>> - output_surface_height = FFMAX(output_surface_height, vo_dheight);
>>> - }
>>> - // Creation of output_surfaces
>>> - for (i = 0; i <= NUM_OUTPUT_SURFACES; i++) {
>>> - if (output_surfaces[i] != VDP_INVALID_HANDLE)
>>> - vdp_output_surface_destroy(output_surfaces[i]);
>>> - vdp_st = vdp_output_surface_create(vdp_device, VDP_RGBA_FORMAT_B8G8R8A8,
>>> - output_surface_width, output_surface_height,
>>> - &output_surfaces[i]);
>>> - CHECK_ST_WARNING("Error when calling vdp_output_surface_create")
>>> - mp_msg(MSGT_VO, MSGL_DBG2, "OUT CREATE: %u\n", output_surfaces[i]);
>>> + output_surface_height = FFMIN(vo_screenheight,FFMAX(output_surface_height, vo_dheight));
>>> }
>>> + // Creation of osd surface
>>> + if (osd_surface != VDP_INVALID_HANDLE)
>>> + vdp_output_surface_destroy(osd_surface);
>>> + vdp_st = vdp_output_surface_create(vdp_device, VDP_RGBA_FORMAT_B8G8R8A8,
>>> + output_surface_width, output_surface_height,
>>> + &osd_surface);
>>> + CHECK_ST_WARNING("Error when calling vdp_output_surface_create")
>>> + mp_msg(MSGT_VO, MSGL_DBG2, "OUT CREATE OSD: %u\n", osd_surface);
>>
>> cosmetics
>
> What is bad here?
The vdp_st = vdp_output_surface_create() call, and the following
mp_msg(), seem unchanged except for being reindented. That reindentation
constitutes a cosmetic change, and so needs to be done as a separate
patch following the functional changes.
--
The Wanderer
Warning: Simply because I argue an issue does not mean I agree with any
side of it.
Secrecy is the beginning of tyranny.
More information about the MPlayer-dev-eng
mailing list