[FFmpeg-devel] [PATCH] RDT/Realmedia patches #2

[Michael Niedermayer]

On Sun, Nov 16, 2008 at 04:06:06PM -0500, Ronald S. Bultje wrote:
> Hi,
> 
> On Sun, Nov 16, 2008 at 3:58 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> > On Sun, Nov 16, 2008 at 02:14:17PM -0500, Ronald S. Bultje wrote:
> >> On Sun, Nov 16, 2008 at 1:25 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> >> >> @@ -178,7 +179,8 @@
> >> >>                      int *pset_id, int *pseq_no, int *pstream_id,
> >> >>                      int *pis_keyframe, uint32_t *ptimestamp)
> >> >>  {
> >> >> -    int consumed = 10;
> >> >> +    GetBitContext gb;
> >> >> +    int consumed = 0;
> >> >>
> >> >>      /* skip status packets */
> >> >>      while (len >= 5 && buf[1] == 0xFF /* status packet */) {
> >> >> @@ -245,13 +247,18 @@
> >> >>       * [2] http://www.wireshark.org/docs/dfref/r/rdt.html and
> >> >>       *     http://anonsvn.wireshark.org/viewvc/trunk/epan/dissectors/packet-rdt.c
> >> >>       */
> >> >> -    if (pset_id)      *pset_id      = (buf[0]>>1) & 0x1f;
> >> >> -    if (pseq_no)      *pseq_no      = AV_RB16(buf+1);
> >> >> -    if (ptimestamp)   *ptimestamp   = AV_RB32(buf+4);
> >> >> -    if (pstream_id)   *pstream_id   = (buf[3]>>1) & 0x1f;
> >> >> -    if (pis_keyframe) *pis_keyframe = !(buf[3] & 0x1);
> >> >> +    init_get_bits(&gb, buf, len << 3);
> >> >> +    skip_bits(&gb, 2);
> >> >> +    if (pset_id)      *pset_id      = get_bits(&gb, 5);
> >> >> +    skip_bits(&gb, 1);
> >> >> +    if (pseq_no)      *pseq_no      = get_bits(&gb, 16);
> >> >> +    skip_bits(&gb, 2);
> >> >> +    if (pstream_id)   *pstream_id   = get_bits(&gb, 5);
> >> >> +    if (pis_keyframe) *pis_keyframe = get_bits1(&gb);
> >> >> +    if (ptimestamp)   *ptimestamp   = get_bits_long(&gb, 32);
> >> >> +    skip_bits(&gb, 16);
> >> >
> >> > this code is not equivalent, and i suspect the new does not work
> >>
> >> I tested it and it worked (i.e. I played a rtsp:// stream from a Real
> >> server and I heard sound coming out of my speakers). Maybe not the
> >> best test... Could you explain which parts would not be equivalent?
> >> The only one I see is the is_keyframe flag (sorry, fixed in attached).
> >
> > the bitstream reading is under if() if the if is false the bit position
> > will be wrong
> 
> aha, of course. OK, so would you suggest using temporary variables
> anywhere or use else skip_bits()?
> 
> So:
> 
> void function ( [..], int *pis_key, [..])
> {
> int is_key;
> [..]
> is_key = !get_bits1(&gb);
> [..]
> if (pis_key) *pis_key = is_key;
> 
> or
> 
> void function ( [..], int *pis_key, [..])
> {
> [..]
> if (pis_key) *pis_key = !get_bits1(&gb);
> else skip_bits1(&gb);
> 
> Or is there a better option? I kind of dislike both solutions...

i prefer the temporary vars

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

No human being will ever know the Truth, for even if they happen to say it
by chance, they would not even known they had done so. -- Xenophanes
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20081116/7ef86243/attachment.pgp>