Add track display view to AUP

[Edgar]

On -devel
http://audacity.238276.n2.nabble.com/Fr ... 64569.html
Martyn asked:

it would be great if the view of each track (Waveform, Waveform (dB), Spectrogram, Spectrogram (logF), Pitch (EAC)) were saved to the project (AUP) file

It's only a couple lines of code:

Code: Select all

Index: src/WaveTrack.cpp
===================================================================
--- src/WaveTrack.cpp	(revision 13514)
+++ src/WaveTrack.cpp	(working copy)
@@ -1473,6 +1473,8 @@
                return false;
             mChannel = nValue;
          }
+         else if (!wxStrcmp(attr, wxT("waveTrackDisplay")) && XMLValueChecker::IsGoodInt(strValue) && strValue.ToLong(&nValue))
+            mDisplay = nValue;
          else if (!wxStrcmp(attr, wxT("linked")) &&
                   XMLValueChecker::IsGoodInt(strValue) && strValue.ToLong(&nValue))
             SetLinked(nValue != 0);
@@ -1553,6 +1555,7 @@
    xmlFile.WriteAttr(wxT("rate"), mRate);
    xmlFile.WriteAttr(wxT("gain"), (double)mGain);
    xmlFile.WriteAttr(wxT("pan"), (double)mPan);
+   xmlFile.WriteAttr(wxT("waveTrackDisplay"), GetDisplay());
 
    for (WaveClipList::compatibility_iterator it=GetClipIterator(); it; it=it->GetNext())
    {

Patch attached:

[steve]

Would it be worth making it a little longer so that the XML is clearer for humans? (example: display=Waveform_dB)

[Edgar]

Would it be worth making it a little longer so that the XML is clearer for humans? (example: display=Waveform_dB)

Funny you should ask; that's the way I originally tried to code it. Unfortunately, the value is an enumerated type so we would need to:
1) add a global constant static array of wxStrings (which would need to be kept in step with the enumerated types);
2) write & read the value as a string, then convert into an enumerated type (ditto – keep in step).
This adds a lot of complexity to what is otherwise trivially simple code.

One possible option would be to add a "NOOP" XML Tag when we write the track. Once again this would require that global constant static array of strings which would need to be kept in step with the enumerated types.

Code: Select all

<?xml version="1.0" standalone="no" ?>
<!DOCTYPE project PUBLIC "-//audacityproject-1.3.0//DTD//EN" "http://audacityteam.org/xml/audacityproject-1.3.0.dtd" >
<project xmlns="http://audacityteam.org/xml/" projname="Delta_data" version="1.3.0" audacityversion="2.0.1" sel0="0.0000000000" sel1="5.0000000000" vpos="0" h="0.0000000000" zoom="348.4000000000" rate="44100.0">
	<tags/>
	<wavetrack name="Audio Track" channel="2" linked="0" mute="0" solo="0" height="150" minimized="0" rate="44100" gain="1.0" pan="0.0" waveTrackDisplay="0" trackDisplaysAs="Waveform_dB">
		<waveclip offset="0.00000000">
			<sequence maxsamples="262144" sampleformat="262159" numsamples="220500">
				<waveblock start="0">
					<simpleblockfile filename="e0000c3f.au" len="220500" min="-0.799729" max="0.799916" rms="0.319909"/>
				</waveblock>
			</sequence>
			<envelope numpoints="0"/>
		</waveclip>
	</wavetrack>
</project>

waveTrackDisplay="0" trackDisplaysAs="Waveform_dB"
The only problem here is keeping the string array in step with the enumerated types; we already have at least one similar problem in the TimeTextControl code where it is dealt with by a comment in the code.

I'd be happy to write a patch either way (or both ways) if you would like.

[steve]

which would need to be kept in step with the enumerated types

1) What would happen with your original code if a later version of Audacity changed the enumerated types (so they were not in step with previous versions) and a user opens an old project?

2) What would happen if strings were used and a later version changed the strings?

My guess is that in the first case, the user will get the wrong waveform views (which could appear "random"). The integer values would be interpreted as unintended types.
In the second case, my guess is that any non-matching strings would be silently ignored, giving the default "Waveform" view for that track.

[Edgar]

1) What would happen with your original code if a later version of Audacity changed the enumerated types (so they were not in step with previous versions) and a user opens an old project?

2) What would happen if strings were used and a later version changed the strings?

My guess is that in the first case, the user will get the wrong waveform views (which could appear "random"). The integer values would be interpreted as unintended types.
In the second case, my guess is that any non-matching strings would be silently ignored, giving the default "Waveform" view for that track.

