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

[Ronald S. Bultje]

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...

Thanks,
Ronald