["Kukosa, Tomas" <tomas.kukosa@xxxxxxxxxxxxxxxxxxxxxx>]

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