Wireshark · Wireshark-dev: Re: [Wireshark-dev] G.722 and G.726 decoders for Wireshark

[Dietfrid Mali <karx11erx@xxxxxxxxxxx>]

I'll stick with hard coded for the time being then. I just followed the implementation of other hard coded codecs anyway.

> From: tomas.kukosa@xxxxxxxxxxxxxxxxxxxxxx
> To: wireshark-dev@xxxxxxxxxxxxx
> Date: Thu, 27 Jan 2011 15:05:45 +0100
> Subject: Re: [Wireshark-dev] G.722 and G.726 decoders for Wireshark
>
> Hi,
>
> for the first step I would stay with 8kHz audio format and rtp_palyer can be extended for other rates in the next step.
>
> Current implementation works just for assigned payload types (not dynamic) and codec has to be registered with name from rtp_payload_type_short_vals table (see epan/dissectors/packet-rtp.c)
>
> As G.726 uses dynamic payload type it could be registered with encoding name (G726-32) and RTP player needs to get information about dynamic payload type from RTP conversation (it is not implemented now).
>
> ________________________________
>
> From: wireshark-dev-bounces@xxxxxxxxxxxxx [mailto:wireshark-dev-bounces@xxxxxxxxxxxxx] On Behalf Of Dietfrid Mali
> Sent: Thursday, January 27, 2011 11:22 AM
> To: wireshark-dev@xxxxxxxxxxxxx
> Subject: Re: [Wireshark-dev] G.722 and G.726 decoders for Wireshark
>
>
> What about G.722 delivering a 16 khz audio stream, and not 8 khz as is hardcoded in rtp_player.c's device opening call to PortAudio currently?
>
> Where is an explanation about the string tags used in register_codec_module? Can I use something like "g726_32"?
>
> > From: tomas.kukosa@xxxxxxxxxxxxxxxxxxxxxx
> > To: wireshark-dev@xxxxxxxxxxxxx
> > Date: Thu, 27 Jan 2011 10:02:37 +0100
> > Subject: Re: [Wireshark-dev] G.722 and G.726 decoders for Wireshark
> >
> > Hi,
> >
> > I would recommend to use codec table (see epan/codecs.h) for new codecs instead of including them hardcoded into the rtp_player.
> > It should not require any changes in the rtp_player for any new codec.
> > Only changes for dynamic payload handling should be implemented but it is common for all codecs.
> >
> > The codec table is used just for codec plugins now but using for internal Wireshak codec would be good too and does not need many changes (I hope ;-).
> >
> > Example of G.722 implemented inside plugin (using external library) is here:
> > http://anonsvn.wireshark.org/viewvc/trunk/plugins/easy_codec/easy_codec_plugin.c?view=markup
> > http://anonsvn.wireshark.org/viewvc/trunk/plugins/easy_codec/codec-g722.c?view=markup
> >
> > Wireshak internal implementation using spandsp library could be very similar.
> >
> > Best regards,
> > Tomas
> >
> >
> > -----Original Message-----
> > From: wireshark-dev-bounces@xxxxxxxxxxxxx [mailto:wireshark-dev-bounces@xxxxxxxxxxxxx] On Behalf Of Jaap Keuter
> > Sent: Wednesday, January 26, 2011 10:22 PM
> > To: Developer support list for Wireshark
> > Subject: Re: [Wireshark-dev] G.722 and G.726 decoders for Wireshark
> >
> > Hi,
> >
> > Found it, I'll have a look.
> >
> > Thanks,
> > Jaap
> >
> > On 01/26/2011 04:59 PM, Dietfrid Mali wrote:
> > > G.722 and G.726 (-32) codec integration using spandsp:
> > > https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=5619
> > >
> > > > Date: Wed, 26 Jan 2011 00:09:50 +0100
> > > > From: jaap.keuter@xxxxxxxxx
> > > > To: wireshark-dev@xxxxxxxxxxxxx
> > > > Subject: Re: [Wireshark-dev] G.722 and G.726 decoders for Wireshark
> > > >
> > > > On 01/25/2011 05:48 PM, Dietfrid Mali wrote:
> > > > > The problem with e.g. G.726 is that Wireshark gives those packets RTP
> > > > > type 102 which afaik is an error code ("unknown encoding").
> > > >
> > > > No, that's your RTP endpoint configured to label these as such. RFC
> > > 3550 says:
> > > > "A profile MAY specify a default static mapping of payload type codes
> > > to payload
> > > > formats. Additional payload type codes MAY be defined dynamically
> > > through
> > > > non-RTP means (see Section 3)."
> > > > RFC 1890/RFC 3551 defines the "RTP Profile for Audio and Video
> > > Conferences with
> > > > Minimal Control", which lists several static payload types. The old
> > > RFC lists
> > > > G.721 (aka G.726-32), while the new one dropped that one and added
> > > references to
> > > > G.726 at various bit rate, with a dynamic payload type.
> > > > RFC 3550 says in Section 3: "Non-RTP means: Protocols and mechanisms
> > > that may be
> > > > needed in addition to RTP to provide a usable service. In particular,
> > > ..., and
> > > > define dynamic mappings between RTP payload type values and the
> > > payload formats
> > > > they represent for formats that do not have a predefined payload type
> > > value."
> > > > with reference to Session Description Protocol (SDP)
> > > >
> > > > So, payload type 102 is a dynamic payload type which has to be given
> > > meaning
> > > > (through SDP for instance) within the session. In your case Wireshark
> > > didn't
> > > > pick that up from the trace, hence cannot give you the proper
> > > interpretation of
> > > > that payload type within that session.
> > > >
> > > > > I would need to know where and how Wireshark maps dynamic payload types
> > > > > (negotiated via SDP) to internal static ones. Above that RFC3551
> > > notes that
> > > > > static G.726 payload types are obsolete, and afaik there aren't even
> > > > > (obsolete) static payload types for all G.726 variants, so Wireshark
> > > > > would need to
> > > > > take care of that by using some (more or less arbitrary) internal
> > > static
> > > > > type numbers.
> > > >
> > > > Yep, that is done by the SDP dissector. It tries to interpret the SDP
> > > offer
> > > > (should be the answer, but that a whole other story) and create
> > > conversations
> > > > (see doc/README.developer, section 2.2) for the RTP dissector,
> > > feeding it
> > > > dynamic payload type information it has learned from the media
> > > attributes.
> > > >
> > > > The RTP dissector does then the heavy lifting on the RTP packets,
> > > based on the
> > > > information feed in by the SDP dissector.
> > > >
> > > > > I will do my best to provide a patch once I have fully integrated all
> > > > > codecs (currently only G.726-32 has been implemented as proof of
> > > > > concept, but since
> > > > > this is working adding more is no big deal).
> > > >
> > > > Just one to get started is fine. Does it integrate into codecs/
> > > directory
> > > > besides G711a and G711u (and G729 and G723, if you have them)?
> > > >
> > > > > Getting G.726 to work was a bit of a pain btw because of the weird
> > > frame
> > > > > sync calculation in rtp_player.c::play_channels() as this function
> > > seems to
> > > > > assume 1:1 relationships of decoder input and output stream sizes and
> > > > > thus simply halves the decoder output batch sizes to determine whether
> > > > > frames
> > > > > are properly sync'd. This doesn't work for compressed audio. To
> > > > > compensate, my decodeG726_32() function doubles the number of bytes
> > > returned
> > > > > (as it has a 1:2 relationship of input and output buffer sizes). Before
> > > > > it did that, lots of silence frames were inserted and half of the audio
> > > > > data was
> > > > > dropped by the player.
> > > >
> > > > I'm no sure if I understand you correctly. Working with these decode
> > > functions
> > > > there is an input buffer with its length as input, and two output
> > > parameters,
> > > > being the output buffer and it a pointer to store its size. This size
> > > of the
> > > > output buffer has to be set, by the decoder, to the number of samples
> > > in output
> > > > buffer. That should be enough, see for instance
> > > rtp_player.c:decode_rtp_packet()
> > > > the handling of G.279 and G.723.
> > > > Be aware that you have to store 16 bit linear samples in the output
> > > buffer,
> > > > maybe that's your factor 2?
> > > >
> > > > Thanks,
> > > > Jaap
> > > >
> > > >
> > > > >
> > > > > Dietfrid
> > > > >
> > > > > > From: jaap.keuter@xxxxxxxxx
> > > > > > Date: Tue, 25 Jan 2011 16:54:25 +0100
> > > > > > To: wireshark-dev@xxxxxxxxxxxxx
> > > > > > Subject: Re: [Wireshark-dev] G.722 and G.726 decoders for Wireshark
> > > > > >
> > > > > > Hi,
> > > > > >
> > > > > > That would be interesting. Can you put the code in a patch on
> > > bugzilla?
> > > > > >
> > > > > > Can't work on it right now, but would be nice to have.
> > > > > >
> > > > > > btw: their are already static RTP types assigned for both codecs. The
> > > > > dynamic types should come in through protocols like SDP, or a dissector
> > > > > preference.
> > > > > >
> > > > > > Thanks,
> > > > > > Jaap
> > > > > >
> > > > > > Send from my iPhone
> > > > > >
> > > > > > On 25 jan. 2011, at 16:07, Dietfrid Mali <karx11erx@xxxxxxxxxxx>
> > > wrote:
> > > > > >
> > > > > > > Hi,
> > > > > > >
> > > > > > > using spandsp, I have added G.722 and G.726 decoders to Wireshark.
> > > > > > >
> > > > > > > Currently this is a bit of a hack job, particularly regarding
> > > > > inclusion of the spandsp lib, and I could need a bit help to properly
> > > > > integrate it into Wireshark's automake hell (configure.in).
> > > > > > >
> > > > > > > There also isn't a proper Wireshark signature for that RTP type (I
> > > > > am simply reacting to RTP type 102, which actually is an error
> > > code), so
> > > > > some help getting this straight and introducing proper codec types
> > > would
> > > > > be appreciated, too.
> > > > > > >
> > > > > > >
> > > > > > >
> >
> > ___________________________________________________________________________
> > Sent via: Wireshark-dev mailing list <wireshark-dev@xxxxxxxxxxxxx>
> > Archives: http://www.wireshark.org/lists/wireshark-dev
> > Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
> > mailto:wireshark-dev-request@xxxxxxxxxxxxx?subject=unsubscribe
> > ___________________________________________________________________________
> > Sent via: Wireshark-dev mailing list <wireshark-dev@xxxxxxxxxxxxx>
> > Archives: http://www.wireshark.org/lists/wireshark-dev
> > Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
> > mailto:wireshark-dev-request@xxxxxxxxxxxxx?subject=unsubscribe
>
> ___________________________________________________________________________
> Sent via: Wireshark-dev mailing list <wireshark-dev@xxxxxxxxxxxxx>
> Archives: http://www.wireshark.org/lists/wireshark-dev
> Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
> mailto:wireshark-dev-request@xxxxxxxxxxxxx?subject=unsubscribe