Please clarify...

In project.cpp we have this code:

#define SAMPLE_SIZE_DISK(SampleFormat) ((SampleFormat == int24Sample) ? \
   3 : SAMPLE_SIZE(SampleFormat) )

recTime /= SAMPLE_SIZE_DISK(gAudioIO->GetCaptureFormat());
// note size on disk (=3 for 24-bit) not in memory (=4 for 24-bit)

What does the comment "Audacity does not currently record in 24 bit" mean?  I can select 24 bits as an option in preferences.  If that is taking 3 bytes per sample (on disk) then the above calculation looks right.  If that is taking four bytes per sample (on disk) then we need to change the 3 to a 4 in the #define and fix the comment about size on disk.  Either way it looks like we can clear this one easily??

(In reply to comment #1)
> #define SAMPLE_SIZE_DISK(SampleFormat) ((SampleFormat == int24Sample) ? \
>
>   3 : SAMPLE_SIZE(SampleFormat) )
>
> recTime /= SAMPLE_SIZE_DISK(gAudioIO->GetCaptureFormat());
> // note size on disk (=3 for 24-bit) not in memory (=4 for 24-bit)
>
> What does the comment "Audacity does not currently record in 24 bit" mean? 
> I can select 24 bits as an option in preferences.  If that is taking 
> 3 bytes per sample (on disk) then the above calculation looks right. 
> If that is taking four bytes per sample (on disk) then we need to 
> change the 3 to a 4 in the #define and fix the comment about size
> on disk.  Either way it looks like we can clear this one easily??

It seems that code comment is correct as far as it goes. For example when recording 24-bit @ 96000 Hz on disk it takes about 33 MB per minute. But it 
takes about 46 MB per minute in RAM. 

The comment was probably something taken out of context from -devel list that nonetheless was given as an explanation why the size calculation for 24-bit 
was actually calculating what 32-bit requires.

(In reply to comment #2)

So is this really a bug?

(In reply to comment #3)
> So is this really a bug?
From user perspective, yes. When users record 24-bit we tell them they need 50% more disk space than they actually need.

(In reply to comment #4)

Gale, that was not clear to me in your previous comment. Your saying
33MB per minute on disk and 46 in RAM (which I took to be record-to-RAM
pref) read to me like it was reporting correctly.

I had checked that in the code for SAMPLE_SIZE_DISK, it sets the SAMPLE_SIZE_DISK divisor to 3 for int24Sample.


The actual bug is completely elsewhere. Commit 5712 (from 2006!) -- if captureFormat is passed in to AudioIO::StartPortAudioStream() as int24bit, it is always treated as 32-bit float thereafter in the AudioIO class. So I think the calculations are correct as things stand, but r5712 is probably questionable. 

This has been around a long time, and has other repercussions, so I do not think it's a cherry, and removed that keyword.

(In reply to comment #5)
> from http://code.google.com/p/audacity/source/detail?r=11903
> So we *never* actually handle 24-bit?! This causes mCapture to be set to 
> floatSample below  
FWIW (to clarify) the actual disk space taken by the recording is equivalent to 24-bit (150% of 16-bit and 75% of 32-bit).

(In reply to Gale Andrews from comment #6)
In https://github.com/audacity/audacity/commit/18eb2bb James added this answer

>   // ANSWER-ME: So we *never* actually handle 24-bit?! This causes mCapture 
>   // to be set to floatSample below.
>   // JKC: YES that's right.  Internally Audacity uses float, and float has 
>   // space for 24 bits as well as exponent.  Actual 24 bit would require 
>   // packing and unpacking unaligned bytes and would be inefficient.

Does that answer have implications for this bug?

Re Comment #7 - It supports what you said in comment #2.  In RAM it takes 4 bytes per sample and is stored as floats.  When saved, apparently, Audacity converts (losslessly) from float to 24 bit integer.  Lossless because 32 bit float represents 24 bit integer without error.

RAs macro is correct and in 
GetEstimatedRecordingMinsLeftOnDisk()
used from timer record gives the correct value.  However when generating the status message from OnTimer() it calculates using the internal capture format (float) in its calculation.  If it used the preference /SamplingRate/DefaultProjectSampleFormat (int24) it would get the right answer.  However due to performance Bug 1436 I am reluctant to make that simple change since this happens every time Audacity writes a block during recording.

DEVEL-FIX MADE in
https://github.com/audacity/audacity/commit/101b3ee8cd324ee57f66789214f1b5084e37b722

In the end I went for using the preferences as the writes are about every 3 seconds and the slight performance hit does not seem to be of significance in practice.

(In reply to James Crook from comment #9)
Testing on W10 and Mac El Capitan on r145a54d 05Jul16

Looks to be good now on 24-bit settings

W10 remaining disk space
32-bit: 313 hours 43 minutes
24-bit: 418 hours 17 minutes
16-bit: 627 hours 25 minutes 


Mac El Capitan remaining disk space
32-bit: 133 hours 16 minutes
24-bit: 177 hours 41 minutes
16-bit: 266 hours 32 minutes

Tested at 396a6f0 from audacity/audacity on Linux. 

32-bit: 3 hours 18 minutes
24-bit: 4 hours 24 minutes
16-bit: 6 hours 36 minutes

Works correctly, so flagging this as Test-OK-Lin.

Main issue fixed so RESOLVED-FIXED.  

Append Record "disk space remains" is incorrect (unchanged) if you change the track format or rate. These issues are now at bug 1562 and bug 1563 respectively (both P4).

Just for information...  The dependence on Bug 1436 was discussed in comment #8, and should have been removed with comment #9.  Instead this bug stayed open (due to the dependence) until today when bug #1436 was closed as WORKSFORME.

(In reply to James Crook from comment #13)
Hmm - looks like there is a bug in the software or settings then because I recall no warning about dependencies when setting RESOLVED - FIXED on 19Dec2016. History thinks that resolution was FIXED. 

I would suggest that comment #8 might have said that a dependency had actually been set. I did not infer that from the comment - had I noticed the dependency I would have removed it after comment #9.