Contributors Request #4
Comments
|
@low-batt I really appreciate your PRs with very detailed explanation and most of them have been cherry-picked into this repo. Would you like to join this group? |
|
I tried forking iina-plus/iina today as I like the ability to look over your submission on GitHub the PR workflow gives you before you push it into the public repository. Yes I'm paranoid about accidentally messing up and pushing something I didn't intend. I found I couldn't fork ina-plus/iina. Seemed like this is a restriction on GitHub free accounts. So yes, if you grant me access I will add my changes to this fork as well. What is your vision for coordinating changes given this setup? |
|
I have a local commit with the fix for the camera housing issue but: |
|
Let me have a check |
|
Try again please? |
|
Worked! |
|
Some commits you made had conflict with my repo. I skipped them. You may have a check. |
|
I will check into it and where appropriate adjust the commits and get them pushed into this fork. |
|
Pushed a commit with the fix for iina#3574 |
|
I have just submitted a pull request with a fix for issue iina#3475. I was unable to reproduce the crash, so I can't confirm it will fix the crashes. Do you want to wait for the core developers to review the fix, or do you want me to push it to iina-plus? |
|
iina-plus is a repo that experiments new features and bug fixes that haven't been officially accepted by upstream. It's ok to push your changes directly into iina-plus repo and we may revert it later if necessary. Let's release a new binary for users to test the changes. |
|
Pushed fix for iina#3475 |
|
Pushed fix for iina#3584 |
|
Pushed fix for iina#3590 Going to have to keep an eye on this one. The fix is correct and needed. But because IINA is now waiting for mpv to shutdown we may find issues in the way mpv is being shutdown that usually were hidden by premature termination of the application. I sometimes see log messages from mpv that makes me think they might have some bugs in their quit command. |
|
Pushed fix for iina#3551 |
|
Pushed fix for iina#3593 This fixes an existing issue in the shutdown sequence that was made more obvious by the fix to properly wait for mpv to terminate. |
|
Thanks. If your PR gets merged, I'd like to rebase everything on top of the official branch. Please don't be surprised if you get any conflicts. |
|
Pushed a fix to the fix for iina#3558 |
|
Pushed a commit to update the copyright year to 2022 |
|
Pushed fix for iina#3601 |
|
Pushed a tiny fix for iina#3607 |
|
Pushed small fix for iina#3211 |
|
Just rebased everything onto the official repo |
|
Pushed a revision to the fix for iina#3558 |
|
That is beyond my knowledge of git. How would I do that? |
|
Just remove the old commit using Force-pushing is needed. It's dangerous but necessary to maintain a fork repo |
|
Pushed fix for iina#3692 |
|
Pushed fix for iina#3695 |
|
With the latest upstream changes, I got conflicts when rebasing. Can you please resolve it? |
|
Yes, I will take care of rebasing, probably later today. Reviewing and merging is currently happening in the main IINA repository. I'm about to respond to review feedback. |
|
Pushed commit that adds OSD messages for file loop and playlist loop, iina#3229 There is other cleanup that might be needed for IINA+. There are commits that were added for debugging when trying to figure out the hang that turned out to be an issue with hardware drivers and FLAC encoded files. I will check over the commits and get IINA+ aligned with the official repository. |
|
Resolved rebase conflicts. Up to date with upstream for now. |
|
Pushed fix for iina#3462 |
|
More commits have been merged, so IINA+ is behind again. Likely these will trigger merge conflicts as well. |
|
Starting rebase again. May take a while as I expect merge conflicts with my changes. @CarterLi Did you notice the request that you post a HDR PR in the official repository? |
|
Completed rebase. |
|
Starting rebase with IINA again. |
|
Completed rebase with IINA. |
|
Did a major rebase |
|
I pulled the latest IINA+ and rebuilt. Starting with both I also experienced another inconsistency. On another topic, I am actively working on the issues with the display refresh rate matching feature. |
|
Starting rebase with IINA again. |
|
Completed rebase with IINA. |
|
Rebased with IINA again. |
|
It's hard to maintain a fork. It's even harder to maintain an Xcode project fork with a lot of IDE-generated code. The upstream commit iina@d37734b introduced major conflicts against this repo. I have tried to resolve the conflicts and I failed. And even if I manage resolving the conflicts, the upstream branch https://github.com/iina/iina/tree/plugin-system will introduce much more conflicts and I don't think I can resolve them. As the major feature, HDR playback, has been merged into upstream, I truly doubt if it's worth maintaining this fork... |
|
Pushed another fix for iina#3590 This fix is compatible with the previous fix. The new fix addresses crashes in the logger directly. |
|
No. There are a lot of conflicts between my repo and official one. I don't bother resolve them. |
|
How do we get those issues fixed in the official repo? Do you want me to try and figure out what changes are needed? |
|
All the issues you mentioned above were linked with my changes. All HDR related commits were prefixed with |
|
Found the commits. I will get those two issues fixed in the main repo. I've not had time to try a rebase and see how bad the conflicts are. I can easily deal with conflicts in the changes I've made. I've been worried all those changes for the plugin system would prove challenging to deal with. I've not had a big block of time to look into this. |
|
I've just upgraded to the publicly released Ventura and that strange dimming that only affects HDR in full screen mode is still there. This is issue iina#3880 reported against the Ventura Beta that we decided must be a macOS defect. After reproducing it and looking into it I don't see how it can be anything other than a defect. But Apple has let it out to the public and more users are now reporting it. This is not slight dimming. In the test I'm running the picture is nearly black. Definitely unwatchable. I just tested a truly horrible awful hackaround. If I add a tiny (0.1 x 0.1) nearly opaque (0.01) view as a subview then the screen does not dim. I really hate this workaround. Before proposing such an awful workaround I wanted to check in and see if you had any other ideas about this problem or better ways to workaround it. |
|
I haven't had time to look into it. Will investigate it later |
|
No. I don't like the hack. |
|
Yes, it is a horrible hackaround. I will not put it in IINA+. Ventura definitely has some odd graphics defects. It has been reported that 13.0.1 is no better. Some of these problems have been reproduced without IINA, so it is definitely macOS that is at fault. |
As IINA is my favorite macOS player, I really hope that IINA can go further. However the official repo seemed to be no longer in active development ( but not dead, hopefully ).
Since then, many features, fixes were made in various personal fork repo. However because they were separated in different repos:
So I decided to create this iina-plus group and want it to be open and easy to contribute ( not open PR, but join the group and push code directly into the repo )
However, to prevent from spam, there is a requirement to prove you have the ability to contribute.
The text was updated successfully, but these errors were encountered: