[FFmpeg-devel] avformat/yuv4mpegdec: simplify math

Submitted by Paul B Mahol on May 1, 2018, 3:48 p.m.

Details

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

Commit Message

Paul B Mahol May 1, 2018, 3:48 p.m.
This one actually works with hd1080 y4m files when seeking backwards.

Signed-off-by: Paul B Mahol <onemda@gmail.com>
---
 libavformat/yuv4mpegdec.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

Comments

wm4 May 2, 2018, 12:39 a.m.
On Tue,  1 May 2018 17:48:32 +0200
Paul B Mahol <onemda@gmail.com> wrote:

> This one actually works with hd1080 y4m files when seeking backwards.
> 
> Signed-off-by: Paul B Mahol <onemda@gmail.com>
> ---
>  libavformat/yuv4mpegdec.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/libavformat/yuv4mpegdec.c b/libavformat/yuv4mpegdec.c
> index 8662a42a4c..de3d5637fe 100644
> --- a/libavformat/yuv4mpegdec.c
> +++ b/libavformat/yuv4mpegdec.c
> @@ -317,11 +317,9 @@ static int yuv4_read_seek(AVFormatContext *s, int stream_index,
>      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 (flags & AVSEEK_FLAG_BACKWARD)
> +        pts -= 1;
> +    pos = pts * s->packet_size;
>  
>      if (avio_seek(s->pb, pos + s->internal->data_offset, SEEK_SET) < 0)
>          return -1;

That seems questionable too. Though I'm not sure if the previous code
is correct.

In theory, you're supposed to do this:
1. Determine seek target as accurately as possible
2. If the seek target is not exact then:
2a. if AVSEEK_FLAG_FORWARD is set, then round to the next frame
2b. otherwise, round to the previous frame

But if pts is in the stream timebase, then 2. can never happen. (And
also, I have no idea what the code removed by this patch does.)

Regarding mpv, can you just send me a reproducible test case?
Paul B Mahol May 2, 2018, 6:52 a.m.
On 5/2/18, wm4 <nfxjfg@googlemail.com> wrote:
> On Tue,  1 May 2018 17:48:32 +0200
> Paul B Mahol <onemda@gmail.com> wrote:
>
>> This one actually works with hd1080 y4m files when seeking backwards.
>>
>> Signed-off-by: Paul B Mahol <onemda@gmail.com>
>> ---
>>  libavformat/yuv4mpegdec.c | 8 +++-----
>>  1 file changed, 3 insertions(+), 5 deletions(-)
>>
>> diff --git a/libavformat/yuv4mpegdec.c b/libavformat/yuv4mpegdec.c
>> index 8662a42a4c..de3d5637fe 100644
>> --- a/libavformat/yuv4mpegdec.c
>> +++ b/libavformat/yuv4mpegdec.c
>> @@ -317,11 +317,9 @@ static int yuv4_read_seek(AVFormatContext *s, int
>> stream_index,
>>      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 (flags & AVSEEK_FLAG_BACKWARD)
>> +        pts -= 1;
>> +    pos = pts * s->packet_size;
>>
>>      if (avio_seek(s->pb, pos + s->internal->data_offset, SEEK_SET) < 0)
>>          return -1;
>
> That seems questionable too. Though I'm not sure if the previous code
> is correct.
>
> In theory, you're supposed to do this:
> 1. Determine seek target as accurately as possible
> 2. If the seek target is not exact then:
> 2a. if AVSEEK_FLAG_FORWARD is set, then round to the next frame
> 2b. otherwise, round to the previous frame
>
> But if pts is in the stream timebase, then 2. can never happen. (And
> also, I have no idea what the code removed by this patch does.)
>
> Regarding mpv, can you just send me a reproducible test case?

Download any hd1080 derf sample and play it until end and than do backstepping,
with --no-correct-pts and without.

Patch hide | download patch | download mbox

diff --git a/libavformat/yuv4mpegdec.c b/libavformat/yuv4mpegdec.c
index 8662a42a4c..de3d5637fe 100644
--- a/libavformat/yuv4mpegdec.c
+++ b/libavformat/yuv4mpegdec.c
@@ -317,11 +317,9 @@  static int yuv4_read_seek(AVFormatContext *s, int stream_index,
     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 (flags & AVSEEK_FLAG_BACKWARD)
+        pts -= 1;
+    pos = pts * s->packet_size;
 
     if (avio_seek(s->pb, pos + s->internal->data_offset, SEEK_SET) < 0)
         return -1;