Re: [Flightgear-devel] 2020.3.15 RCs

[Florent R. <f.r...@fr...>]

Erik Hofman <er...@eh...> wrote:

> The patch looks quite okay to me. I'm just not sure why it is needed...

When using the reproducing instructions at

  https://forum.flightgear.org/viewtopic.php?f=25&t=40890

(i.e., tune standby and selected freqs in NAV1 or NAV2, listen to the
Morse code for one of them that is close enough to reach you, then swap
the standby and selected frequencies), the following code from
flightgear/src/Sound/audioident.cxx is run (it belongs to
AudioIdent::setIdent()):

  if ( _sgr->exists( _fx_name ) )
      _sgr->remove( _fx_name );

  if (!ident.empty()) {
      SGSoundSample* sound = FGMorse::instance()->make_ident(ident, _frequency );
      sound->set_volume( volumeNorm );
      if (!_sgr->add( sound, _fx_name )) {
          SG_LOG(SG_SOUND, SG_WARN, "Failed to add sound '" << _fx_name << "' for ident '" << ident << "'" );
          delete sound;
          return;
      }

This code removes from the "avionics" group the SGSoundSample with
refname "nav-ident-0" (for NAV1) or "nav-ident-1" (for NAV2) if it
already exists, then adds it again with the same refname but presumably
the Morse code for the newly-tuned beacon.

With the recent changes, the _sgr->add() call fails because
SGSampleGroup::remove() doesn't erase the item from
SGSampleGroup::_samples anymore. Therefore, the log message is printed
and the 'delete' executed. This is the 'delete' that causes the crash (I
don't know why, maybe a double free somewhere?). But we shouldn't be
there anyway, the root problem is the failing _sgr->add() call.

My patch makes it so that *by default*, SGSampleGroup::remove()
immediately removes the sample from SGSampleGroup::_samples (as it used
to do until the 'instant' queue changes), which ensures that the above
_sgr->add() call doesn't fail. During the loop over the _samples map in
SGSampleGroup::update(), my patch passes true as the new optional
argument of SGSampleGroup::remove() so that in this case, the removal
from _samples is delayed until after said loop.

I hope this is clearer now. :-)

[ Note: the report at

  https://forum.flightgear.org/viewtopic.php?f=25&t=40890#p405708

  is correct: when switching NAV1 or NAV2 from a station for which one
  can hear the Morse code to one that is too far, what seems to be a
  truncated Morse code of the new station (the too far one) is played at
  first, for approx. a second or less; then comes the expected silence.
  This is certainly a bug, but not very severe and it was already
  present in 2020.3.13 (built and tested yesterday to check this). So,
  this is a different, separate problem. ]

Regards

-- 
Florent