Closed Bug 1177514 Opened 4 years ago Closed 4 years ago

Final text of SpeechRecognitionResult::transcript is "ERROR" if there is an error in recognition

Categories

(Core :: Web Speech, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla42
Tracking Status
firefox42 --- fixed

People

(Reporter: kdavis, Assigned: kdavis)

References

Details

(Whiteboard: [webspeechapi][systemsfe])

Attachments

(1 file, 1 obsolete file)

Final text of SpeechRecognitionResult::transcript is "ERROR" if there is an error in recognition 

See DecodeTask::Run() around line 105
Assignee: nobody → kdavis
Whiteboard: [webspeechapi][systemsfe]
Depends on: 1185235
The depends on Bug 185235 was added simply due to the possibility of merge
conflicts if this bug is landed first.
(In reply to kdavis from comment #1)
> The depends on Bug 185235 was added simply due to the possibility of merge
> conflicts if this bug is landed first.

This was a typo the depends is on Bug 1185235 not the bug in the above comment
which is missing the leading '1'.
Part 1 of 1: Removed final text of 'ERROR' on recognition error, should be signaled by SpeechRecognitionError

Part 1 of 1 for this bug.

The Pocketsphinx WebSpeech API implementation raised an error condition
by setting the SpeechRecognitionResult::transcript to the text "ERROR".
This is problematic in that:

1. It is not the manner specified by the WebSpeech API to raise errors. The 
   official mechanism[1] is through the EventHandler that is assigned to the 
   SpeechRecognition::onerror attribute.
2. In some languages it may not be clear if the transcript "ERROR" indicates 
   an error condition or is in fact a valid transcript.

This commit simply does not set the SpeechRecognitionResult::transcript to
the text "ERROR" when an error is raised and defers the raising of errors
to the EventHandler SpeechRecognition::onerror.

The try for this patch is here[2]

[1] https://dvcs.w3.org/hg/speech-api/raw-file/tip/webspeechapi.html#dfn-onerror
[2] https://treeherder.mozilla.org/#/jobs?repo=try&revision=c41a60c74793
Attachment #8638571 - Flags: review?(bugs)
Comment on attachment 8638571 [details] [diff] [review]
Part 1 of 1: Removed final text of 'ERROR' on recognition error, should be signaled by SpeechRecognitionError

>--- a/dom/media/webspeech/recognition/PocketSphinxSpeechRecognitionService.cpp
>+++ b/dom/media/webspeech/recognition/PocketSphinxSpeechRecognitionService.cpp
>@@ -106,9 +106,7 @@ public:
>     rv = ps_end_utt(mPs);
>     if (rv >= 0) {
>       hyp = ps_get_hyp(mPs, &score);
>-      if (hyp == nullptr) {
>-        hypoValue.Assign("ERROR");
>-      } else {
>+      if (hyp != nullptr) {
You could just drop != nullptr here and have
if (hyp) {
  hypoValue.Assign(hyp);
}
Attachment #8638571 - Flags: review?(bugs) → review+
(In reply to Olli Pettay [:smaug] from comment #4)
> Comment on attachment 8638571 [details] [diff] [review]
> Part 1 of 1: Removed final text of 'ERROR' on recognition error, should be
> signaled by SpeechRecognitionError
> 
> >--- a/dom/media/webspeech/recognition/PocketSphinxSpeechRecognitionService.cpp
> >+++ b/dom/media/webspeech/recognition/PocketSphinxSpeechRecognitionService.cpp
> >@@ -106,9 +106,7 @@ public:
> >     rv = ps_end_utt(mPs);
> >     if (rv >= 0) {
> >       hyp = ps_get_hyp(mPs, &score);
> >-      if (hyp == nullptr) {
> >-        hypoValue.Assign("ERROR");
> >-      } else {
> >+      if (hyp != nullptr) {
> You could just drop != nullptr here and have
> if (hyp) {
>   hypoValue.Assign(hyp);
> }

Will do.
Part 1 of 1: Removed final text of 'ERROR' on recognition error, should be signaled by SpeechRecognitionError

Part 1 of 1 for this bug.

This patch makes no functional changes. It only makes the single
change suggested in Comment 4, replacing

if (hyp != nullptr) {
...
}

with

if (hyp) {
...
}

The try for this patch is here https://treeherder.mozilla.org/#/jobs?repo=try&revision=a76076a11d6a

The interdiff between this patch and the last patch is as follows

diff -u b/dom/media/webspeech/recognition/PocketSphinxSpeechRecognitionService.cpp b/dom/media/webspeech/recognition/PocketSphinxSpeechRecognitionService.cpp
--- b/dom/media/webspeech/recognition/PocketSphinxSpeechRecognitionService.cpp
+++ b/dom/media/webspeech/recognition/PocketSphinxSpeechRecognitionService.cpp
@@ -106,7 +106,7 @@
     rv = ps_end_utt(mPs);
     if (rv >= 0) {
       hyp = ps_get_hyp(mPs, &score);
-      if (hyp != nullptr) {
+      if (hyp) {
         hypoValue.Assign(hyp);
       }
     }
Attachment #8638571 - Attachment is obsolete: true
Attachment #8639770 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/726366fd7c18
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
You need to log in before you can comment on or make changes to this bug.