[FFmpeg-devel] [PATCH][RFC] nsv seeking

Michael Niedermayer michaelni
Tue Apr 14 23:23:41 CEST 2009


On Tue, Apr 14, 2009 at 10:22:56PM +0530, Jai Menon wrote:
> On 3/20/09, Michael Niedermayer <michaelni at gmx.at> wrote:
> > On Mon, Mar 16, 2009 at 12:15:27PM +0530, Jai Menon wrote:
> >  > Hi,
> >  >
> >  > Attached patch adds seeking support for NSV files. I'm not sure if its
> >  > completely correct, hence the RFC ;). It works fine here and is an
> >  > improvement compared to the current situation.
> >  > Comments?
> >  >
> >  > As a side note, the whole file could use a cleanup ;)
> >
> >
> > [...]
> >  > @@ -347,12 +348,22 @@
> >  >      PRINT(("NSV got infos; filepos %"PRId64"\n", url_ftell(pb)));
> >  >
> >  >      if (table_entries_used > 0) {
> >  > +        int i;
> >  >          nsv->index_entries = table_entries_used;
> >  >          if((unsigned)table_entries >= UINT_MAX / sizeof(uint32_t))
> >  >              return -1;
> >  > -        nsv->nsvf_index_data = av_malloc(table_entries * sizeof(uint32_t));
> >  > -#warning "FIXME: Byteswap buffer as needed"
> >  > -        get_buffer(pb, (unsigned char *)nsv->nsvf_index_data, table_entries * sizeof(uint32_t));
> >  > +        nsv->nsvf_index_data = av_malloc(table_entries_used * sizeof(uint32_t));
> >
> >  integer overflow leading to heap buffer overflow
> 
> modified
> 
> >  > @@ -453,6 +465,16 @@
> >  >              av_set_pts_info(st, 64, framerate.den, framerate.num);
> >  >              st->start_time = 0;
> >  >              st->duration = av_rescale(nsv->duration, framerate.num, 1000*framerate.den);
> >  > +
> >  > +            for(i=0;i<nsv->index_entries;i++) {
> >  > +                if(nsv->extended_index_data) {
> >  > +                    av_add_index_entry(st, nsv->nsvf_index_data[i], nsv->extended_index_data[i],
> >  > +                                       0, 0, AVINDEX_KEYFRAME);
> >  > +                } else {
> >  > +                    int64_t ts = av_rescale((i/nsv->index_entries)*nsv->duration, framerate.num, 1000*framerate.den);
> >  > +                    av_add_index_entry(st, nsv->nsvf_index_data[i], ts, 0, 0, AVINDEX_KEYFRAME);
> >  > +                }
> >  > +            }
> >  >          }
> >  >          if (atag != T_NONE) {
> >  >  #ifndef DISABLE_AUDIO
> >
> >  is there a reason for the temp table? i mean cant they be droped
> >  in av_add_index_entry() directly?
> 
> the toc is read while file header and metadata is read so there are no
> streams at this point of time. or am missing something?
> 
> >  > @@ -598,6 +620,11 @@
> >  >          pkt->stream_index = st[NSV_ST_VIDEO]->index;//NSV_ST_VIDEO;
> >  >          pkt->dts = nst->frame_offset;
> >  >          pkt->flags |= nsv->state == NSV_HAS_READ_NSVS ? PKT_FLAG_KEY : 0; /* keyframe only likely on a sync frame */
> >  > +
> >  > +        if(!nsv->nsvf_index_data) {
> >  > +            av_add_index_entry(st[NSV_ST_VIDEO], url_ftell(pb) - vsize, nst->frame_offset,
> >  > +                               0, 0, nsv->state == NSV_HAS_READ_NSVS ? AVINDEX_KEYFRAME : 0);
> >  > +        }
> >  >  /*
> >  >          for (i = 0; i < MIN(8, vsize); i++)
> >  >              PRINT(("NSV video: [%d] = %02x\n", i, pkt->data[i]));
> >
> >  > @@ -683,15 +710,25 @@
> >  >
> >  >  static int nsv_read_seek(AVFormatContext *s, int stream_index, int64_t timestamp, int flags)
> >  >  {
> >  > -#if 0
> >
> >  ideally this should use the new seeking api
> 
> when it does end up in svn, i'll rtfs and look at this again :)
> updated patch attached.
[...]
> @@ -453,6 +465,16 @@ static int nsv_parse_NSVs_header(AVFormatContext *s, AVFormatParameters *ap)
>              av_set_pts_info(st, 64, framerate.den, framerate.num);
>              st->start_time = 0;
>              st->duration = av_rescale(nsv->duration, framerate.num, 1000*framerate.den);
> +
> +            for(i=0;i<nsv->index_entries;i++) {
> +                if(nsv->extended_index_data) {
> +                    av_add_index_entry(st, nsv->nsvf_index_data[i], nsv->extended_index_data[i],

i think nsvf_index_data and extended_index_data are poor names
nsvf is in a nsv file demuxer redundant
index_data is a little generic, is it time? file position?  ...


[...]
> +        if(nsv->extended_index_data) {
> +            av_freep(&nsv->extended_index_data);
> +        }

the if() is redundant

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

Democracy is the form of government in which you can choose your dictator
-------------- 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/20090414/70194719/attachment.pgp>



More information about the ffmpeg-devel mailing list