Fixes for being able to save a Tagged File

[Winterleaf]

Switched Stream out with MemoryStream,

Also created a constructor which allows a byte[] to be passed in as initialization versus just a stream.

There is a issue when you create a stream using a byte[], the stream created is not writable.
So this changes converts the byte[] properly to a stream so it is writable.
I also added a property to the FileAbstraction that lets you get a byte[] of the new data so the users don't need to convert the stream themselves.

var abs = new StreamFileAbstraction(filename, songData);
using (var file = File.Create(abs))
{
file.Tag.Album = Album;
file.Tag.Title = SongTitle;
file.Tag.AlbumArtists = new[] {Artist};
file.Tag.AmazonId = Song.AmazonTrackId;
file.Save();
}
StorageFile sf = await folder.CreateFileAsync(filename, CreationCollisionOption.FailIfExists);
sf.WriteAllBytes(abs.TaggedMediaData);

[timheuer]

Hmm, breaking change in the signature?

[Winterleaf]

Yes, I guess it could be considered that, or it could be considered an improvement because it would prevent others from having the problem I had when I created a memory stream to use and initialized it using a byte[] thus making not expandable.

[timheuer]

I'm not disagreeing with the improvement, but the API signature changes here. Someone picking up this update might be broken, etc. and causing them to have to change their code. That's the definition of a breaking change here in my context. We need to maintain API compatibility. What we'd want to understand if is someone was using this API against a Stream object, then they took this update...what happens to that app? Because MemoryStream derives from Stream is that okay? Do they see warnings/errors on recompile? And if so, what code would their app need to do to change to a MemoryStream.

Simply making the change here could have compat impact that I don't want to take without those answers well understood. It may be a non-issue due to the derived/base relationship of Stream/MemoryStream, but we need to test the scenario.

[Winterleaf]

Couldn’t we just leave the old constructor and have this an a additional one?

From: Tim Heuer [mailto:notifications@github.com]
Sent: Wednesday, July 06, 2016 12:45 PM
To: timheuer/taglib-sharp-portable taglib-sharp-portable@noreply.github.com
Cc: Vincent Gee vgee@winterleafentertainment.com; Author author@noreply.github.com
Subject: Re: [timheuer/taglib-sharp-portable] Fixes for being able to save a Tagged File (#16)

In src/TagLib.Shared/TagLib/StreamFileAbstraction.cs #16 (comment) :

•    ///                }
  

•    ///                StorageFile sf = await folder.CreateFileAsync(filename, CreationCollisionOption.FailIfExists);
  

•    ///                sf.WriteAllBytes(abs.TaggedMediaData); 
  

•    /// 
  

•    /// </summary>
  

•    /// <param name="name"> Name of the media file</param>
  

•    /// <param name="data"> Byte Array of the media source</param>
  

•    public StreamFileAbstraction(string name, byte[] data)
  

•    {
  

•        ReadStream = new MemoryStream();
  

•        ReadStream.Write(data, 0, data.Length);
  

•        WriteStream = ReadStream;
  

•        Name = name;
  

•    }
  

•    public StreamFileAbstraction(string name, MemoryStream readStream)
  

I'm not disagreeing with the improvement, but the API signature changes here. Someone picking up this update might be broken, etc. and causing them to have to change their code. That's the definition of a breaking change here in my context. We need to maintain API compatibility. What we'd want to understand if is someone was using this API against a Stream object, then they took this update...what happens to that app? Because MemoryStream derives from Stream is that okay? Do they see warnings/errors on recompile? And if so, what code would their app need to do to change to a MemoryStream.

Simply making the change here could have compat impact that I don't want to take without those answers well understood. It may be a non-issue due to the derived/base relationship of Stream/MemoryStream, but we need to test the scenario.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub https://github.com/timheuer/taglib-sharp-portable/pull/16/files/281591e90e7a39ac6f0461d3751d4ce78abb1699#r69767164 , or mute the thread https://github.com/notifications/unsubscribe/ACUNZENW_yoyOG9yv4tEdmSgV2DukTVCks5qS9uWgaJpZM4JFlol . https://github.com/notifications/beacon/ACUNZDHdDkegmXzl2X5NGg9SULi_IQmwks5qS9uWgaJpZM4JFlol.gif