I've committed the patches.
Someone on Linux will need to add 'OpenSaveCommands.cpp'
to the build.  I've done it for Windows only.






-------

According to stack overflow:

http://stackoverflow.com/questions/2132747/warning-c4251-when-building-a-dll-that-exports-a-class-containing-an-atlcstrin

The C4241 warning is relevant where a dll exported class uses a non dll exported class.
I've gone ahead and exported the TagMap class, and added a comment as to why.
That means we can remove the #pragma to disable the warning and I did.

Probably safest to do this, as in theory the private members could have a constructor that
causes the DLL to crash through not having been exported when an instance of the class is
created by the DLL, so being private members is not enough of a guarantee by itself.


On 13/01/2014 11:51, Stephen Parry wrote:
Hi Martyn,
Thanks for your careful consideration of this - my replies are below.

Message: 4
Date: Sun, 12 Jan 2014 23:59:41 +0000
From: Martyn Shaw <martynshaw99@gmail.com>
Subject: Re: [Audacity-devel] Second Exports Patch
To: audacity-devel@lists.sourceforge.net
Message-ID: <52D32C6D.8050007@gmail.com>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed

Hi  

On 11/01/2014 18:29, Stephen G. Parry wrote:
> @Martyn
> If you look at the example for WX_DECLARE_OBJARRAY on the reference
> page: http://docs.wxwidgets.org/trunk/dynarray_8h.html you will see
> the comment says:
>
> class MyClass;
> WX_DECLARE_OBJARRAY
> <http://docs.wxwidgets.org/trunk/dynarray_8h.html#a515e9265b474d8a152ec3ec42179abde>(MyClass,
> wxArrayOfMyClass); // note: not "MyClass *"!
>
> That warning is then repeated for the other macros. That's all there
> is. Thin I know, but can we safely ignore it?

There is
http://docs.wxwidgets.org/2.8/wx_wxarray.html#wxdeclareobjarray
where it says "you should use the _PTR versions of the macros", so
maybe that's a clue?

Also
http://msdn.microsoft.com/en-us/library/esew7y1w.aspx
seems to be saying that you shouldn't export things to dll that could
muck you up or mess with you.  Maybe that's pointers.

Like I said, I'm no expert on this stuff.  But I'm all for improving
the plug-in dll stuff; I just wish I had the expertise!


OK I will review the use of the _PTR macros - the code has been there a while, but you never know what unresolved bugs might be down to things like this.
We don't need to suppress warnings now.

> @James
> Yes, all I am doing is exposing more of the API and I am happy with
> the usual contractual caveats.

But warning are saying (perhaps) that we are exposing too much.

The warnings go away if you expose MORE of the API, not less...
So I went ahead and exposed more.



  I have to recompile my plug-in for each
> rev of audacity anyhow, so there it is.

'Unfortunately' there is a known way around that, it is not a good
system.  Aurora just read the version from the prefs.

Having to review and recompile each version is an acceptable shortcoming for us, but not if we have re-rip and re-hack large chunks, which is where we are without the exports I've proposed.
+1. I agree.

 

> The warning I've suppressed in my first patch is C4251
>
> 1>c:\development\audacity-trunk\src\tags.h(112) : warning C4251:
> 'Tags::mIter' : class 'TagMap_wxImplementation_HashTable::iterator'
> needs to have dll-interface to be used by clients of class 'Tags'
> 1>        c:\development\audacity-trunk\src\tags.h(53) : see
> declaration of 'TagMap_wxImplementation_HashTable::iterator'
> 1>c:\development\audacity-trunk\src\tags.h(113) : warning C4251:
> 'Tags::mXref' : class 'TagMap' needs to have dll-interface to be used
> by clients of class 'Tags'
> 1>        c:\development\audacity-trunk\src\tags.h(53) : see
> declaration of 'TagMap'
> 1>c:\development\audacity-trunk\src\tags.h(114) : warning C4251:
> 'Tags::mMap' : class 'TagMap' needs to have dll-interface to be used
> by clients of class 'Tags'
> 1>        c:\development\audacity-trunk\src\tags.h(53) : see
> declaration of 'TagMap'
>
> The compiler assumes that because we have exported Tags, we want to
> export all the classes that define its members.

I'm not sure that that is the correct assessment of the message (see
above).  But again, I'm not at expert with this.

I have checked different scenarios and am deep in testing at the minute. So far Audacity is 100% stable with the exports in place. I am still reworking our tag handling code to use the exports, and that will give a greater level of certainty. We have some memory leaks, but these seem to be from an unrelated problem in Audacity trunk.
Cool.
I've enabled better memory leak reporting.  For example it is now clear we leak a timer - whereas it wasn't clear where that leak came from before.

 

  We don't, especially
> as the members in question (mIter, mXref and mMap) are private. I've
> added an explanatory comment. New patch attached, replacing BOTH
> previous ones.
>
> If we went for the more limited export approach on the second patch to
> class Exporter and its children,  then it would need the same pragma
> and comments, but if we are happy with the full patch exposing the
> three classes (FileFormatInfo, ExporterPlugin and Exporter) and the
> supporting arrays, then no pragmas are needed.

I currently think that we should make the minimum changes needed to
support easy and safe support of your project (which I see at
https://github.com/sgparry/eutychus) (thanks from to OS community!).
My problem is that I don't know what they are, and don't have the
knowledge to approve them.

The patches I have proposed are a tiny proportion of the code base. With the exception of the SaveProject and OpenProject script commands I put in in patch 3 (the rest of that patch, the new SaveAs overload in class Project, is essential), they are the minimum I need to support our code - they are just enough to allow me to dispense with the ugly hacks I have had to do and no more. I have specifically avoided changing any existing functionality in Audacity by adding not changing. The new DLL entry points should only conceivably destabilize Audacity if someone actually uses them - which is our lookout as anything at that level is experimental anyhow.

I've added patch 3 now too.
As far as I can tell it is essentially making an existing command available for scripting.  Yes?


> Are you OK to commit this please?

I am not, at this point.  Sorry.  I really am.  I want you to be able
to very easily develop dlls that do great stuff.  If others are
reading this, please approach us earlier in your development cycle.

So where to we go from here? Our live code is working great with the current release of audacity, but to get there I have had to make so many stupid ugly hacks - e.g. copying in code from Audacity wholesale, cutting out references to unneeded code and compiling that into our plugin. When the next release appears I am going to potentially have to redo all that work again.

We carefully considered all alternatives before embarking on this development, but:
- Manually using Audacity without the plug-in increases the work involved on a weekly basis by 300% and introduces errors at every stage.
- The scripting interface simply does not do what we need. There is no access to meta-data and no way of automating Save As. Plus using it would mean having a separate UI, which is counter-intuitive for novice users.
- We could fork Audacity, but that has been tried by others in the past and proved a vast waste of effort.
- We need both automation and interaction so things like sox are no use.
- We need lossless recording for archival purposes.
- No other Open Source product does what Audacity does.
Are they any possibilities I've missed that you can suggest?
Checked in now.  :-)

Please document the new commands/parameters for the wiki.  http://manual.audacityteam.org/man/Scripting

I should point out that I am willing to commit to help long term on the extensibility features in Audacity. I am a C++ developer with 20 years industry experience on Windows, GNU/Linux and Unix. Plugin APIs, scripting interfaces and such are very much my bag and I would very much like to help in turning Audacity into a fully modular, extensible and scriptable beast.

That's great.
Yes, we would like to improve plug-in use.

Thanks for your work on this Stephen.

--James.