[FFmpeg-devel] [RFC] Feed whole frames to RV* decoders

Kostya kostya.shishkov
Tue Oct 9 07:10:52 CEST 2007


On Tue, Oct 09, 2007 at 12:18:59AM +0200, Roberto Togni wrote:
> On Tue, 2 Oct 2007 08:02:59 +0300
> Kostya <kostya.shishkov at gmail.com> wrote:
> 
> > Here is an updated working version of gathering complete frames
> > for Real Video decoders. Both RM and Matroska demuxers are passing
> > whole frames to RealVideo decoder (RV10/20 is patched to accept
> > this format).
> > 
> > Please comment. 
> 
> Sorry for the late review. I haven't tested the patch, but look mostly
> ok. See below.
> 
> As a side note, the format chosen for the video frame is compatible
> with matroska, files but does not contain the frame timestamp from the
> data packet header; unless the binary codecs can work without it the
> demuxer will be incompatible with them.

Looks like they can (there is an additional timestamp in each slice header).  
And it would be easier to construct data for binary decoder from this
(for now I see hack to pass data from demux_real to vd_realvid).

> > Index: libavformat/rmdec.c
> > ===================================================================
> > --- libavformat/rmdec.c	(revision 10505)
> > +++ libavformat/rmdec.c	(working copy)
> > @@ -364,6 +364,10 @@
> >  
> >      n = get_be16(pb);
> >      (*len)-=2;
> > +    if (n >= 0x8000) {
> > +        av_log(NULL,0,"Got number %X\n",n);
> > +        n -= 0x8000;
> > +    }
> >      if (n >= 0x4000) {
> >          return n - 0x4000;
> >      } else {
> 
> Maybe if(n & 0xc000) return n & 0x3fff; your version is ok too if you
> prefer it, but I'd remove the av_log().

In my development version I set flag 'truncated' depending on it and merge
last slices if needed. 
 
> > @@ -432,6 +436,112 @@
> >      return -1;
> >  }
> >  
> > +static int rm_assemble_frame(AVFormatContext *s, RMContext *rm,
> > ByteIOContext *pb, AVPacket *pkt, int len) +{
> > +    int hdr, type, seq, pic_num, pkt_len, pos, frame_length, ret;
> > +    int slices, slice_count;
> > +    uint8_t *slice_hdr, *slice_data, *slice_data_start, *data_end;
> > +    int flags, idx;
> > +    int64_t timestamp, pos2;
> > +
> > +    hdr = get_byte(pb); len--;
> > +    type = hdr >> 6;
> > +
> > +    switch (type) {
> > +    case 0: // partial frame start
> > +    case 2: // partial frame end - should not happen here
> > +        seq = get_byte(pb); len--;
> > +        pkt_len = get_num(pb, &len);
> > +        pos = get_num(pb, &len);
> > +        pic_num = get_byte(pb); len--;
> > +        frame_length = len;
> > +        slices = (((hdr << 8) | seq) >> 7) & 0x7F;
> 
> Shouldn't this be (hdr << 1) ?

it will require different parenthesis placement around seq but it is the same
 
> > +        slices++; //sometimes first slice report one less slice that
> > consequent slices
> 
> Are you sure about the slice number calculation? MPlayer demuxer does
> (hdr & 0x3f)*2, and the number of slices is always one less than the
> real number.

You can't be sure with RM format
>From http://www.bunkus.org/videotools/librmff/doc/index.html
Partial packet format is AABBBBBB BCCCCCCC where AA - packet type,
BBBBBBB - number of slices and CCCCCCC - number of current slice.

So MPlayer demuxer just ignores that last bit anyway.

For some reason in half of the cases number of slices indicated in subsequent slices
is greater by one than in the first slice.

> > +        break;
> > +    case 1: // whole frame
> > +        pic_num = get_byte(pb); len--;
> > +        pos = 0;
> > +        frame_length = pkt_len = len;
> > +        slices = 1;
> > +        break;
> > +    case 3: // one of multiple frames in one packet
> > +        pkt_len = get_num(pb, &len);
> > +        pos = get_num(pb, &len);
> > +        pic_num = get_byte(pb); len--;
> > +        frame_length = pkt_len;
> > +        slices = 1;
> > +        break;
> > +    }
> > +    
> > +    if (type == 2) //frame tail should not be met here
> > +        return -1;
> > +
> > +    if(av_new_packet(pkt, pkt_len + slices * 8 + 1))
> > +        return AVERROR(ENOMEM);
> > +    slice_hdr = pkt->data + 1;
> > +    slice_data = slice_data_start = slice_hdr + slices * 8;
> > +    data_end = pkt->data + pkt_len + slices * 8 + 1;
> > +   
> > +    pkt->data[0] = slices - 1;
> > +    
> > +    AV_WL32(slice_hdr + 0, 1);
> > +    AV_WL32(slice_hdr + 4, 0);
> > +    slice_hdr += 8;
> > +    ret = get_buffer(pb, slice_data, frame_length);
> > +    if (ret != frame_length) {
> > +         av_free_packet(pkt);
> > +         return AVERROR(EIO);
> > +    }
> > +    slice_data += frame_length;
> > +    slice_count = 1;
> > +    rm->remaining_len = len - frame_length;
> > +    while(slice_data < data_end && slice_count < slices) {
> > +        len=sync(s, &timestamp, &flags, &idx, &pos2);
> 
> This will work only if there are no audio packet mixed with the parts
> of the video frame, but I guess this is ok for real vorld, video
> packets were grouped together in the files I checked.

Well, at least 20040506.rm has audio packet stored between packets for
the first video frame so different approach is needed.
 
> > +        if (len<0) {
> > +            av_free_packet(pkt);
> > +            return AVERROR(EIO);
> > +        }
> > +
> > +        hdr = get_byte(pb); len--;
> > +        type = hdr >> 6;
> > +        seq = get_byte(pb); len--;
> > +        pkt_len = get_num(pb, &len);
> > +        pos = get_num(pb, &len);
> > +        pic_num = get_byte(pb); len--;
> > +        
> > +        if (type == 2) {
> > +            rm->remaining_len = len - pos;
> > +            frame_length = pos;
> > +        } else {
> > +            frame_length = len;
> > +        }
> > +        if (slice_data + frame_length > data_end) {
> > +            av_free_packet(pkt);
> > +            return AVERROR(EIO);
> > +        }
> > +        
> > +        AV_WL32(slice_hdr + 0, 1);
> > +        AV_WL32(slice_hdr + 4, slice_data - slice_data_start);
> > +        slice_hdr += 8;
> > +
> > +        ret = get_buffer(pb, slice_data, frame_length);
> > +        if (ret != frame_length) {
> > +            av_free_packet(pkt);
> > +            return AVERROR(EIO);
> > +        }
> > +        slice_data += frame_length;
> > +        slice_count++;
> > +        if (type == 2) 
> > +            break;
> > +    }
> > +    if(slice_count < slices){
> > +        pkt->data[0] = slice_count - 1;
> > +        memmove(slice_hdr, slice_hdr+8*(slices - slice_count),
> > pkt->size - 1 - 8*slices);
> > +        pkt->size -= 8*(slices - slice_count);
> > +    }
> 
> If the slice number thing can be solved this can probably be removed.

I use it also for truncated slice merging.
 
> > +    return 0;
> > +}
> > +
> >  static int rm_read_packet(AVFormatContext *s, AVPacket *pkt)
> >  {
> >      RMContext *rm = s->priv_data;
> [...]
> 
> There is no error checking for out of sequence slices, incomplete
> packets or similar things, but those can be added later.
> 
> 
> Ciao,
>  Roberto
> 
> -- 
> Better is the enemy of good enough.




More information about the ffmpeg-devel mailing list