androidx
session: fix a deadlock due to onPlayRequested()
Hi @marcbaechinger, it's only been 3 weeks since the last ping (sorry if I am bothering you) but considering you are active rn, maybe you could add this PR to your review queue? thanks alot :)
Hi @marcbaechinger, sorry for repeatedly pinging, could I convince you to review this PR? I've shipped it in production for about two months already with only one minor issue (which I have already fixed). It's been working pretty well. Would be appreciated!
Thanks for your contributions! To help me review your pull requests more effectively, I'd like to outline what I'll be looking for in each submission:
- A unit test that reproduces the bug you are fixing. This is crucial for me to understand the problem and verify the solution.
- A brief explanation of the reasoning behind your design choices.
- Code that is consistent with the existing Media3 coding style.
My review will also verify that the changes are efficient, work correctly with older versions of Media3 and platform APIs, and that any API changes include a backward-compatible migration path. While it's great that you are already using these changes in your app, we still need this comprehensive test coverage to proceed with our internal review process.
I plan to start looking at this PR this week or next. Providing the information above will make the review process much smoother and help me reach a decision on your contribution more quickly.
Hi @marcbaechinger, sorry for flooding you with PRs :) I am not a professional so please excuse any stupid questions:
A unit test that reproduces the bug you are fixing. This is crucial for me to understand the problem and verify the solution.
Sure, I'll add a unit test to this PR and #2626.
For the other PRs assigned to you (#2642, #2643, #2647), they only make things faster. I'm not sure how to add a unit test there. Do you want me to add some to these too? If so, could you please give me a small hint how those are supposed to work?
A brief explanation of the reasoning behind your design choices.
No problem, I'll do that.
Code that is consistent with the existing Media3 coding style.
If you mean this: https://github.com/androidx/media/blob/release/CONTRIBUTING.md#code-style I generally try to format my PRs with google-java-format before uploading. I'll make sure I haven't forgotten it in any of the PRs.
If you're talking about coding style on a more general level (word choice in variable names, methods used for switching threads, etc), I'm trying my best to adapt to the code around me, but please do tell me if I do anything wrong. :)
Never mind the point regarding code style. I haven't meant all these point are not met by your code and I can imagine it's not always easy to know what it needs. This is just a general statement around what I have to do for making a PR ready for internal code review and everything that deviates from that means that it may take a certain time to adjust it.
I've added a unit test to my PR that fails before my fix and passes after my fix. For brief explaination of the fix, see: https://github.com/androidx/media/issues/1197#issuecomment-2751594360