[FFmpeg-devel] avformat/yuv4mpegdec: use generic code for seeking

Submitted by Paul B Mahol on May 1, 2018, 9:47 a.m.

Details

Message ID 20180501094704.14408-1-onemda@gmail.com
State New
Headers show

Commit Message

Paul B Mahol May 1, 2018, 9:47 a.m.
Signed-off-by: Paul B Mahol <onemda@gmail.com>
---
 libavformat/yuv4mpegdec.c    | 23 ++++----------------
 tests/ref/seek/lavf-yuv4mpeg | 52 ++++++++++++++++++++------------------------
 2 files changed, 28 insertions(+), 47 deletions(-)

Comments

wm4 May 1, 2018, 1:05 p.m.
On Tue,  1 May 2018 11:47:04 +0200
Paul B Mahol <onemda@gmail.com> wrote:

> Signed-off-by: Paul B Mahol <onemda@gmail.com>
> ---
>  libavformat/yuv4mpegdec.c    | 23 ++++----------------
>  tests/ref/seek/lavf-yuv4mpeg | 52 ++++++++++++++++++++------------------------
>  2 files changed, 28 insertions(+), 47 deletions(-)
> 
> diff --git a/libavformat/yuv4mpegdec.c b/libavformat/yuv4mpegdec.c
> index 8662a42a4c..cf6da2a2ef 100644
> --- a/libavformat/yuv4mpegdec.c
> +++ b/libavformat/yuv4mpegdec.c
> @@ -306,28 +306,13 @@ static int yuv4_read_packet(AVFormatContext *s, AVPacket *pkt)
>          return s->pb->eof_reached ? AVERROR_EOF : AVERROR(EIO);
>      }
>      pkt->stream_index = 0;
> -    pkt->pts = (off - s->internal->data_offset) / s->packet_size;
> +    pkt->pos = off;
> +    pkt->dts = pkt->pts = (off - s->internal->data_offset) / s->packet_size;
> +    pkt->flags |= AV_PKT_FLAG_KEY;
>      pkt->duration = 1;
>      return 0;
>  }
>  
> -static int yuv4_read_seek(AVFormatContext *s, int stream_index,
> -                          int64_t pts, int flags)
> -{
> -    AVStream *st = s->streams[0];
> -    int64_t pos;
> -
> -    pos = av_rescale_rnd(pts * s->packet_size,
> -                         st->time_base.num,
> -                         st->time_base.den * s->packet_size,
> -                         (flags & AVSEEK_FLAG_BACKWARD) ? AV_ROUND_DOWN : AV_ROUND_UP);
> -    pos *= s->packet_size;
> -
> -    if (avio_seek(s->pb, pos + s->internal->data_offset, SEEK_SET) < 0)
> -        return -1;
> -    return 0;
> -}
> -
>  static int yuv4_probe(AVProbeData *pd)
>  {
>      /* check file header */
> @@ -343,6 +328,6 @@ AVInputFormat ff_yuv4mpegpipe_demuxer = {
>      .read_probe     = yuv4_probe,
>      .read_header    = yuv4_read_header,
>      .read_packet    = yuv4_read_packet,
> -    .read_seek      = yuv4_read_seek,
>      .extensions     = "y4m",
> +    .flags          = AVFMT_GENERIC_INDEX,
>  };
> diff --git a/tests/ref/seek/lavf-yuv4mpeg b/tests/ref/seek/lavf-yuv4mpeg

Seems like a bad idea. This will make seeking read and skip all data
when seeking to the middle of the stream, instead of just seeking there.
Paul B Mahol May 1, 2018, 1:17 p.m.
On 5/1/18, wm4 <nfxjfg@googlemail.com> wrote:
> On Tue,  1 May 2018 11:47:04 +0200
> Paul B Mahol <onemda@gmail.com> wrote:
>
>> Signed-off-by: Paul B Mahol <onemda@gmail.com>
>> ---
>>  libavformat/yuv4mpegdec.c    | 23 ++++----------------
>>  tests/ref/seek/lavf-yuv4mpeg | 52
>> ++++++++++++++++++++------------------------
>>  2 files changed, 28 insertions(+), 47 deletions(-)
>>
>> diff --git a/libavformat/yuv4mpegdec.c b/libavformat/yuv4mpegdec.c
>> index 8662a42a4c..cf6da2a2ef 100644
>> --- a/libavformat/yuv4mpegdec.c
>> +++ b/libavformat/yuv4mpegdec.c
>> @@ -306,28 +306,13 @@ static int yuv4_read_packet(AVFormatContext *s,
>> AVPacket *pkt)
>>          return s->pb->eof_reached ? AVERROR_EOF : AVERROR(EIO);
>>      }
>>      pkt->stream_index = 0;
>> -    pkt->pts = (off - s->internal->data_offset) / s->packet_size;
>> +    pkt->pos = off;
>> +    pkt->dts = pkt->pts = (off - s->internal->data_offset) /
>> s->packet_size;
>> +    pkt->flags |= AV_PKT_FLAG_KEY;
>>      pkt->duration = 1;
>>      return 0;
>>  }
>>
>> -static int yuv4_read_seek(AVFormatContext *s, int stream_index,
>> -                          int64_t pts, int flags)
>> -{
>> -    AVStream *st = s->streams[0];
>> -    int64_t pos;
>> -
>> -    pos = av_rescale_rnd(pts * s->packet_size,
>> -                         st->time_base.num,
>> -                         st->time_base.den * s->packet_size,
>> -                         (flags & AVSEEK_FLAG_BACKWARD) ? AV_ROUND_DOWN :
>> AV_ROUND_UP);
>> -    pos *= s->packet_size;
>> -
>> -    if (avio_seek(s->pb, pos + s->internal->data_offset, SEEK_SET) < 0)
>> -        return -1;
>> -    return 0;
>> -}
>> -
>>  static int yuv4_probe(AVProbeData *pd)
>>  {
>>      /* check file header */
>> @@ -343,6 +328,6 @@ AVInputFormat ff_yuv4mpegpipe_demuxer = {
>>      .read_probe     = yuv4_probe,
>>      .read_header    = yuv4_read_header,
>>      .read_packet    = yuv4_read_packet,
>> -    .read_seek      = yuv4_read_seek,
>>      .extensions     = "y4m",
>> +    .flags          = AVFMT_GENERIC_INDEX,
>>  };
>> diff --git a/tests/ref/seek/lavf-yuv4mpeg b/tests/ref/seek/lavf-yuv4mpeg
>
> Seems like a bad idea. This will make seeking read and skip all data
> when seeking to the middle of the stream, instead of just seeking there.

How do you explain that this patch makes backsteeping with mpv magnitude faster?

Without it, more far you are from start of file backstepping is linearly slower.
wm4 May 1, 2018, 1:45 p.m.
On Tue, 1 May 2018 15:17:53 +0200
Paul B Mahol <onemda@gmail.com> wrote:

> On 5/1/18, wm4 <nfxjfg@googlemail.com> wrote:
> > On Tue,  1 May 2018 11:47:04 +0200
> > Paul B Mahol <onemda@gmail.com> wrote:
> >  
> >> Signed-off-by: Paul B Mahol <onemda@gmail.com>
> >> ---
> >>  libavformat/yuv4mpegdec.c    | 23 ++++----------------
> >>  tests/ref/seek/lavf-yuv4mpeg | 52
> >> ++++++++++++++++++++------------------------
> >>  2 files changed, 28 insertions(+), 47 deletions(-)
> >>
> >> diff --git a/libavformat/yuv4mpegdec.c b/libavformat/yuv4mpegdec.c
> >> index 8662a42a4c..cf6da2a2ef 100644
> >> --- a/libavformat/yuv4mpegdec.c
> >> +++ b/libavformat/yuv4mpegdec.c
> >> @@ -306,28 +306,13 @@ static int yuv4_read_packet(AVFormatContext *s,
> >> AVPacket *pkt)
> >>          return s->pb->eof_reached ? AVERROR_EOF : AVERROR(EIO);
> >>      }
> >>      pkt->stream_index = 0;
> >> -    pkt->pts = (off - s->internal->data_offset) / s->packet_size;
> >> +    pkt->pos = off;
> >> +    pkt->dts = pkt->pts = (off - s->internal->data_offset) /
> >> s->packet_size;
> >> +    pkt->flags |= AV_PKT_FLAG_KEY;
> >>      pkt->duration = 1;
> >>      return 0;
> >>  }
> >>
> >> -static int yuv4_read_seek(AVFormatContext *s, int stream_index,
> >> -                          int64_t pts, int flags)
> >> -{
> >> -    AVStream *st = s->streams[0];
> >> -    int64_t pos;
> >> -
> >> -    pos = av_rescale_rnd(pts * s->packet_size,
> >> -                         st->time_base.num,
> >> -                         st->time_base.den * s->packet_size,
> >> -                         (flags & AVSEEK_FLAG_BACKWARD) ? AV_ROUND_DOWN :
> >> AV_ROUND_UP);
> >> -    pos *= s->packet_size;
> >> -
> >> -    if (avio_seek(s->pb, pos + s->internal->data_offset, SEEK_SET) < 0)
> >> -        return -1;
> >> -    return 0;
> >> -}
> >> -
> >>  static int yuv4_probe(AVProbeData *pd)
> >>  {
> >>      /* check file header */
> >> @@ -343,6 +328,6 @@ AVInputFormat ff_yuv4mpegpipe_demuxer = {
> >>      .read_probe     = yuv4_probe,
> >>      .read_header    = yuv4_read_header,
> >>      .read_packet    = yuv4_read_packet,
> >> -    .read_seek      = yuv4_read_seek,
> >>      .extensions     = "y4m",
> >> +    .flags          = AVFMT_GENERIC_INDEX,
> >>  };
> >> diff --git a/tests/ref/seek/lavf-yuv4mpeg b/tests/ref/seek/lavf-yuv4mpeg  
> >
> > Seems like a bad idea. This will make seeking read and skip all data
> > when seeking to the middle of the stream, instead of just seeking there.  
> 
> How do you explain that this patch makes backsteeping with mpv magnitude faster?
> 
> Without it, more far you are from start of file backstepping is linearly slower.

I don't know how this interacts with mpv, but obviously it's unexpected
that O(1) demuxer level seeking is slower than O(n) seeking. You
probably have some sort of overflow or something in the seek routine
or elsewhere. Maybe it's an lavf internal fallback to something worse
if the seek fails (e.g. when trying to seek before the start of the
file). Obviously you should analyze this.

(mpv seeks before the start of the file because it can't reliable know
a pts is before the start.)

Patch hide | download patch | download mbox

diff --git a/libavformat/yuv4mpegdec.c b/libavformat/yuv4mpegdec.c
index 8662a42a4c..cf6da2a2ef 100644
--- a/libavformat/yuv4mpegdec.c
+++ b/libavformat/yuv4mpegdec.c
@@ -306,28 +306,13 @@  static int yuv4_read_packet(AVFormatContext *s, AVPacket *pkt)
         return s->pb->eof_reached ? AVERROR_EOF : AVERROR(EIO);
     }
     pkt->stream_index = 0;
