[FFmpeg-devel] Fwd: [PATCH] Psygnosis YOP demuxer

[Michael Niedermayer]

On Fri, Mar 26, 2010 at 05:20:29AM +0530, Mohamed Naufal wrote:
> On 24 March 2010 07:51, Michael Niedermayer <michaelni at gmx.at> wrote:
> 
> 
> [...]
> 
> > you could put a 0 or 1 in AVPacket.data[0]
> > but this is ugly, so lets just keep the ugly marking of half of them as
> > keyframes.
> >
> >
> 
> The demuxer now marks pkt.data[0] as odd/even and all pkt.flags as key-frames.
> Doing this so that seeking using
> 
> frame_pos = timestamp * frame_size + pos_min;
> seek(frame_pos);
> 
> wouldn't be wrong.
> The first 4 B of pkt.data is unused anyway.
> 
> Also modified the decoder to output in PAL8.
> 
> Naufal
[...]
> +/**
> + * Paint a macroblock using the pattern in paint_lut.
> + * @param s codec context
> + * @param tag The tag that was in the nibble.
> + */
> +static void yop_paint_block(YopDecContext *s, int tag)
> +{
> +    int offset, color_num, i;
> +
> +    for (i = 0; i < 4; i++) {
> +        offset                = s->block_offset_lut[i];
> +        color_num             = *(s->srcptr + paint_lut[tag][i]);
> +        *(s->dstptr + offset) = color_num;
> +    }

you dot need the color_num nor offset variables and it probably makes sense
to unroll this loop as its rather speed relevant and small


> +    // The number of src bytes consumed is in the last part of the lut entry.
> +    s->srcptr += paint_lut[tag][4];
> +}
> +
> +/**
> + * Copy a previously painted macroblock to the current_block.
> + * @param copy_tag The tag that was in the nibble.
> + */
> +static void yop_copy_previous_block(YopDecContext *s, int copy_tag)
> +{
> +    uint8_t *bufptr;
> +    int offset, i;
> +
> +    // Calculate position for the copy source
> +    bufptr = s->dstptr + motion_vector[copy_tag][0] +
> +             s->frame.linesize[0] * motion_vector[copy_tag][1];
> +

> +    for (i = 0; i < 4; i++) {
> +        offset                = s->block_offset_lut[i];
> +        *(s->dstptr + offset) = *(bufptr + offset);
> +    }

s->dstptr[offset] looks nicer
and this loop too probably should be unrolled


> +}
> +
> +/**
> + * Return the next nibble in sequence, consuming a new byte on the input
> + * only if necessary.
> + */
> +static uint8_t yop_get_next_nibble(YopDecContext *s)
> +{
> +    int ret;
> +
> +    if (s->low_nibble) {
> +        ret           = *s->low_nibble & 0xf;
> +        s->low_nibble = NULL;
> +    }else {
> +        s->low_nibble = s->srcptr++;
> +        ret           = *s->low_nibble >> 4;
> +    }
> +    return ret;
> +}
> +

> +/**
> + * Take s->dstptr to the next macroblock in sequence.
> + */
> +static void yop_next_macroblock(YopDecContext *s)
> +{
> +    // If we are advancing to the next row of macroblocks
> +    if (s->row_pos == s->frame.linesize[0] - 2) {
> +        s->dstptr  += s->frame.linesize[0];

this looks like it assumes width == linesize
this is not guranteed by get_buffer()


[...]
> +    palette_size = 4 + yop->num_pal_colors * 3;
[...]
> +    int palette_size           = 4 + yop->num_pal_colors * 3;

duplicate


[...]
> +static int yop_read_seek(AVFormatContext *s, int stream_index,
> +                         int64_t timestamp, int flags)
> +{
> +    YopDecContext *yop = s->priv_data;
> +    int64_t frame_pos, pos_min, pos_max;
> +
> +    av_free_packet(&yop->video_packet);
> +
> +    pos_min = s->data_offset;
> +    pos_max = url_fsize(s->pb) - yop->frame_size;
> +
> +    if(!stream_index)
> +        return -1;
> +
> +    frame_pos      = timestamp * yop->frame_size + pos_min;
> +    yop->odd_frame = timestamp & 1;
> +
> +    if (frame_pos < pos_min) {
> +        frame_pos      = pos_min;
> +        yop->odd_frame = 0;
> +    }else if (frame_pos > pos_max) {
> +        frame_pos      = pos_max;
> +        yop->odd_frame = (pos_max - pos_min) /
> +                         yop->frame_size & 1;
> +    }

cliping timestamp instead of pos would make the special case handling of
odd_frame unneeded

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

I wish the Xiph folks would stop pretending they've got something they
do not.  Somehow I fear this will remain a wish. -- M?ns Rullg?rd
-------------- 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/20100326/9a096e3b/attachment.pgp>