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

[Roberto Togni]

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.


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

> @@ -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) ?

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

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

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

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