Re: mciavi32: Add partial support for MCI_DGV_PLAY_REPEAT

Bruno,

>If you ever have time, please take a look.
+            wma->dwCurrVideoFrame = wma->dwCurrAudioBlock = 0;
I'm sorry I'd reject this patch because it's missing integration for the MCI_FROM keyword.

Example using MS' clock.avi:
OPEN clock.avi alias c
WINDOW c State Show ; may still be needed in Wine
PLAY c FROM 2 TO 5 repeat
should open the window and repeat stepping the clock from 3 to 6 o'clock.
(You remember my MCI shell in bugzilla, don't you?)

You could argue that there's no need to bother for MCI_FROM, because it's not yet supported with audio output (see mmoutput.c). However, play c FROM x to y currently works with video only output (e.g. clock.avi), thus new patches should at least support what's already in place.

BTW (not in your patch):
         FIXME("Should recompute dwCurrAudioBlock, except unsynchronized sound & video\n");
Typo, should be "expect"

Supporting sound with MCI_FROM non-zero would be the matter of another patch for a volunteer...

In summary, I believe that a better patch would either add dwFromVideoFrame to WINE_MCIAVI or remember dwCurrentVideoFrame (but see below) at the entrance of MCIAVI_Player and use that.

Note that I'd expect any patch to handle this scenario correctly:
Play c FROM 2 TO 5 repeat
Play c TO 8 repeat
In native, the second play would *continue* playing from the current position upto 9 o'clock, then resume from 0 (not 3 o'clock, because 0 is the default FROM value in the second play command).

Well, that's probably too much for a single patch, since it would likely require keeping the player thread alive, just changing its start/repeat and end positions, rather than restarting a new one.

Regards,
	Jörg Höhle

mciavi32: Add partial support for MCI_DGV_PLAY_REPEAT

[Bruno Jesus]

On Monday, August 31, 2015, <Joerg-Cyril.Hoehle(a)t-systems.com> wrote:

> Bruno,
>
> >If you ever have time, please take a look.
> +            wma->dwCurrVideoFrame = wma->dwCurrAudioBlock = 0;
> I'm sorry I'd reject this patch because it's missing integration for the
> MCI_FROM keyword.
>
> Example using MS' clock.avi:
> OPEN clock.avi alias c
> WINDOW c State Show ; may still be needed in Wine
> PLAY c FROM 2 TO 5 repeat
> should open the window and repeat stepping the clock from 3 to 6 o'clock.
> (You remember my MCI shell in bugzilla, don't you?)
>
> You could argue that there's no need to bother for MCI_FROM, because it's
> not yet supported with audio output (see mmoutput.c). However, play c FROM
> x to y currently works with video only output (e.g. clock.avi), thus new
> patches should at least support what's already in place.
>
> BTW (not in your patch):
>          FIXME("Should recompute dwCurrAudioBlock, except unsynchronized
> sound & video\n");
> Typo, should be "expect"
>
> Supporting sound with MCI_FROM non-zero would be the matter of another
> patch for a volunteer...
>
> In summary, I believe that a better patch would either add
> dwFromVideoFrame to WINE_MCIAVI or remember dwCurrentVideoFrame (but see
> below) at the entrance of MCIAVI_Player and use that.
>
> Note that I'd expect any patch to handle this scenario correctly:
> Play c FROM 2 TO 5 repeat
> Play c TO 8 repeat
> In native, the second play would *continue* playing from the current
> position upto 9 o'clock, then resume from 0 (not 3 o'clock, because 0 is
> the default FROM value in the second play command).
>
> Well, that's probably too much for a single patch, since it would likely
> require keeping the player thread alive, just changing its start/repeat and
> end positions, rather than restarting a new one.


Thank you very much for your review, i knew you would be interested. I did
use the mcishell to test with other avi files. I'll try to address the
issues mentioned and resend, I was not aware of the from argument.