In the first case your guess is partially right based on how the enumerated types were changed; if new types were simply added on the end than the old enumerated types would still be valid and should work. The opposite is not true, opening a newer project in an older version could cause grief.

In your second case your guess is correct (assuming that the Developers properly initialized the Track class' member for display type).

I suppose a sanity check should be added to ensure that, however the integer display type is calculated from the XML file, it is within the bounds of the enumeration type. I do not know if enumerations have a method to determine if a given value is a member of the enumeration or if that would need to be handrolled.

[steve]

In the first case your guess is partially right based on how the enumerated types were changed;

In the example: "so they were not in step with previous versions"
In other words, not just adding a new one, but jumbling them up to some degree.

I suppose a sanity check should be added to ensure that, however the integer display type is calculated from the XML file, it is within the bounds of the enumeration type. I do not know if enumerations have a method to determine if a given value is a member of the enumeration or if that would need to be handrolled.

From a quick test with the current release version of Audacity:

• Unrecognised attributes are ignored
• Attribute values are always quoted strings ("NmTokens")
• Handling / validating attribute values is handled in the Audacity code

What I'm suggesting, is that we use meaningful NmTokens rather than a zero indexed list of values (which still have to be written as quoted string values). Maintaining the list should be trivial as long as both the track view enumeration list and the associated NmToken string list are kept together somewhere sensible in the code.

(just a thought; if someone thoughtlessly changed from a zero indexed list to "one indexed", that would really screw things up )

I would be inclined to use an attribute rather than an element for this because:
1) It's not part of the project "data" (nothing to do with the actual audio)
2) There can only ever be one value per track (in the current track panel). XML allows multiple elements per tag but only one value per attribute.
3) XML (in Audacity) is robust against "foreign" attributes, whereas foreign elements can invalidate the entire tag (or entire project).
4) Maintaining backward and future compatibility is probably easier.

[Edgar]

Sanity check (after applying the previous patch):
In srcWaveTrack.h at or near line number 391 we need to change the enumeration slightly and add a new function:

Code: Select all

enum WaveTrackDisplay {
      WaveformDisplay,
      WaveformDBDisplay,
      SpectrumDisplay,
      SpectrumLogDisplay,
      PitchDisplay
   };
   const static bool ItIsAMemberOfWaveTrackDisplay(WaveTrackDisplay pDisplayType) {
      if ((pDisplayType == WaveformDisplay) ||
         (pDisplayType == WaveformDBDisplay) ||
         (pDisplayType == SpectrumDisplay) ||
         (pDisplayType == SpectrumLogDisplay) ||
         (pDisplayType == PitchDisplay)) return true;
      return false;
   };

and use the function for a sanity check in srcWaveTrack.cpp at or near line number 1476:

Code: Select all

         else if (!wxStrcmp(attr, wxT("waveTrackDisplay")) && XMLValueChecker::IsGoodInt(strValue) && strValue.ToLong(&nValue)) {
            if (ItIsAMemberOfWaveTrackDisplay(((WaveTrackDisplay)nValue)))
               mDisplay = nValue;
            else mDisplay = WaveformDisplay;
         }

*Edit – 28 October 2014: error in sanity check code

[Edgar]

In the first case your guess is partially right based on how the enumerated types were changed;

In the example: "so they were not in step with previous versions"
In other words, not just adding a new one, but jumbling them up to some degree.

In short - chaos!

From a quick test with the current release version of Audacity:

• Unrecognised attributes are ignored
• Attribute values are always quoted strings ("NmTokens")

Unrecognized attributes are ignored - yes.
Attribute values are written as strings, but in this case they are converted from integers and then back into integers and there is no (easy) way for the Write/Read code to convert from the enumerated integer into a string. It could be done but as I pointed out above it would require duplicating the integer enumeration with a string array (both of these declarations/definitions must be in the header file WaveTrack.h). Then you would have to have, in both the XML Write & XML Read functions, a complex set of if/else if statements stepping through all of the possible strings (obviously, this would need to be in WaveTrack.cpp). These two sets of complex if/else if statements would need to be kept in step with the declarations (both of which would need to be kept in step with each other) in the header file. In short ugly and not something I would want to try to get past Developer review.

[Edgar]

I would be inclined to use an attribute rather than an element for this because:
1) It's not part of the project "data" (nothing to do with the actual audio)
2) There can only ever be one value per track (in the current track panel). XML allows multiple elements per tag but only one value per attribute.
3) XML (in Audacity) is robust against "foreign" attributes, whereas foreign elements can invalidate the entire tag (or entire project).
4) Maintaining backward and future compatibility is probably easier.

