[FFmpeg-devel] [PATCH] Make parser not favor packets with pts/dts (and related fixes)

[Michael Niedermayer]

On Wed, Mar 04, 2009 at 11:40:01PM +0100, Ivan Schreter wrote:
> Michael Niedermayer wrote:
> > On Wed, Mar 04, 2009 at 09:13:54PM +0100, Ivan Schreter wrote:
> >   
> >> Hi,
> >>
> >> Michael Niedermayer wrote:
> >>     
> >>> patch below does make the parser treat packets equal, <put some joke 
> >>> mixing
> >>> human and packet rights here>
> >>>
> >>> [...]
> >>> this should make it easier to feed pos in the parser as well
> >>> [...]
> >>>   
> >>>       
> >> Attached patches add support for storing packet position alongside dts/pts 
> >> in lavc and using it in lavf to determine correct frame position (provided 
> >> cur_pkt.pos is set correctly). They are prerequisite for seeking changes, 
> >> which rely on having AVPacket.pos set correctly.
> >>
> >> lavf patch causes regression in seek test (attached as well), since 
> >> positions are corrected. PTS/DTS is the same, so it seems perfectly OK.
> >>
> >>     
> >
> >   
> >> BTW, when committing changes, should the regression change go together in 
> >> one commit with the code which causes the regression, or separately?
> >>     
> >
> > regression changes should be in the comment that causes them to change so that
> > one can check out any revission and has a working "make test"
> >
> >   
> It was also my idea how it should work :-)
> 
> > [..]
> >   
> >>                  got_packet:
> >>                      pkt->duration = 0;
> >>                      pkt->stream_index = st->index;
> >>                      pkt->pts = st->parser->pts;
> >>                      pkt->dts = st->parser->dts;
> >> +                    if (st->parser->pos != AV_NOPTS_VALUE)
> >> +                        pkt->pos = st->parser->pos;
> >> +                    else
> >> +                        pkt->pos = st->cur_pkt.pos;
> >>     
> >
> > is this still needed?
> > if so why?
> >   
> Actually not anymore.
> 
> So this hunk should be OK:
> 
> @@ -967,12 +968,12 @@
> 
>                  /* return packet if any */
>                  if (pkt->size) {
> -                    pkt->pos = st->cur_pkt.pos;              // Isn't 
> quite accurate butclose.
>                  got_packet:
>                      pkt->duration = 0;
>                      pkt->stream_index = st->index;
>                      pkt->pts = st->parser->pts;
>                      pkt->dts = st->parser->dts;
> +                    pkt->pos = st->parser->pos;
>                      pkt->destruct = av_destruct_packet_nofree;
>                      compute_pkt_fields(s, st, st->parser, pkt);
> 
> Any other comments or shall I commit?

i think the rest is ok

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

There will always be a question for which you do not know the correct awnser.
-------------- 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/20090305/bcd6c626/attachment.pgp>