GNOME Bugzilla – Bug 758328
[Patch]: initial progressbar beeps
Last modified: 2018-02-08 13:12:00 UTC
Created attachment 315878 [details] [review] Initial progressbar beeps Howdy, Here the current state of the orca progressbar beeps based on the current master. cheers chrys
Thanks! I'll take a look later today. In the meantime, we'll need to put a header on it. I will take care of that, but I need your full name and email address as you want it to appear for "author". What we've been doing is ascribing copyright to "The Orca Team" unless there is a company you need us to ascribe it to instead. For instance, my employer Igalia donates my time to work on Orca. If you don't have an employer who wants the copyright, please confirm that the generic "The Orca Team" is ok with you. Thanks!
(In reply to Joanmarie Diggs (IRC: joanie) from comment #1) > Thanks! I'll take a look later today. In the meantime, we'll need to put a > header on it. I will take care of that, but I need your full name and email > address as you want it to appear for "author". Full name: Christian Hempfling E-Mail: mail AT chrys.de > > What we've been doing is ascribing copyright to "The Orca Team" unless there > is a company you need us to ascribe it to instead. For instance, my employer > Igalia donates my time to work on Orca. If you don't have an employer who > wants the copyright, please confirm that the generic "The Orca Team" is ok > with you. yay :), I m part of the "Orca Team" -> confirmed ;) I have not company that donates my time for orca. Its just a hobby. > > Thanks! Coud i do something better in creating a patch? I i could please give me feedback. I like to do more things in future for orca.
Chris, you can use the git format-patch command to produce patch files that include appropriate headers for git to attribute the commit to you. Just make sure your details given earlier in this bug are in the author line of the commit, i.e full name <email address>. You can edit commit details with git commit --amend, adding the --author argument to specify the commit author. Hope that helps.
Created attachment 315963 [details] patch created with git format-patch (rebased on the most current master)
Review of attachment 315878 [details] [review]: Obsolete
is this better?
I'm in the process of rewriting the existing progress bar code (i.e. what's already in master). Reviewing your patch reminded me of how old and scary that code is. I'm getting nearly done. After that, let's talk about incorporating your new feature. :)
Ah OK :). I see that orca have speech-/braile_generator.py script. what do you think about to keep this name pattern and i will rename "sound-utils" to "sound_generator" to be more firm wiht the names? I will take a look at your work later today and will change my code to the master. (hopefully more clean :) ). What do you think about gstreamer beeing a depency of orca? should I keep the sound stuff optional? I think behind the sound stuff is many potential, also for to other thinks (maybe sound Icons?). I also like to add a Volume for the Sound, this makes really sense.
Created attachment 317164 [details] Updated patch to the most current master changelog: - rebased to the most current master - seperate utils (sound_generator.py) from sound generator (sound.py) new settings but wihtout GUI for now - add setting for changeing the volume (soundVolume Float 0..1) - add setting for global enable/disable sound (enableSound Boolean)
Created attachment 317165 [details] [review] Updated patch to the most current master Cleanup spaces ;)
Created attachment 317166 [details] [review] Updated patch to the most current master Remove some empty lines (sorry for the noise... )
Created attachment 317167 [details] [review] Updated patch to the most current master Removed an Unneeded field "volume" that may lead to problems while reloading the settings.
Created attachment 317204 [details] [review] Patch Add posiblity to play sound sequences. could be used for "sound" icons
Created attachment 317240 [details] [review] Pathc Bugfix
Created attachment 317248 [details] [review] Patch add plausibility to createTone
Created attachment 317251 [details] [review] Patch Add Pan to soundgenerator for later usage
Comment on attachment 317251 [details] [review] Patch So as I announced on the list, we're nearly there. In order for you to test and debug the beeps, you'll need another patch from me. But I'm still debugging something related to this and about to go into a meeting. So I'll fill you in more later today/tonight. Sorry and thanks! Here's the commit btw: https://git.gnome.org/browse/orca/commit/?id=ef3b4cbf3e
OK, progress bar beeps are now working for me and hooked up in master. Please let me know if the experience matches what you did in your patch. If I got something wrong, lemme know and I'll see if I can fix it. If you want to tweak the nature of the tones, you can attach patches here. Thanks!
Created attachment 321548 [details] [review] Limit frequencies to an octave This proposal fixes the frequency bounds for progress bar beeps to middle C and an octave higher. I don't particularly care whether we stick with 1 octave or 8, where the octave starts, etc. When I ran the numbers of the current implementation, the frequency range ran from something like 200 Hz to 2 KHz. This makes it difficult to intuitively know where a progress bar begins or ends. Were this a visual progress bar, it'd be an amorphous line with no clearly-defined start or end. This patch essentially draws a border around the progress bar, starting it at one note and progressing up an octave. I don't particularly care about the extents of the borders, I just think they should make sense and not be arbitrarily-chosen numbers of no significance. :) We shouldn't just punch in numbers and get something that kinda sorta sounds somewhat OK. Instead, the beeps should be fairly easy to hear and say "Ah, I think that's around 70% done because it's 70% of the way through the octave."
(In reply to Nolan Darilek from comment #19) > Created attachment 321548 [details] [review] [review] > Limit frequencies to an octave > > This proposal fixes the frequency bounds for progress bar beeps to middle C > and an octave higher. > > I don't particularly care whether we stick with 1 octave or 8, where the > octave starts, etc. When I ran the numbers of the current implementation, > the frequency range ran from something like 200 Hz to 2 KHz. This makes it > difficult to intuitively know where a progress bar begins or ends. Were this > a visual progress bar, it'd be an amorphous line with no clearly-defined > start or end. > > This patch essentially draws a border around the progress bar, starting it > at one note and progressing up an octave. I don't particularly care about > the extents of the borders, I just think they should make sense and not be > arbitrarily-chosen numbers of no significance. :) We shouldn't just punch in > numbers and get something that kinda sorta sounds somewhat OK. Instead, the > beeps should be fairly easy to hear and say "Ah, I think that's around 70% > done because it's 70% of the way through the octave." beside that your code doesn't work caused by an typo, there is not any difference hearable in your stuff... why sending a patch about this while not waiting for answers at the mailinglist?
(In reply to chris from comment #20) > why sending a patch about this while not waiting for answers at the > mailinglist? Perhaps because I asked him to? See the mailing list. I'm very much in favor of discussion on list. But when people have patches to try, I like them gathered on a bug so one doesn't have to go searching through mail archives.
Right, doing this per request. Also, not trying to be hostile, I just don't like how NVDA implements this feature and thought I'd suggest an alternative. Anyhow, I've given up too much of my workday to this already. A more helpful code review would point out the specific nature of my typo. As it stands, I appear to be using this patch just fine, though admittedly my internet is slow and I haven't been able to download something to put a progress bar through its full paces. I haven't used Python in a while and tried to look up a quick way to have the interpreter do a syntax check, but couldn't find anything and now have a bus to catch.
Created attachment 330508 [details] [review] Adjust duration of beeps Users reporting me that the duration is to high. shorter is better because the beep doesnt interrupt live so long :) here I m correct this.
Created attachment 330509 [details] [review] Ignore Interval for Beeps Most people i talk with, wont have the interval for beeps because beeps need no "speech time". they could do the work without interruption from the beeps.in practice the voice or braille announcement is also enabled with a much higher interval to make the percentage concrete.
Comment on attachment 330509 [details] [review] Ignore Interval for Beeps Thanks for the patch. But I'm afraid I think ignoring the values is a bad idea. I think making them individually configurable makes a lot more sense. So that's what I've done. I hope it addresses your concerns.
Review of attachment 321548 [details] [review]: Hey Nolan. Since everyone is feeling all progressbar beepy again, I've gone back and forth trying your patch versus what we currently have. I will no doubt hear progress bars beeping at me in my dreams. Anyway.... What I like about your patch is there are no mystery numbers like 5.4 and 22. And the beeps at the beginning seem more reliable. However, I'm finding it hard to hear the beeps at the end because of the decrease in volume (which you're inheriting from the existing code). If you want to sort out how best to deal with that and give me an updated patch, that would be great. Thanks!