Append mode for Writer class

[jstrait]

Append mode for Writer class

[jstrait]

Thanks for the suggestion! This is something I've briefly thought about before, but never implemented because it hasn't scratched a personal itch. It seems like a reasonable feature, but would need to update the Writer constructor signature to allow indicating that it should be in append mode. Is there a particular use case where this would be helpful for you?

[jamestunnell]

If an exception occurs while writing, before Writer#close is called, I think a wave file would then be left in a non-usable state since the RIFF header wouldn't have been finalized yet. True? So, as a user I would like to be sure that the data I've written so far is safe and usable in the event of an exception.

I think a simple way to implement this is to close the file and open it in append mode. Then the Writer class could be set up to close/reopen on its own at a set interval (every N samples, or something). But at least giving the user the option to close and reopen in append mode would be a step in the right direction.

There may be other convenience reasons for wanting to append.

[jstrait]

I hadn't been thinking about the point you brought up of dealing with exceptions that occur when writing a file using a block. For example, in this code:

Writer.new("error.wav", format) do |writer|
  writer.write(buffer)
  x = 1 / 0
end

the resulting Wave file will not be valid, and there's no way (in the current version of WaveFile) to catch the ZeroDivisionError and close the file.

I think this would be simple to fix. In Writer.new, this code:

if block_given?
  yield(self)
  close
end

could be replaced with this code:

if block_given?
  begin
    yield(self)
  ensure
    close
  end
end

Which would ensure that the header gets written even if an exception occurs.

While the wave file would be playable, you wouldn't be able to go back and add more data to the file. Your suggestion of being able to open a file in append mode would resolve that.

[jstrait]

I opened a separate bug for the issue of no header being written when an exception occurs. If you are interested in fixing that bug and adding tests just assign the issue to yourself. If not, no problem, I can do it.

One other thought about adding support for appending is that the Writer.samples_written attribute could become misleading. That is, you might want a distinction between the number of samples in the entire file, and the number of samples written since the file was opened. When opened in regular mode, these would be the same. Something like Writer.total_samples and Writer.samples_written.

[jamestunnell]

I added support for append mode over at my fork: https://github.com/jamestunnell/wavefile

Included in the changes, I added a 'total_sample_frames' method like you suggested. Though now that I think about it, I suppose for naming consistency it could also be called 'sample_frames_total'. Maybe a 'sample_frames_existing' method would be desirable too?

Would you take a look at the changes? Any feedback?

[jstrait]

Thanks for the code! Unfortunately, while I was looking at it I realized a few basic problems with appending data to a Wave file. The issues revolve around the fact that a Wave file is composed of "chunks" which are not guaranteed to be in a particular order.

First, the data chunk is not guaranteed to be the last chunk in the file. If it's not, then appending would write the data in the wrong place. I think it's sensible for the data chunk to be last, and assume that that is the case for most Wave files (including all files written by this gem), but I'm not sure how safe of an assumption that is universally.

Another issue is that when the file is closed, the act of rewriting the header could cause existing chunks to be overwritten. For example, this gem always writes a format chunk, optionally followed by a fact chunk, and then a data chunk. If the existing file has other chunks between the format and data chunks then they will be overwritten and the file will likely be corrupted.

An implementation of this feature would need to be aware of the chunks in the file, and deal with them so that they all end up in the output file. In addition, the data in certain chunks (such as the fact chunk) depends on the amount of data in the data chunk, and they would have to be updated accordingly when the file is closed.

Sorry for not realizing this earlier. This feature seems more complicated than it did on the surface. I think this is something that would have to be thought about more before being implemented.

[jamestunnell]

Yeah, I see your points and I agree.