Re: [Jsynthlib-devel] checksum handling

[<tt_...@gm...>]

Hi Hiroo,

Hiroo Hayashi wrote:
> Hi Torsten,
> 
> Thank you for your prompt reply.

My pleasure :-)
Even if I didn't wrote comments in the last time I'm still watching the mailinglist and the work you (and others) are doing.
BTW, great job.

> Torsten> I have no objections, but I think calculateChecksum is needed
> Torsten> in some defined cases.
> 
> Torsten> 1. If we import a patch or request a patch from a synth to
> Torsten> check the integrity of the received patch.
> 
> Core does not do this now.  If it will do this, I'll propose to add
> 'boolean IPatch.checkChecksum()' method.
> 
> # validateChecksum() may be better? :-)
> 
> Torsten> 2. if we change a patch; for example if we edit a single
> Torsten> patch or if we change the content of a bank patch.
> 
> Torsten> But I don't think that we need to call calculateChecksum if
> Torsten> we only send or store a patch from library to the synth,
> Torsten> because all patches which are stored resp. hold in the
> Torsten> library should have the right Checksum. If not, we made a
> Torsten> mistake while importing, loading, requesting a patch
> Torsten> resp. editing or changing the content of a bank patch.
> Torsten> Because of this scenario I don't think we need to call
> Torsten> calculateChecksum for every send or store action.  Of course
> Torsten> we need to call calculateChecksum if we are working in the
> Torsten> editor and send/play a patch for test purpose.
> 
> Good point.
> 
> Here is what I thought.
> 
> Checksum must be calculated after a patch is modified and before the
> patch goes to outside of JSynthLib.  I thought of the following three
> options.
> 
> 1. Calculate checksum every time a patch goes to outside of synth
>    driver. (my proposal)
> 
> Calculate checksum at the checkpoint of the borderline. (We can do this,
> because we reduced the number of road crossing the borderline.)
> 
> As you wrote, we have to calculate checksum even if the patch has
> correct checksum.  But I think this is minor for performance.
> 
> 2. Calculate checksum every time a patch is modified.
> 
> Before the patch goes to outside of JSynthLib, the patch may be
> modified multiple times.  We don't have to calculate checksum multiple
> times.  But this is not so bad.
> 
> Problem is that some synth drivers has to calculate checksum in send()
> methods, because patch number and/or bank number information is a part
> of sysex message.  Unless I check every synth driver carefully, I have
> to calculate checksum in send() methods as we do for option 1.  Then
> there is no advantage to option 1.
> 
> 3. Mark a patch as 'modified' every time a patch is modified, and
>    calculate checksum every time a patch goes to outside of synth
>    driver if the patch is marked as 'modified'.
> 
> This is the best way.  But this requires more efforts than option 2.
> This is an extension of option 1.  If we want to do this, we can do this
> after we do option 1.
> 
> As a result I chose option 1.

Performance wasn't the main issue of my remarks.
I lost a little bit the overview of the current core structure.
So I thought a little bit about the general question if a call of calculateChecksum is necessary or not if a certain action is started.
Another topic is that I only know my DX7 (and his brothers/sisters) which has a simple checksum handling.

But I don't want to start a discussion about the most sophisticated checksum handling.
You have absolutly right with looking for a pragmatic solution.
And making option 1 as the first step is looking good.

Option 3 (marking a changed patch as modified) is looking very interesting for the actions "send", "play", "store", "export", "pasting
into library", etc...
But as you wrote this can be a future step.

BTW, your mentioned method validateChecksum() would also calculate the checksum of "imported" or "requested" patches. The only
difference would be (from my point of view) that validateChecksum() compares the calculated checksum with the original checksum of the
imported/requested patch (and start some while calculateChecksum() replaces the checksum of the patch without comparing immediatly.
I don't know if the information of a wrong checksum of an imported/requested patch will give us an additional benefit.

> Torsten> I also think this is the right time to decide, if it's not
> Torsten> better to put all the calculateChecksum stuff in an own core
> Torsten> class and remove it from the drivers. Of course the drivers
> Torsten> must contain the necessary information (in the constucto?) of
> Torsten> the kind of checksum (2's complement, ...), the start and end
> Torsten> of data which are concerned by the checksum, the place of the
> Torsten> checksum itself in the patch, ...
> 
> As the result of my proposal, core does not care about checksum at
> all.  It will be driver's responsibility to pass a patch with correct
> checksum to core.
> 
> As you wrote checksum is dependent with each driver.  I don't think
> there is many code which core can have for checksum.  Now we have
> DriverUtil.calculateChecksum() method.  We can add some other similar
> method for other algorithm.
> 
> I'm thinking to define a new IPatch class which supports a patch
> consists from multiple sysex messages.  The new patch class will have
> methods  and fields of the checksum (ex.  the start and end of data
> which are concerned by the checksum, the place of the checksum itself in
> the patch, etc.).  For different checksum algorithm, overriding is the
> easiest way, I think. As you see DriverUtil.calculateChecksum() method,
> it a simple method.

I see and I agree to you.
calculateChecksum() should be driver's task.
This is the most flexible way (and maybe the easiest :-) ).


Bye
Torsten