sillsdev / libpalaso

Palaso Library: A set of .Net libraries useful for developers of Language Software.
MIT License
44 stars 51 forks source link

add audio monitoring without exception catching #1280

Closed nabalone closed 1 year ago

nabalone commented 1 year ago

For Bloom https://issues.bloomlibrary.org/youtrack/issue/BL-12406/Dont-show-error-Dialog-if-mic-access-is-off Create a version of BeginMonitoring that doesn't catch the exceptions so that they can be handled by the caller


This change is Reviewable

tombogle commented 1 year ago

SIL.Media/Naudio/AudioRecorder.cs line 226 at r1 (raw file):

          bool monitoringStarted = false;
          monitoringStarted = BeginMonitoringIfNeeded();
          if (!monitoringStarted)

To avoid duplication of code and prevent a scenario where a future improvement to the message might cause the two versions to get out of sync, maybe better to factor out this code to throw the exception. OR, have a single implementation method that takes a reportExceptionToUser parameter and in the catch blow merely re-throws the caught exception if !reportExceptiontoUser.

tombogle commented 1 year ago

SIL.Media/Naudio/AudioRecorder.cs line 177 at r2 (raw file):

      /// ------------------------------------------------------------------------------------
      /// <summary>
      /// For the IAudioRecorder interface. See BeginMonitoring(bool catchAndReportException)

You can use the tag to turn that reference into a clickable link. That will also ensure enable the signature to be checked and kept up to date if there is a future change.

tombogle commented 1 year ago

SIL.Media/Naudio/AudioRecorder.cs line 181 at r2 (raw file):

      /// <remarks>
      /// Methods in this class should not call this method (i.e., from inside code that has
      /// a lock on this). Instead use the private internal version.</remarks>

It looks like the other version is also public.

tombogle commented 1 year ago

SIL.Media/Naudio/AudioRecorder.cs line 210 at r2 (raw file):

      /// and reported via <see cref="ErrorReport.NotifyUserOfProblem(string,object[])"/>.
      /// </param>
      /// <remarks> Methods in this class should not call this method (i.e., from inside

Looks like this remarks comment is not relevant here.

nabalone commented 1 year ago

The remarks comment Methods in this class should not call this method (i.e., from inside code that has a lock on this). Instead use the private internal version. was on the original BeginMonitoring method. Could it have referred to the private BeginMonitoringIfNeeded method? or is it just out of date?

tombogle commented 1 year ago

SIL.Media/Naudio/AudioRecorder.cs line 177 at r2 (raw file):

Previously, tombogle (Tom Bogle) wrote…
You can use the tag to turn that reference into a clickable link. That will also ensure enable the signature to be checked and kept up to date if there is a future change.

That was supposed to say the "see" tag. Apparently, putting it in angle brackets caused Reviewable to make it disappear.

tombogle commented 1 year ago

You might need to look at the file history to see if you can figure out what it referred to. In any case, if doesn't make sense and there is no way to make it make sense, then better to remove it.