Re: [FFmpeg-devel] [PATCH] avformat/flacdec: support fast-seek
2015-10-04 7:37 GMT+08:00 Michael Niedermayer <michae...@gmx.at>:
> On Sat, Oct 03, 2015 at 01:14:26AM +0800, Ching-Yi Chan wrote:
>
> iam not sure changing the format flags is a great idea, i think no
> other demuxer does that
> that said, the documentation does not say that only the user can
> change them so this is more a note that this looks a bit odd not that
> it is wrong
>
> using a context to keep found_seektable state.
> > +
> > +reset_index_position(avio_tell(s->pb), st);
> > return 0;
> >
> > fail:
> > @@ -249,12 +283,33 @@ static av_unused int64_t
> flac_read_timestamp(AVFormatContext *s, int stream_inde
> > return pts;
> > }
> >
> > +static int flac_seek(AVFormatContext *s, int stream_index, int64_t
> timestamp, int flags) {
> > +int index;
> > +int64_t pos;
> > +AVIndexEntry e;
> > +if (!(s->flags_FLAG_FAST_SEEK)) {
> > +return -1;
> > +}
> > +
> > +index = av_index_search_timestamp(s->streams[0], timestamp, flags);
> > +if(index<0 || index >= s->streams[0]->nb_index_entries)
> > +return -1;
> > +
> > +e = s->streams[0]->index_entries[index];
> > +pos = avio_seek(s->pb, e.pos, SEEK_SET);
> > +if (pos >= 0) {
>
> > +return pos;
>
> if pos is larger than INT_MAX the this can overflow and be interpreted
> as an error by the caller
>
>
> fix it by returning 0
0001-avformat-flacdec-support-fast-seek.patch
Description: Binary data
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/flacdec: support fast-seek
I found the problem is no seektable in our test sample acodec-flac.flac,
but application will append the new index entry to AVStream.
So, I need to a variable or some place to mark "no seektable, don't use
fast-seek method", some choices:
1. disable AVFMT_FLAG_FAST_SEEK flag when no seektable
2. use priv_class and keep the state in it
3. use a static varible to keep the state in flacdec.c
Need a suggestion for implementation.
2015-10-01 20:31 GMT+08:00 Michael Niedermayer <michae...@gmx.at>:
> On Fri, Sep 25, 2015 at 11:58:31AM +0800, Ching-Yi Chan wrote:
> > Thanks for checking.
> >
> > I also check the AVFMT_FLAG_FAST_SEEK flag with parsing headers,
> > to fill the seektable into index entries only if AVFMT_FLAG_FAST_SEEK
> flag
> > is on.
> >
> > If the AVFMT_FLAG_FAST_SEEK flag is not enabled, it will seek in original
> > way.
>
> i think we misunderstand us somehow
>
> your code does not work for me
> seeking to 1.894167 results in a seek to
> pts: 0.00 pos: 8256
>
> seeking to 1.470835 results in a seek to
> pts: 0.809796 pos: 27366
>
> you notice that moving the target to a later point moves the result
> to a earlier one.
>
> please see below for further comments
>
> [...]
>
> > +#define SEEKPOINT_SIZE 18
> > +
> > +static void ff_reset_index_position(int64_t metadata_head_size,
> AVStream *st)
>
> the ff_ prefix is for non static functions
>
>
> [...9
>
> > @@ -249,12 +277,30 @@ static av_unused int64_t
> flac_read_timestamp(AVFormatContext *s, int stream_inde
> > return pts;
> > }
> >
> > +static int flac_seek(AVFormatContext *s, int stream_index, int64_t
> timestamp, int flags) {
> > +if (!(s->flags_FLAG_FAST_SEEK)) {
> > +return -1;
> > +}
> > +
> > +int index = av_index_search_timestamp(s->streams[0], timestamp,
> flags);
>
> libavformat/flacdec.c:285:5: warning: ISO C90 forbids mixed declarations
> and code [-Wdeclaration-after-statement]
> libavformat/flacdec.c:289:5: warning: ISO C90 forbids mixed declarations
> and code [-Wdeclaration-after-statement]
>
> please make sure your patch adds no compiler warnings
>
>
> > +if(index<0 || index >= s->streams[0]->nb_index_entries)
> > +return -1;
> > +
> > +AVIndexEntry e = s->streams[0]->index_entries[index];
>
> > +int ret = avio_seek(s->pb, e.pos, SEEK_SET);
>
> this is wrong and must use int64_t otherwise the following >= 0 check
> can be wrong
>
> [...]
>
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Democracy is the form of government in which you can choose your dictator
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/flacdec: support fast-seek
Here is a new patch: 1. fix compilation warning 2. remove ff_ prefix on my patch 3. toggle AVFMT_FLAG_FAST_SEEK when no seektalbe in the flac metadata (this will disable flac_seek when no seekpoint) 0001-avformat-flacdec-support-fast-seek.patch Description: Binary data ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/flacdec: support fast-seek
2015-10-01 20:31 GMT+08:00 Michael Niedermayer <michae...@gmx.at>: > On Fri, Sep 25, 2015 at 11:58:31AM +0800, Ching-Yi Chan wrote: > > Thanks for checking. > > > > I also check the AVFMT_FLAG_FAST_SEEK flag with parsing headers, > > to fill the seektable into index entries only if AVFMT_FLAG_FAST_SEEK > flag > > is on. > > > > If the AVFMT_FLAG_FAST_SEEK flag is not enabled, it will seek in original > > way. > > i think we misunderstand us somehow > > your code does not work for me > seeking to 1.894167 results in a seek to > pts: 0.00 pos: 8256 > > seeking to 1.470835 results in a seek to > pts: 0.809796 pos: 27366 > > you notice that moving the target to a later point moves the result > to a earlier one. > > oops ! May I have a copy about the tests code. for improving my patch. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/flacdec: support fast-seek
Hi Hendrik, I move the index data into AVStream, but seeking is slow. (it try to download many data). Is there something wrong ? 2015-09-24 16:09 GMT+08:00 Hendrik Leppkes <h.lepp...@gmail.com>: > On Thu, Sep 24, 2015 at 9:08 AM, Ching-Yi Chan <chingyichan...@gmail.com> > wrote: > > flacdec is not supported fast-seek yet. > > > > changes: > > > > 1. parse the seekpoint from metadata block > > 2. add seek callback to look up seek position > > > > You should store the index in the AVStream index_entries field, that > way you don't need to duplicate the structure in the private flac > context, and the generic seeking code can make use of them without you > needing a specialized seekign function. > > - Hendrik > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > 0001-avformat-flacdec-support-fast-seek.patch Description: Binary data ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avformat/flacdec: support fast-seek
flacdec is not supported fast-seek yet. changes: 1. parse the seekpoint from metadata block 2. add seek callback to look up seek position 0001-avformat-flacdec-support-fast-seek.patch Description: Binary data ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/flacdec: support fast-seek
I found the generic seek will invoke the flac_read_timestamp function. It read exta packet will slow down the seek time https://github.com/FFmpeg/FFmpeg/blob/master/libavformat/flacdec.c#L224 2015-09-24 17:27 GMT+08:00 Ching-Yi Chan <chingyichan...@gmail.com>: > Hi Hendrik, > > I move the index data into AVStream, but seeking is slow. (it try to > download many data). > Is there something wrong ? > > 2015-09-24 16:09 GMT+08:00 Hendrik Leppkes <h.lepp...@gmail.com>: > >> On Thu, Sep 24, 2015 at 9:08 AM, Ching-Yi Chan <chingyichan...@gmail.com> >> wrote: >> > flacdec is not supported fast-seek yet. >> > >> > changes: >> > >> > 1. parse the seekpoint from metadata block >> > 2. add seek callback to look up seek position >> > >> >> You should store the index in the AVStream index_entries field, that >> way you don't need to duplicate the structure in the private flac >> context, and the generic seeking code can make use of them without you >> needing a specialized seekign function. >> >> - Hendrik >> ___ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> > > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/flacdec: support fast-seek
Is it acceptable to provide an option for different methods between accuracy seek and fast but inaccurate. For example, mp3dec having "usetoc" option to seek by toc. 2015-09-24 22:02 GMT+08:00 Hendrik Leppkes <h.lepp...@gmail.com>: > On Thu, Sep 24, 2015 at 2:44 PM, Ching-Yi Chan <chingyichan...@gmail.com> > wrote: > > I found the generic seek will invoke the flac_read_timestamp function. > > It read exta packet will slow down the seek time > > https://github.com/FFmpeg/FFmpeg/blob/master/libavformat/flacdec.c#L224 > > > > > > It does that to get to an accurate seek position. We do not have a way > to tell avformat to do a fast and inaccurate seek, and accuracy seems > more important to me, if the format allows for it. > Reading the index would however still allow this seek function to be > faster than it is now, since it knows the points in the index as > boundary points. > > - Hendrik > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/flacdec: support fast-seek
I do it with AVFMT_FLAG_FAST_SEEK flag. 2015-09-24 23:47 GMT+08:00 wm4 <nfx...@googlemail.com>: > On Thu, 24 Sep 2015 23:41:03 +0800 > Ching-Yi Chan <chingyichan...@gmail.com> wrote: > > > Is it acceptable to provide an option for different methods > > between accuracy seek and fast but inaccurate. > > > > For example, mp3dec having "usetoc" option to seek by toc. > > You could use the AVFMT_FLAG_FAST_SEEK flag. (It's a flag set on the > format context.) > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > 0001-avformat-flacdec-support-fast-seek.patch Description: Binary data ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/flacdec: support fast-seek
Thanks for checking.
I also check the AVFMT_FLAG_FAST_SEEK flag with parsing headers,
to fill the seektable into index entries only if AVFMT_FLAG_FAST_SEEK flag
is on.
If the AVFMT_FLAG_FAST_SEEK flag is not enabled, it will seek in original
way.
2015-09-25 10:37 GMT+08:00 Michael Niedermayer <michae...@gmx.at>:
> On Fri, Sep 25, 2015 at 09:22:41AM +0800, Ching-Yi Chan wrote:
> > I do it with AVFMT_FLAG_FAST_SEEK flag.
> >
> >
> > 2015-09-24 23:47 GMT+08:00 wm4 <nfx...@googlemail.com>:
> >
> > > On Thu, 24 Sep 2015 23:41:03 +0800
> > > Ching-Yi Chan <chingyichan...@gmail.com> wrote:
> > >
> > > > Is it acceptable to provide an option for different methods
> > > > between accuracy seek and fast but inaccurate.
> > > >
> > > > For example, mp3dec having "usetoc" option to seek by toc.
> > >
> > > You could use the AVFMT_FLAG_FAST_SEEK flag. (It's a flag set on the
> > > format context.)
> > > ___
> > > ffmpeg-devel mailing list
> > > ffmpeg-devel@ffmpeg.org
> > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > >
>
> > flacdec.c | 43 ++-
> > 1 file changed, 42 insertions(+), 1 deletion(-)
> > 980c23fbd623f488c24194ec92d75aebe5e183f8
> 0001-avformat-flacdec-support-fast-seek.patch
> > From 2e1d69e0a24455136d1a5dcc0898eac1bb2cb602 Mon Sep 17 00:00:00 2001
> > From: "Ching Yi, Chan" <chingyichan...@gmail.com>
> > Date: Thu, 24 Sep 2015 13:04:40 +0800
> > Subject: [PATCH] avformat/flacdec: support fast-seek
> >
> > ---
> > libavformat/flacdec.c | 43 ++-
> > 1 files changed, 42 insertions(+), 1 deletions(-)
> >
> > diff --git a/libavformat/flacdec.c b/libavformat/flacdec.c
> > index 4c1f943..3e4d867 100644
> > --- a/libavformat/flacdec.c
> > +++ b/libavformat/flacdec.c
> > @@ -28,11 +28,14 @@
> > #include "vorbiscomment.h"
> > #include "replaygain.h"
> >
> > +#define SEEKPOINT_SIZE 18
> > +
> > static int flac_read_header(AVFormatContext *s)
> > {
> > int ret, metadata_last=0, metadata_type, metadata_size,
> found_streaminfo=0;
> > uint8_t header[4];
> > uint8_t *buffer=NULL;
> > +
> > AVStream *st = avformat_new_stream(s, NULL);
> > if (!st)
> > return AVERROR(ENOMEM);
> > @@ -58,6 +61,7 @@ static int flac_read_header(AVFormatContext *s)
> > case FLAC_METADATA_TYPE_CUESHEET:
> > case FLAC_METADATA_TYPE_PICTURE:
> > case FLAC_METADATA_TYPE_VORBIS_COMMENT:
> > +case FLAC_METADATA_TYPE_SEEKTABLE:
> > buffer = av_mallocz(metadata_size +
> AV_INPUT_BUFFER_PADDING_SIZE);
> > if (!buffer) {
> > return AVERROR(ENOMEM);
> > @@ -132,7 +136,20 @@ static int flac_read_header(AVFormatContext *s)
> > av_log(s, AV_LOG_ERROR, "Error parsing attached
> picture.\n");
> > return ret;
> > }
> > -} else {
> > +} else if (metadata_type == FLAC_METADATA_TYPE_SEEKTABLE) {
> > +const uint8_t *seekpoint = buffer;
> > +int i, seek_point_count = metadata_size/SEEKPOINT_SIZE;
> > +for(i=0; i<seek_point_count; i++) {
> > +int64_t timestamp = bytestream_get_be64();
> > +int64_t pos = bytestream_get_be64();
> > +/* skip number of samples */
> > +bytestream_get_be16();
> > +av_add_index_entry(st, pos, timestamp, 0, 0,
> AVINDEX_KEYFRAME);
> > +}
> > +av_freep();
> > +}
> > +else {
> > +
> > /* STREAMINFO must be the first block */
> > if (!found_streaminfo) {
> > RETURN_ERROR(AVERROR_INVALIDDATA);
>
> > @@ -169,6 +186,12 @@ static int flac_read_header(AVFormatContext *s)
> > if (ret < 0)
> > return ret;
> >
> > +/* the real seek index offset should be the size of metadata blocks
> with the offset in the frame blocks */
> > +int metadata_head_size = avio_tell(s->pb);
>
> this should be int64_t probably
> also declarations and statments should not be mixed
>
>
> > +int i;
> > +for(i=0; inb_index_entries; i++) {
> > +st->index_entries[i].pos += metadata_head_size;
> > +}
> >
