marqs85 / ossc

Open Source Scan Converter
http://junkerhq.net/xrgb/index.php/OSSC
GNU General Public License v3.0
480 stars 65 forks source link

Two-way profile linking #23

Closed megari closed 6 years ago

megari commented 6 years ago

Inspired by the discussion in Issue #22 and the work on this feature in borti4938's branch, I created something minimal which hopefully fulfils the needs stated in #22 . I am a bit worried that I may have overlooked something despite having thought about this quite a bit, so please take a look.

megari commented 6 years ago

Whoops, forgot to push the actual "meat" of the changes. Please stand by.

marqs85 commented 6 years ago

Profile->input and Input->profile should activate only when profile or input is selected manually (with "initial input" as possible exception). The former works correctly in current implementation, but with latter you may run into a situation where loading profile X results to profile Y getting loaded. That could be easily fixed by setting a flag in load_profile() which disables subsequent profile loading due to input change.

megari commented 6 years ago

@marqs85 Ah, indeed. I thought I had prevented profile loads from inducing subsequent profile loads, but apparently that was not the case. Will push some fixes soon.

borti4938 commented 6 years ago

Profile->input and Input->profile should activate only when profile or input is selected manually (with "initial input" as possible exception). The former works correctly in current implementation, but with latter you may run into a situation where loading profile X results to profile Y getting loaded.

I don't see how this may happen as he does not go into load_profile() on input->profile link. What do I oversee, here?

megari commented 6 years ago

@borti4938 it goes like this:

  1. load_profile(X) is called, so profile X is loaded and target_input is changed to, say, A.
  2. If profile_link is true and input_profile[A] is Y, profile Y is loaded on line 861.

This, of course, requires the input->profile and profile->input links to be inconsistent.

megari commented 6 years ago

Pushed some changes that should fix the issues pointed to me above. Only compile-tested, though.

megari commented 6 years ago

I now reverted the changes to avinput_t. I don't see any reason for the change in the context of my version, anyway. I will try to test this properly on the device tomorrow.

megari commented 6 years ago

I may want to rebase this branch at some point, given the large number of fixups.

borti4938 commented 6 years ago

Go ahead for a rebase. Sounds good!

megari commented 6 years ago

Rebased the fixups and reverts away. Preserved the actual development activity, though.

megari commented 6 years ago

Hmm, I don't like it how little space we have left after these changes for the audio-enabled firmware:

Info: (sys_controller.elf) 36 KBytes program size (code + initialized data).
Info:                      1156 Bytes free for stack + heap.

Will see if there are places where I can reclaim some of that space.

borti4938 commented 6 years ago

you can save some code space if you update global and input->prof. link simultaneously in load_profile() and save_profile(). If I don't oversee something, this does not break any functionality... https://github.com/borti4938/ossc/commit/882d8454b3940f1345fd370dc1e9110f3919caa9

marqs85 commented 6 years ago

Last time I checked, the stack topped around 700B, so it's definitely getting tight. It's still probably not worth spending too much time cutting away a few bytes here and there - I'll be looking into integrating Pulpino (RISC-V core supporting compressed ISA) into the project in near future which might be a short-term solution to the code space issue.