| dvratil | |
| sitter |
| Restricted Owners Package | (Owns No Changed Paths) |
The problems presents in latest gstreamer, due to some changes i guess, how to reproduce
CCBUG: 396256
It's not tested, i propagate that can be a fix.
| Automatic diff as part of commit; lint not applicable. |
| Automatic diff as part of commit; unit tests not applicable. |
from IRC
[14:20] <dvratil> sitter: I've seen the review, but I currently don't have any means to test it
I myself am on an ancient gstreamer, so I can't really reproduce the original issue either. With the patch applied I still can stream though, so I suppose it's at least not regressing on older gstreamers.
@dvratil maybe you could comment on the code in of itself? Could always land it and have it implicitly tested by people using kdesrc-build or whatever.
The change looks good to me codewise. I haven't tested it, but it does exactly what the documentation says. Just increase the timeout, please.
| gstreamer/pipeline.cpp | ||
|---|---|---|
| 255 | Docs say that GstClockTime is measured in nanoseconds. I would propose a timeout of at least 10ms here since the operation is asynchronous and I doubt much is accomplished in 1 µs. | |
I believe you the issue is real, all I was saying is that it still works on ancient libgstreamer as I have no recent one to test with :)
Anyway, I've updated the test plan with steps to do.
That's change in 1.14 i think, so right after state goes to ready on stream gstreamer triggers signal sync-message::buffering with percentage 0 that's cb_buffering on other hand we change immediately state to paused in, so gstreamer starts buffering again and when goes to 100% we end up with playing state.
@sitter @dvratil we should make a new tag on gstreamer backend because now phonon users cannot stops online streams at all.
Well, I can't make a release if you don't land the commit?
| gstreamer/pipeline.cpp | ||
|---|---|---|
| 329 | I think our code style is if (percent == 0) { | |
I'll commit it as soon you accept.
| gstreamer/pipeline.cpp | ||
|---|---|---|
| 329 | Yes, but it's related to other code style in the file. | |
Well, dvratil is maintainer you don't need me to accept really, I'll just defer to him anyway :P