After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 167810 - dectalk driver may report wrong markers
dectalk driver may report wrong markers
Status: RESOLVED FIXED
Product: gnome-speech
Classification: Deprecated
Component: drivers
unspecified
Other Linux
: Normal normal
: ---
Assigned To: Oana Serb
Oana Serb
AP0
Depends on:
Blocks:
 
 
Reported: 2005-02-18 14:35 UTC by remus draica
Modified: 2005-03-22 09:49 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
proposed patch (3.80 KB, patch)
2005-02-18 14:35 UTC, remus draica
none Details | Review
reworked patch (4.42 KB, patch)
2005-03-03 14:44 UTC, remus draica
none Details | Review
proposed patch (4.68 KB, patch)
2005-03-18 09:38 UTC, Oana Serb
none Details | Review
reworked patch (4.65 KB, patch)
2005-03-18 11:12 UTC, Oana Serb
needs-work Details | Review
reworked patch (4.82 KB, patch)
2005-03-18 14:28 UTC, Oana Serb
needs-work Details | Review
reworked patch (5.10 KB, patch)
2005-03-21 13:41 UTC, Oana Serb
accepted-commit_now Details | Review

Description remus draica 2005-02-18 14:35:19 UTC
When large pieces of text are sent to dectalk tts, the tts callback is not
called for all markers. Because of that not all markers in the
index_marker_queue are presented by the driver.

All markers before the current one in that queue shoud be presented in the callback.
Comment 1 remus draica 2005-02-18 14:35:54 UTC
Created attachment 37644 [details] [review]
proposed patch
Comment 2 remus draica 2005-03-03 14:44:54 UTC
Created attachment 38204 [details] [review]
reworked patch
Comment 3 Oana Serb 2005-03-18 09:38:17 UTC
Created attachment 38885 [details] [review]
proposed patch

Strings with length 0 can be sent to DECtalk and this is not good.
The patch solves this problem.
Comment 4 Oana Serb 2005-03-18 11:12:49 UTC
Created attachment 38892 [details] [review]
reworked patch
Comment 5 bill.haneman 2005-03-18 13:41:12 UTC
Comment on attachment 38892 [details] [review]
reworked patch

I'm a little concerned about the use of g_ascii_isspace(); can we use something
UTF-8-safe here?
Otherwise patch looks good.
Please remember when replacing patches, to mark the old ones obsolete.	Thanks
Comment 6 bill.haneman 2005-03-18 13:43:56 UTC
Comment on attachment 38892 [details] [review]
reworked patch

please replace usage of g_ascii_isspace with g_unichar_isspace (you'll have to
use g_utf8_get_char() to obtain the unichar to check).
Comment 7 Oana Serb 2005-03-18 14:28:01 UTC
Created attachment 38902 [details] [review]
reworked patch

g_ascii_isspace() was used before I made the changes.
Comment 8 bill.haneman 2005-03-18 15:40:44 UTC
Comment on attachment 38902 [details] [review]
reworked patch

the problem with the text handling where you look for spaces is that you are 
using "++c" instead of properly incrementing the UTF-8 character position. 
This will fail for non-ASCII text strings!  Of course, this problem has been
there before you started submitting this patch, but it does need to be
fixed.... thanks!
Comment 9 Oana Serb 2005-03-21 13:41:10 UTC
Created attachment 39006 [details] [review]
reworked patch
Comment 10 bill.haneman 2005-03-21 13:52:54 UTC
Comment on attachment 39006 [details] [review]
reworked patch

...
> 		/* Add index between words */
> 		
> 		while (*beginning) {
> 			end = beginning;
>-			while (*end && !g_ascii_isspace(*end))
>-				end++;

Oana:  I don't understand why the first 'while' below is needed now, and wasn't
needed before.	It seems to subtly alter the logic.  Otherwise this looks good,
thanks!

>+			while (*end && g_unichar_isspace(g_utf8_get_char (end)))
>+				end = beginning = g_utf8_next_char (end);
>+			while (*end && !g_unichar_isspace(g_utf8_get_char (end)))
>+				end = g_utf8_next_char (end);
> 			word = g_strndup (beginning, end-beginning);
> 			dectalk_add_string (d, word);
> 			g_free (word);
> 			dectalk_synthesis_driver_add_index(d,
> 						   speaker->cb,
> 						   GNOME_Speech_speech_callback_speech_progress,
>-						   text_id, end-text);
>+						   text_id, end-text, marker_id++);
> 			if (!*end)
> 				break;
>-			beginning = end+1;
>+			beginning = g_utf8_next_char (end);
> 		}
Comment 11 Oana Serb 2005-03-21 14:35:52 UTC
If the string text sent to be spoken by DEctalk contains many spaces one
followed by another then strings with length 0 are sent to DECtalk.
For example, if the string "1     2" is send to be spoken, then the following
strings are sent to DECtalk:

"1"
""
""
""
""
""
"2"
(I used quotation marks just to see better the strings sent to DECtalk)

If many strings with length 0 are sent (one after another), DECtalk crashes. I
seen this behavior in 'flat review mode'.

We found that this problem should be fixed, too.
Comment 12 bill.haneman 2005-03-21 15:10:43 UTC
Comment on attachment 39006 [details] [review]
reworked patch

Please apply, after thorough testing.
Comment 13 Oana Serb 2005-03-22 09:49:03 UTC
Comment on attachment 39006 [details] [review]
reworked patch

Patch committed to head.