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

[Michael Niedermayer]

On Tue, Mar 09, 2010 at 12:27:52AM +0530, Mohamed Naufal wrote:
> On 22 February 2010 01:40, Michael Niedermayer <michaelni at gmx.at> wrote:
> 
> > On Thu, Feb 18, 2010 at 05:35:59PM +0530, Mohamed Naufal wrote:
> > > On 15 January 2010 05:41, Michael Niedermayer <michaelni at gmx.at> wrote:
> > >
> > >
> > > [...]
> > >
> > >
> > > > > +typedef struct YopDecContext {
> > > > > +    AVFrame frame;
> > > > > +    AVCodecContext *avctx;
> > > > > +    AVPacket *avpkt;
> > > > > +
> > > > > +    int num_pal_colors;
> > > > > +    int first_color[2];
> > > > > +    int tag_needs_next_byte;
> > > >
> > > > > +    int current_tag_byte;
> > > >
> > > > you dont need that variable
> > > >
> > >
> > > If I don't increment s->srcptr in the if block of yop_get_next_nibble(),
> > > color_num in yop_paint_block() can have an incorrect value.
> >
> > you dont need the current_tag_byte variable
> >
> 
> 
> What I meant to say was if incrementing s->srcptr is postponed to the else
> block and yop_paint_block is called immediately after the if block, the
> local variable
> color_num = *(s->srcptr + paint_lut[tag][i])
> will have an incorrect value.
> Therefore s->srcptr is incremented in the if block and the original value
> stored in current_tag_byte.

you do not need the current_tag_byte variable, iam not speaking
about any increasing of any pointers


> 
> [...]
> 
> 
> > > > +    firstcolor = s->first_color[s->avctx->frame_number & 1];
> > > >
> > > > this will be wrong with seeking id suspect
> > > >
> > >
> > >
> > > How else do I do it?
> >
> > Either the demuxer marks only even frames as keyframe then the decoder
> > can when it detects seeking start counting from even
> > or
> > the demuxer has to indicate if a frame is even or odd to the decoder
> > also there is the question if they are keyframes or not, you obviosuly
> > should not mark then incorrectly
> >
> 
> Now checked using (pkt->pos - HDR_SIZE) / frame_size & 1
> Hope this is OK.

no, pos can be arbitrary after remuxing, you cannot use its value in the
decoder


> 
[...]
> >
> >
> > [...]
> > > +AVInputFormat yop_demuxer = {
> > > +    "yop",
> > > +    NULL_IF_CONFIG_SMALL("Psygnosis YOP Format"),
> > > +    sizeof(YopDecContext),
> > > +    yop_probe,
> > > +    yop_read_header,
> > > +    yop_read_packet,
> > > +    yop_read_close,
> > > +    .extensions = "yop",
> > > +    .flags = AVFMT_GENERIC_INDEX,
> > > +};
> >
> > thats missing code to flush the buffered packet in case of seeking
> >
> 
> Don't the buffered packets get freed in av_seek_frame_generic()?. I must be
> missing something. Anyway, the demuxer now uses a custom seeking function
> and seeking works without any problems.

you still aren freeing the packet, the generic code cannot free your privately
allocatd packet

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

The real ebay dictionary, page 2
"100% positive feedback" - "All either got their money back or didnt complain"
"Best seller ever, very honest" - "Seller refunded buyer after failed scam"
-------------- 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/20100310/f89d5172/attachment.pgp>