winegstream reintroduce parser flushing

[Zlice Z]

Sent a patch yesterday that was mediocre and barely helps.

commit
https://github.com/wine-mirror/wine/commit/5144b27661fcd6705353d832e0383085f...

removed flushing from parser, which after commit bisecting turns out to be
the cause of Fallout 3 radio chugging every new song.

- re-introduced parser flushing (self explanatory, commit said it's not
worth it but it helps here)

- moved some variable initialization in wm_reader.c to after initial
conditional returns. these were in a switch-case before in the middle of
the function and it seems to be part of the stuttering, wasn't moved in the
mentioned commit though

- commented out and left my name by some EOS sets. they used to be checked
for EOS 'events' but those references have been changed to 'buffer' and idk
what the right action is. wine seemed to randomly seg fault on exit with
leaving them on though. suspect they incorrectly guess and set 'end of
stream' ?

[Bernhard Kölbl]

Hi Zlice,

thanks for your patch. Unfortunately you're missing out on quite a few
requirements for this to get accepted. I'd recommend to refactor your
submission using this guide as reference:
https://wiki.winehq.org/Submitting_Patches and to resend it afterwards.

E.g. you left in C++ comments, which aren't allowed.

Thanks,
Bernhard

Zlice Z  schrieb am Fr., 20. Mai 2022, 09:22:
> Sent a patch yesterday that was mediocre and barely helps.
>
> commit
> https://github.com/wine-mirror/wine/commit/5144b27661fcd6705353d832e0383085f...
>
> removed flushing from parser, which after commit bisecting turns out to be
> the cause of Fallout 3 radio chugging every new song.
> - re-introduced parser flushing (self explanatory, commit said it's not
>
> worth it but it helps here)
> - moved some variable initialization in wm_reader.c to after initial
>
> conditional returns. these were in a switch-case before in the middle of
> the function and it seems to be part of the stuttering, wasn't moved in the
> mentioned commit though
> - commented out and left my name by some EOS sets. they used to be checked
>
> for EOS 'events' but those references have been changed to 'buffer' and idk
> what the right action is. wine seemed to randomly seg fault on exit with
> leaving them on though. suspect they incorrectly guess and set 'end of
> stream' ?

[Zlice Z]

Hi,

I see it on the
https://wiki.winehq.org/Developer_Hints#Using_only_C89-compliant_code page.
It mentions variables being earlier, but that seemed to slow things down in
this instance.

Some comments I left in because I'm not sure if they're the right thing to
do. Will convert.

On Fri, May 20, 2022 at 3:46 AM Bernhard Kölbl  wrote:
> Hi Zlice,
>
> thanks for your patch. Unfortunately you're missing out on quite a few
> requirements for this to get accepted. I'd recommend to refactor your
> submission using this guide as reference:
> https://wiki.winehq.org/Submitting_Patches and to resend it afterwards.
>
> E.g. you left in C++ comments, which aren't allowed.
>
> Thanks,
> Bernhard
>
> Zlice Z  schrieb am Fr., 20. Mai 2022, 09:22:
> > Sent a patch yesterday that was mediocre and barely helps.
> >
> > commit
> > https://github.com/wine-mirror/wine/commit/5144b27661fcd6705353d832e0383085f...
> >
> > removed flushing from parser, which after commit bisecting turns out to
> > be the cause of Fallout 3 radio chugging every new song.
> > - re-introduced parser flushing (self explanatory, commit said it's not
> >
> > worth it but it helps here)
> > - moved some variable initialization in wm_reader.c to after initial
> >
> > conditional returns. these were in a switch-case before in the middle of
> > the function and it seems to be part of the stuttering, wasn't moved in the
> > mentioned commit though
> > - commented out and left my name by some EOS sets. they used to be
> >
> > checked for EOS 'events' but those references have been changed to 'buffer'
> > and idk what the right action is. wine seemed to randomly seg fault on exit
> > with leaving them on though. suspect they incorrectly guess and set 'end of
> > stream' ?

[Zebediah Figura]

On 5/19/22 15:30, Zlice Z wrote:
> Sent a patch yesterday that was mediocre and barely helps.
>
> commit
> https://github.com/wine-mirror/wine/commit/5144b27661fcd6705353d832e0383085f...
>
> removed flushing from parser, which after commit bisecting turns out to be
> the cause of Fallout 3 radio chugging every new song.
> - re-introduced parser flushing (self explanatory, commit said it's not
>
> worth it but it helps here)
> - moved some variable initialization in wm_reader.c to after initial
>
> conditional returns. these were in a switch-case before in the middle of
> the function and it seems to be part of the stuttering, wasn't moved in the
> mentioned commit though
> - commented out and left my name by some EOS sets. they used to be checked
>
> for EOS 'events' but those references have been changed to 'buffer' and idk
> what the right action is. wine seemed to randomly seg fault on exit with
> leaving them on though. suspect they incorrectly guess and set 'end of
> stream' ?

I don't think "reintroduce flushing" is the right answer here, not 
without understanding why it matters.

The point of 5144b2766 is that flushing should not make a difference. It 
allows wg_parser_stream_get_event() to return more quickly, but that 
same cost is added to the subsequent seek or stop request, so 
GST_Seeking_SetPositions() or parser_cleanup_stream() will end up taking 
just as long.

If flushing does make a difference, I think we need to understand why, 
and quite likely solve this a different way.