[Dietfrid Mali <karx11erx@xxxxxxxxxxx>]

There are no problems with my implementation of the G.726 decoder.
It works fine the way I have delivered it in the bug ticket containing my related patch.

> Date: Wed, 26 Jan 2011 17:28:41 +0100
> From: jaap.keuter@xxxxxxxxx
> To: wireshark-dev@xxxxxxxxxxxxx
> Subject: Re: [Wireshark-dev] G.722 and G.726 decoders for Wireshark
>
> Hi,
>
> Well, Wireshark should pick up on SDP negotiations for your RTP stream,
> and set the correct payload type interpretation. I would have to look
> at
> the specific capture file to see what's happening.
>
> The fact that "The G.726 decoder function returns twice the number of
> bytes fed to it as output buffer size" is indeed to cause of your
> problems.
> You have decoded from 4 bit (ADPCM) samples to 8 bit (companded PCM)
> samples, while the player expects 16 bit (linear PCM) samples.
> So your output size should be four times your input size.
>
> Thanks,
> Jaap
>
> On Wed, 26 Jan 2011 09:23:58 +0100, Dietfrid Mali wrote:
>
> > Yeah, I wasn't expressing this quite right. What I basically wanted
> > to
> > say is that Wireshark
> > doesn't assign a payload type using the SDP but just forwards the
> > payload type
> > given in the RTP packets (102, which Wireshark wraps into a define
> > that
> > basically says
> > "undefined packet type").
> >
> >> 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.
> >>
> >
> > The G.726 decoder function returns twice the number of bytes fed to
> > it
> > as
> > output buffer size. Just returning that number leads to incorrect
> > audio
> > playback. I had to double that number once more to make it work. This
> > has something
> > to do with how rtp_player() determines whether it has received enough
> > packets
> > for the given time frame and that it inserts silence frames when it
> > thinks it hasn't.
> > There is no (inline) documentation of what rtp_player() expects.
> > Maybe
> > this is
> > described in some Wireshark programming or API documentation?
> >
> >> 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 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