Code: Select all

void WaveTrack::WriteXML(XMLWriter &xmlFile)
{
   xmlFile.StartTag(wxT("wavetrack"));
   xmlFile.WriteAttr(wxT("name"), mName);
   xmlFile.WriteAttr(wxT("channel"), mChannel);
   xmlFile.WriteAttr(wxT("linked"), mLinked);
   xmlFile.WriteAttr(wxT("mute"), mMute);
   xmlFile.WriteAttr(wxT("solo"), mSolo);
   xmlFile.WriteAttr(wxT("height"), this->GetActualHeight());
   xmlFile.WriteAttr(wxT("minimized"), this->GetMinimized());
   xmlFile.WriteAttr(wxT("rate"), mRate);
   xmlFile.WriteAttr(wxT("gain"), (double)mGain);
   xmlFile.WriteAttr(wxT("pan"), (double)mPan);
   xmlFile.WriteAttr(wxT("waveTrackDisplay"), GetDisplay());
   
   for (WaveClipList::compatibility_iterator it=GetClipIterator(); it; it=it->GetNext())
   {
      it->GetData()->WriteXML(xmlFile);
   }

   xmlFile.EndTag(wxT("wavetrack"));
}

In respect to WaveTracks I do not understand your distinction between "attribute" & "element"; the above is the complete XML Write function for the WaveTrack. I do not really understand all this XML code but from what you are saying I think you mean that we should add a new "sub tag" under the basic "wavetrack" (xmlFile.StartTag(wxT("wavetrack"));), give it a human readable name and no elements. I suppose I could try this but all of the previous complaints about complex if/else if still apply.

[steve]

In respect to WaveTracks I do not understand your distinction between "attribute" & "element";

I'm looking at this from the AUP file (XML) point of view.

Currently we have a structure like:

Code: Select all

<wavetrack name="Audio Track" channel="2" linked="0" mute="0" solo="0" height="150" minimized="0" isSelected="1" rate="44100" gain="1.0" pan="0.0">
  <waveclip offset="0.00000000">
    <sequence maxsamples="262144" sampleformat="262159" numsamples="1323000">
      <waveblock start="0">
        <simpleblockfile filename="e00002b4.au" len="262144" min="-0.799917" max="0.799982" rms="0.517421"/>
      </waveblock>
      <waveblock start="262144">
        <simpleblockfile filename="e000047d.au" len="262144" min="-0.661273" max="0.661167" rms="0.419531"/>
      </waveblock>
      <waveblock start="524288">
        <simpleblockfile filename="e00002ac.au" len="262144" min="-0.522573" max="0.522572" rms="0.32174"/>
      </waveblock>
      <waveblock start="786432">
        <simpleblockfile filename="e000063a.au" len="262144" min="-0.383848" max="0.383812" rms="0.224217"/>
      </waveblock>
      <waveblock start="1048576">
        <simpleblockfile filename="e00006c5.au" len="262144" min="-0.245159" max="0.245132" rms="0.127524"/>
      </waveblock>
      <waveblock start="1310720">
        <simpleblockfile filename="e00002fd.au" len="12280" min="-0.106462" max="0.106465" rms="0.031621"/>
      </waveblock>
    </sequence>
    <envelope numpoints="0"/>
  </waveclip>
</wavetrack>

Simplified:

Code: Select all

<wavetrack attribute="value" ... >
	<waveclip offset="value">
		<sequence attributes="values" ...>
			<waveblock start="0">
				<simpleblockfile attributes="values" ... />
			</waveblock>
			...
			...
		</sequence>
		<envelope numpoints="0"/>
	</waveclip>
</wavetrack>

To add "WaveTrackDisplay" as an element (child element):

Code: Select all

<wavetrack attribute="value" ... >
	<spectrumdisplay>
		<waveclip offset="value">
			<sequence attributes="values" ...>
				<waveblock start="0">
					<simpleblockfile attributes="values" ... />
				</waveblock>
				...
				...
			</sequence>
			<envelope numpoints="0"/>
		</waveclip>
	</spectrumdisplay>
</wavetrack>

As an attribute of the <wavetrack> element:

Code: Select all

<wavetrack name="Audio Track" WaveTrackDisplay="SpectrumDisplay" channel="2" linked="0" mute="0" solo="0" height="150" minimized="0" isSelected="1" rate="44100" gain="1.0" pan="0.0">