-    pkt->pts = (off - s->internal->data_offset) / s->packet_size;
+    pkt->pos = off;
+    pkt->dts = pkt->pts = (off - s->internal->data_offset) / s->packet_size;
+    pkt->flags |= AV_PKT_FLAG_KEY;
     pkt->duration = 1;
     return 0;
 }
 
-static int yuv4_read_seek(AVFormatContext *s, int stream_index,
-                          int64_t pts, int flags)
-{
-    AVStream *st = s->streams[0];
-    int64_t pos;
-
-    pos = av_rescale_rnd(pts * s->packet_size,
-                         st->time_base.num,
-                         st->time_base.den * s->packet_size,
-                         (flags & AVSEEK_FLAG_BACKWARD) ? AV_ROUND_DOWN : AV_ROUND_UP);
-    pos *= s->packet_size;
-
-    if (avio_seek(s->pb, pos + s->internal->data_offset, SEEK_SET) < 0)
-        return -1;
-    return 0;
-}
-
 static int yuv4_probe(AVProbeData *pd)
 {
     /* check file header */
@@ -343,6 +328,6 @@  AVInputFormat ff_yuv4mpegpipe_demuxer = {
     .read_probe     = yuv4_probe,
     .read_header    = yuv4_read_header,
     .read_packet    = yuv4_read_packet,
-    .read_seek      = yuv4_read_seek,
     .extensions     = "y4m",
+    .flags          = AVFMT_GENERIC_INDEX,
 };
diff --git a/tests/ref/seek/lavf-yuv4mpeg b/tests/ref/seek/lavf-yuv4mpeg
index 15d4d8c8c1..0f85ea5a46 100644
--- a/tests/ref/seek/lavf-yuv4mpeg
+++ b/tests/ref/seek/lavf-yuv4mpeg
@@ -1,48 +1,44 @@ 
-ret: 0         st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos:     64 size:152064
-ret:-1         st:-1 flags:0  ts:-1.000000
+ret: 0         st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos:     58 size:152064
+ret: 0         st:-1 flags:0  ts:-1.000000
+ret: 0         st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos:     58 size:152064
 ret: 0         st:-1 flags:1  ts: 1.894167
-ret: 0         st: 0 flags:1 dts: 0.040000 pts: 0.040000 pos: 152134 size:152064
+ret: 0         st: 0 flags:1 dts: 0.960000 pts: 0.960000 pos:3649738 size:152064
 ret: 0         st: 0 flags:0  ts: 0.800000
-ret: 0         st: 0 flags:1 dts: 0.040000 pts: 0.040000 pos: 152134 size:152064
+ret: 0         st: 0 flags:1 dts: 0.800000 pts: 0.800000 pos:3041458 size:152064
 ret:-1         st: 0 flags:1  ts:-0.320000
-ret: 0         st:-1 flags:0  ts: 2.576668
-ret: 0         st: 0 flags:1 dts: 0.120000 pts: 0.120000 pos: 456274 size:152064
+ret:-1         st:-1 flags:0  ts: 2.576668
 ret: 0         st:-1 flags:1  ts: 1.470835
-ret: 0         st: 0 flags:1 dts: 0.040000 pts: 0.040000 pos: 152134 size:152064
+ret: 0         st: 0 flags:1 dts: 0.960000 pts: 0.960000 pos:3649738 size:152064
 ret: 0         st: 0 flags:0  ts: 0.360000
-ret: 0         st: 0 flags:1 dts: 0.040000 pts: 0.040000 pos: 152134 size:152064
+ret: 0         st: 0 flags:1 dts: 0.360000 pts: 0.360000 pos:1368688 size:152064
 ret:-1         st: 0 flags:1  ts:-0.760000
-ret: 0         st:-1 flags:0  ts: 2.153336
-ret: 0         st: 0 flags:1 dts: 0.120000 pts: 0.120000 pos: 456274 size:152064
+ret:-1         st:-1 flags:0  ts: 2.153336
 ret: 0         st:-1 flags:1  ts: 1.047503
-ret: 0         st: 0 flags:1 dts: 0.040000 pts: 0.040000 pos: 152134 size:152064
+ret: 0         st: 0 flags:1 dts: 0.960000 pts: 0.960000 pos:3649738 size:152064
 ret: 0         st: 0 flags:0  ts:-0.040000
-ret: 0         st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos:     64 size:152064
+ret: 0         st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos:     58 size:152064
 ret: 0         st: 0 flags:1  ts: 2.840000
-ret: 0         st: 0 flags:1 dts: 0.080000 pts: 0.080000 pos: 304204 size:152064
-ret: 0         st:-1 flags:0  ts: 1.730004
-ret: 0         st: 0 flags:1 dts: 0.080000 pts: 0.080000 pos: 304204 size:152064
+ret: 0         st: 0 flags:1 dts: 0.960000 pts: 0.960000 pos:3649738 size:152064
+ret:-1         st:-1 flags:0  ts: 1.730004
 ret: 0         st:-1 flags:1  ts: 0.624171
-ret: 0         st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos:     64 size:152064
+ret: 0         st: 0 flags:1 dts: 0.640000 pts: 0.640000 pos:2433178 size:152064
 ret: 0         st: 0 flags:0  ts:-0.480000
-ret: 0         st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos:     64 size:152064
+ret: 0         st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos:     58 size:152064
 ret: 0         st: 0 flags:1  ts: 2.400000
-ret: 0         st: 0 flags:1 dts: 0.080000 pts: 0.080000 pos: 304204 size:152064
-ret: 0         st:-1 flags:0  ts: 1.306672
-ret: 0         st: 0 flags:1 dts: 0.080000 pts: 0.080000 pos: 304204 size:152064
+ret: 0         st: 0 flags:1 dts: 0.960000 pts: 0.960000 pos:3649738 size:152064
+ret:-1         st:-1 flags:0  ts: 1.306672
 ret: 0         st:-1 flags:1  ts: 0.200839
-ret: 0         st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos:     64 size:152064
+ret: 0         st: 0 flags:1 dts: 0.200000 pts: 0.200000 pos: 760408 size:152064
 ret: 0         st: 0 flags:0  ts:-0.920000
-ret: 0         st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos:     64 size:152064
+ret: 0         st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos:     58 size:152064
 ret: 0         st: 0 flags:1  ts: 2.000000
-ret: 0         st: 0 flags:1 dts: 0.080000 pts: 0.080000 pos: 304204 size:152064
+ret: 0         st: 0 flags:1 dts: 0.960000 pts: 0.960000 pos:3649738 size:152064
 ret: 0         st:-1 flags:0  ts: 0.883340
-ret: 0         st: 0 flags:1 dts: 0.040000 pts: 0.040000 pos: 152134 size:152064
+ret: 0         st: 0 flags:1 dts: 0.880000 pts: 0.880000 pos:3345598 size:152064
 ret:-1         st:-1 flags:1  ts:-0.222493
-ret: 0         st: 0 flags:0  ts: 2.680000
-ret: 0         st: 0 flags:1 dts: 0.120000 pts: 0.120000 pos: 456274 size:152064
+ret:-1         st: 0 flags:0  ts: 2.680000
 ret: 0         st: 0 flags:1  ts: 1.560000
-ret: 0         st: 0 flags:1 dts: 0.040000 pts: 0.040000 pos: 152134 size:152064
+ret: 0         st: 0 flags:1 dts: 0.960000 pts: 0.960000 pos:3649738 size:152064
 ret: 0         st:-1 flags:0  ts: 0.460008
-ret: 0         st: 0 flags:1 dts: 0.040000 pts: 0.040000 pos: 152134 size:152064
+ret: 0         st: 0 flags:1 dts: 0.480000 pts: 0.480000 pos:1824898 size:152064
 ret:-1         st:-1 flags:1  ts:-0.645825