On Fri, 22 Jul 2016, at 03:24 PM, Arun Raghavan wrote: > On Fri, 22 Jul 2016, at 02:45 PM, Arun Raghavan wrote: > > > > > > On Thu, 30 Jun 2016, at 02:43 AM, Ulrich Eckhardt wrote: > > > Pierre Ossman wrote: > > > > Triggering the bug requires a specific amount of data to be drained, > > > > which is difficult to achieve in any sensible manner just by being a > > > > client. :/ > > > > > > > > I added the attached test though. It doesn't test the full scope of > > > > the bug as it doesn't include the native protocol side of things, but > > > > it should verify correct minreq behaviour in the core (which in turn > > > > should avoid bugs further out). > > > > > > Hi! > > > > > > I recently tried to look at this report, too, and couldn't quite > > > understand what initially caused the issue, so I started writing tests > > > for various aspects of the memblockq code. The results so far are > > > available at https://github.com/UlrichEckhardt/pulseaudio in the > > > feature/pa_memblockq_tests branch, in case anyone wants to take a look > > > or pull them. > > > > > > There is one commit ("Add a test for missing data behaviour") there, > > > which shows an oddity in the behaviour already: If you exceed the > > > target length and then drain some data, these drained bytes are > > > reported as missing via pop_missing(), even if the current level still > > > exceeds the target length, which seems wrong. > > > > > > I also tried your attached testcase (had to adjust it slightly) and > > > could also reproduce faults with it. For convenience I uploaded it to > > > the feature/pa_memblockq_pop_missing branch there. > > > > Thanks for this, and I'll take a look at the other tests you added as > > well. I'm pushing your patch reattributed to Pierre (since the majority > > of code in there is still his), and with attribution to you in the > > comment. > > I think those tests are something we should definitely add. I've pushed > all but the last which, as you pointed out, fails. Will take a look at > that now. With Pierre's patches, there is no "static" vs. "dynamic" missing any more. Both pa_memblockq_missing() and pa_memblockq_pop_missing() are consistent. I'm fixing up your test to reflect that and pushing it. -- Arun
