[Jaap Keuter <jaap.keuter@xxxxxxxxx>]

Hi,

I've looked at your code and looks close being ready for inclusion.
Here some steps to take:
- Convert to build in dissectors, there's no reason not to.
- Remove calls to check_col()
- In the header fields change the "" blubs by NULL
- You could look into the expert info system for error reporting.
- The occasional // style comment has to go
- some proto_tree_add_text() calls could be proto_tree_add_item() calls
- last parameter of proto_tree_add_item() is a gboolean, so either TRUE of FALSE
- Fuzztest them.
- And finally, add them to an enhancement bug.
- Protocol pages on the Wiki would be welcome documentation as well.

Thanks,
Jaap

Tobias Erichsen wrote:
> Hi Jaap,
>
> like I wrote in my earlier mail, I will be converting the AppleMIDI
> dissector to be builtin in the next couple of days.
>
> As the RTP-MIDI plugin is still work in progress (works perfectly
> already for standard-MIDI-data, but like I wrote does not fully
> decode more esoteric MIDI-message in all depths), I would prefer
> to keep that as plugin for the next time - would that be ok with
> the project?
>
> I will also fuzztest the plugins, have not done that yet.
>
> It would still be good if some of you guys who have done more work
> for Wireshark might take a small look at the code if that is in
> good shape for inclusion otherwise.
>
> Best regards,
> Tobias
>
> > -----Urspr�ngliche Nachricht-----
> > Von: wireshark-dev-bounces@xxxxxxxxxxxxx 
> > [mailto:wireshark-dev-bounces@xxxxxxxxxxxxx] Im Auftrag von 
> > Jaap Keuter
> > Gesendet: Sonntag, 31. Januar 2010 22:41
> > An: Developer support list for Wireshark
> > Betreff: Re: [Wireshark-dev] RTP-MIDI and AppleMIDI dissectors
> >
> > Hi,
> >
> > The way to do this is to file an enhancement bug, so it won't 
> > get lost.
> >
> > Two questions up front:
> > Did you fuzztest the code?
> > Can you work both as build in dissectors? That is really the 
> > prefered way.
> >
> > Thanx,
> > Jaap
> >
> > Send from my iPhone
> >
> > On 31 jan 2010, at 14:19, "Tobias Erichsen" <t.erichsen@xxxxxx> wrote:
> >
> > > Hi everyone,
> > >
> > > after my initial efforts in 2006, I have since then reworked my 
> > > Previous code and have been able to compile it with the source of 
> > > Wireshark 1.2.6 both for x86 and x64.
> > >
> > > RTP-MIDI:
> > > Dissector-plugin to decode MIDI-data which is transmitted via RTP 
> > > based on RFC-4695.  The dissector currently supports the standard- 
> > > MIDI-stuff, but enhancements for more "esoteric" MIDI-sub-standards 
> > > still could to be implemented...
> > >
> > > AppleMIDI:
> > > Dissector-plugin to decode the lightweight 
> > Apple-network-MIDI session 
> > > establishment protocol (which is used to establish an 
> > RTP-session to 
> > > send & receive MIDI-data via RFC-4695).  This protocol is 
> > proprietary.  
> > > The dissector is based on the Apple implementation summary from May 
> > > 2005.
> > >
> > > You will find attached two zip-files containing the source-code and 
> > > one zip-file containing a sample-capture with both the 
> > > Apple-session-protocol and RF-4695-data.  This capture was done 
> > > between a MacBook and a MIDI Kiss-Box
> > >
> > > I would be glad if some of you guys could take a look at 
> > the code and 
> > > give me some feedback if this is already in good shape to be 
> > > integrated into the official Wireshark-release.
> > >
> > > I would be willing to rework the AppleMIDI plugin to work 
> > as a builtin 
> > > dissector, as this part is fully done.
> > >
> > > The RTP-MIDI-dissector should stay a plugin for a while 
> > since I plan 
> > > to implement more enhancements over the next couple of months 
> > > (decoding of some more specialized MIDI-commands for stuff like 
> > > lighting-control etc.)
> > >
> > > Best regards,
> > > Tobias
> > > <RTP-MIDI_plugin.zip>
> > > <AppleMIDI_plugin.zip>
> > > <RTP-MIDI_AppleMIDI_data.zip>