[FFmpeg-devel] [PATCH] nuvdemux: timestamps are signed or well wraped and starting before 0.

[Reimar Döffinger]

On Sun, Nov 25, 2012 at 01:43:07PM +0100, Michael Niedermayer wrote:
> On Sun, Nov 25, 2012 at 12:48:09PM +0100, Reimar Döffinger wrote:
> > On Sun, Nov 25, 2012 at 05:02:30AM +0100, Michael Niedermayer wrote:
> > > Signed-off-by: Michael Niedermayer <michaelni at gmx.at>
> > > ---
> > >  libavformat/nuv.c            |    4 +-
> > >  tests/ref/fate/nuv-rtjpeg-fh |  100 +++++++++++++++++++++---------------------
> > >  2 files changed, 52 insertions(+), 52 deletions(-)
> > > 
> > > diff --git a/libavformat/nuv.c b/libavformat/nuv.c
> > > index c67c1fa..2b2f22c 100644
> > > --- a/libavformat/nuv.c
> > > +++ b/libavformat/nuv.c
> > > @@ -229,7 +229,7 @@ static int nuv_packet(AVFormatContext *s, AVPacket *pkt) {
> > >  
> > >                  pkt->pos = pos;
> > >                  pkt->flags |= hdr[2] == 0 ? AV_PKT_FLAG_KEY : 0;
> > > -                pkt->pts = AV_RL32(&hdr[4]);
> > > +                pkt->pts = (int32_t)AV_RL32(&hdr[4]);
> > 
> > Maybe I am overly pedantic, but isn't this cast strictly speaking
> > undefined? It seems to me we use (e.g. in dsicinav.c) constructs like
> > sign_extend(AV_RL16(buf), 16); instead of casts.
> > Also a test-case/explanation would be nice, because the FATE reference
> > at first sight looks a lot worse, not better and it's fairly non-obvious
> > why this cases the timestamps to be offset by 80...
> 
> in the fate sample
> one stream starts with timestamps a bit below 1<<32 and then wraps
> the other stream starts later after the wrap
> The fate test discards the odd stream so only the stream that starts
> later is shown. (its probably a bad idea to discard that stream)

<rant>
I never understood the point in making separate audio and video tests,
it only reduced coverage for no benefit.
</rant>

> The difference results from different start_time values.
> 
> also see the timestamp wraping patch that results in a similar change
> to this fate sample

Yeah, clear enough.
However I have one more concern: will things still work fine when the
timestamps really wrap after changing to signed?
I suspect NUV TV recordings might get really large in some rare cases.
But I don't have any objections if you think that's the best thing to
do.