melanchall / drywetmidi

.NET library to read, write, process MIDI files and to work with MIDI devices
https://melanchall.github.io/drywetmidi
MIT License
545 stars 75 forks source link

make MidiEvent class serializable #81

Closed Fabrice-Deshayes-aka-Xtream closed 4 years ago

Fabrice-Deshayes-aka-Xtream commented 4 years ago

Hello

Do you thinks that it will be possible to make the Core.MidiEvent (and daughters classes) Serializable ? Maybe it's not a lot of work for you (just annotations?) but It will be very usefull for me :-)

I'm writing a midi fowarder which forwards midi events from source to destination (not only between input/output midi devices but using network too as source/destination). It works in local, thanks to your great lib but I'm stuck on the network part because i can't serialize the MidiEvents :-(

Thanks for your help.

Regards.

melanchall commented 4 years ago

Hi,

Thanks for using the library!

What kind of serialization are you interested in? Binary, XML or something else? You just need [Serializable] attribute on event classes?

Also please say what serialization class do you use so I can create unit tests.

Max

Fabrice-Deshayes-aka-Xtream commented 4 years ago

Hi !

[Serializable] attribute on event classes would be nice, i can choose what kind of serialization to use (json, xml, binary ...).

You can do a unit test for example with JSON serialization using Newtonsoft.Json NuGet package.

// assume that e is a MidiEventReceivedEventArgs
MidiEvent midiEvent = e.Event;
String jsonMidiEvent = JsonConvert.SerializeObject(midiEvent);
MidiEvent midiEventFromSerializedString = JsonConvert.DeserializeObject<MidiEvent>(jsonMidiEvent);

see https://www.newtonsoft.com/json

melanchall commented 4 years ago

Newtonsoft.Json will serialize any type without any specific attribute (like Serializable). So I need serialization that require this attribute. If you want to see this attribute on event classes, I suppose, serialization doesn't work without it. Can you show me serialization code so I can use it in unit tests to cover cases like yours?

Fabrice-Deshayes-aka-Xtream commented 4 years ago

When I execute the code above, i've got this error from newton on the DeserializeObject method. Maybe it's my fault, I understand now that MidiEvent is an abstract class so it can't be instanciate, so the problem is probably not related to serialization :-( but i don't know how to solve it...

Base Exception Type: Newtonsoft.Json.JsonSerializationException: Could not create an instance of type Melanchall.DryWetMidi.Core.MidiEvent. Type is an interface or abstract class and cannot be instantiated. Path 'NoteNumber', line 1, position 14.
   à Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateNewObject(JsonReader reader, JsonObjectContract objectContract, JsonProperty containerMember, JsonProperty containerProperty, String id, Boolean& createdFromNonDefaultCreator)
   à Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateObject(JsonReader reader, Type objectType, JsonContract contract, JsonProperty member, JsonContainerContract containerContract, JsonProperty containerMember, Object existingValue)
   à Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateValueInternal(JsonReader reader, Type objectType, JsonContract contract, JsonProperty member, JsonContainerContract containerContract, JsonProperty containerMember, Object existingValue)
   à Newtonsoft.Json.Serialization.JsonSerializerInternalReader.Deserialize(JsonReader reader, Type objectType, Boolean checkAdditionalContent)
   à Newtonsoft.Json.JsonSerializer.DeserializeInternal(JsonReader reader, Type objectType)
   à Newtonsoft.Json.JsonConvert.DeserializeObject(String value, Type type, JsonSerializerSettings settings)
   à Newtonsoft.Json.JsonConvert.DeserializeObject[T](String value, JsonSerializerSettings settings)
   à Newtonsoft.Json.JsonConvert.DeserializeObject[T](String value)
   à MidiRouter.MidiForwarderForm.OnNetworkEvent(PacketHeader header, Connection connection, String jsonEvent) dans C:\Users\fabrice\Documents\dev\C#\MidiRouter\MidiForwarder.cs:ligne 132
   à NetworkCommsDotNet.Tools.PacketTypeHandlerDelegateWrapper`1.Process(PacketHeader packetHeader, Connection connection, Object obj)
   à NetworkCommsDotNet.NetworkComms.TriggerGlobalPacketHandlers(PacketHeader packetHeader, Connection connection, Object returnObject, Boolean ignoreUnknownPacketTypeOverride)
melanchall commented 4 years ago

You can't deserialize into abstract class or interface by default. You'll get an exception always, Serializable (and any other) attribute won't help.

In case of Newtonsoft.Json you need to create custom converter to perform custom deserialization. You can search on the web by "newtonsoft json deserialize abstract class". In your custom converter you should return appropriate class based on value of the EventType field in JSON.

For example, see this answer: https://stackoverflow.com/a/30579193/2975589. These lines are similar to what you need to do:

switch (jo["ObjType"].Value<int>())
{
    case 1:
        return JsonConvert.DeserializeObject<DerivedType1>(jo.ToString(), SpecifiedSubclassConversion);
    case 2:
        return JsonConvert.DeserializeObject<DerivedType2>(jo.ToString(), SpecifiedSubclassConversion);
    default:
        throw new Exception();
}

Your code will be like that (I didn't test it):

switch (jo["EventType"].Value<MidiEventType>())
{
    case MidiEventType.NoteOn:
        return JsonConvert.DeserializeObject<NoteOnEvent>(jo.ToString(), SpecifiedSubclassConversion);
    // ... and so on ...
}
Fabrice-Deshayes-aka-Xtream commented 4 years ago

Thanks for you time and reactivity. I've understood, You can now close this issue. I will let you know when my program will be available if you want.

Regards.

melanchall commented 4 years ago

Thanks for question :)