diff mbox

[FFmpeg-devel] libavformat/matroskadec: Add test for seeking with codec delay.

Message ID 1469669610-573-1-git-send-email-chcunningham@chromium.org
State Accepted
Commit 52ec4cc09b5be755f166b7ed4755433c95173d6d
Headers show

Commit Message

chcunningham@chromium.org July 28, 2016, 1:33 a.m. UTC 
From: Chris Cunningham <chcunningham@chromium.org>

Also cleanup parens for the skip_to_timecode check.
---
 libavformat/matroskadec.c      |  2 +-
 tests/fate/seek.mak            |  3 +++
 tests/ref/seek/mkv-codec-delay | 48 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 52 insertions(+), 1 deletion(-)
 create mode 100644 tests/ref/seek/mkv-codec-delay

Comments

chcunningham@chromium.org July 28, 2016, 1:35 a.m. UTC | #1 
The file to upload to fate-suite can be found here:
https://storage.googleapis.com/chcunningham-chrome-shared/codec_delay_opus.mkv

Alternatively, you can generate it via:
ffmpeg -i ../../tests/data/lavf/lavf.mkv -acodec opus -vn
codec_delay_opus.mkv

On Wed, Jul 27, 2016 at 6:33 PM, <chcunningham@chromium.org> wrote:

> From: Chris Cunningham <chcunningham@chromium.org>
>
> Also cleanup parens for the skip_to_timecode check.
> ---
>  libavformat/matroskadec.c      |  2 +-
>  tests/fate/seek.mak            |  3 +++
>  tests/ref/seek/mkv-codec-delay | 48
> ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 52 insertions(+), 1 deletion(-)
>  create mode 100644 tests/ref/seek/mkv-codec-delay
>
> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
> index 60b1b34..d07a092 100644
> --- a/libavformat/matroskadec.c
> +++ b/libavformat/matroskadec.c
> @@ -3153,7 +3153,7 @@ static int matroska_parse_block(MatroskaDemuxContext
> *matroska, uint8_t *data,
>          // Compare signed timecodes. Timecode may be negative due to
> codec delay
>          // offset. We don't support timestamps greater than int64_t
> anyway - see
>          // AVPacket's pts.
> -        if ((int64_t)timecode < (int64_t)(matroska->skip_to_timecode))
> +        if ((int64_t)timecode < (int64_t)matroska->skip_to_timecode)
>              return res;
>          if (is_keyframe)
>              matroska->skip_to_keyframe = 0;
> diff --git a/tests/fate/seek.mak b/tests/fate/seek.mak
> index b831cf8..f835da5 100644
> --- a/tests/fate/seek.mak
> +++ b/tests/fate/seek.mak
> @@ -247,8 +247,11 @@ FATE_SEEK += $(FATE_SEEK_LAVF-yes:%=fate-seek-lavf-%)
>
>  FATE_SEEK_EXTRA-$(CONFIG_MP3_DEMUXER)   += fate-seek-extra-mp3
>  FATE_SEEK_EXTRA-$(call ALLYES, CACHE_PROTOCOL PIPE_PROTOCOL MP3_DEMUXER)
> += fate-seek-cache-pipe
> +FATE_SEEK_EXTRA-$(CONFIG_MATROSKA_DEMUXER) += fate-seek-mkv-codec-delay
>  fate-seek-extra-mp3:  CMD = run libavformat/tests/seek$(EXESUF)
> $(TARGET_SAMPLES)/gapless/gapless.mp3 -fastseek 1
>  fate-seek-cache-pipe: CMD = cat $(TARGET_SAMPLES)/gapless/gapless.mp3 |
> run libavformat/tests/seek$(EXESUF) cache:pipe:0 -read_ahead_limit -1
> +fate-seek-mkv-codec-delay:   CMD = run libavformat/tests/seek$(EXESUF)
> $(TARGET_SAMPLES)/mkv/codec_delay_opus.mkv
> +
>  FATE_SEEK_EXTRA += $(FATE_SEEK_EXTRA-yes)
>
>
> diff --git a/tests/ref/seek/mkv-codec-delay
> b/tests/ref/seek/mkv-codec-delay
> new file mode 100644
> index 0000000..9d4582c
> --- /dev/null
> +++ b/tests/ref/seek/mkv-codec-delay
> @@ -0,0 +1,48 @@
> +ret: 0         st: 0 flags:1 dts:-0.007000 pts:-0.007000 pos:    748
> size:   320
> +ret: 0         st:-1 flags:0  ts:-1.000000
> +ret: 0         st: 0 flags:1 dts:-0.007000 pts:-0.007000 pos:    748
> size:   320
> +ret: 0         st:-1 flags:1  ts: 1.894167
> +ret: 0         st: 0 flags:1 dts: 1.014000 pts: 1.014000 pos:   9306
> size:   291
> +ret: 0         st: 0 flags:0  ts: 0.788000
> +ret: 0         st: 0 flags:1 dts: 0.794000 pts: 0.794000 pos:   7358
> size:   154
> +ret: 0         st: 0 flags:1  ts:-0.317000
> +ret: 0         st: 0 flags:1 dts:-0.007000 pts:-0.007000 pos:    748
> size:   320
> +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: 1.014000 pts: 1.014000 pos:   9306
> size:   291
> +ret: 0         st: 0 flags:0  ts: 0.365000
> +ret: 0         st: 0 flags:1 dts: 0.374000 pts: 0.374000 pos:   3963
> size:   150
> +ret: 0         st: 0 flags:1  ts:-0.741000
> +ret: 0         st: 0 flags:1 dts:-0.007000 pts:-0.007000 pos:    748
> size:   320
> +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: 1.014000 pts: 1.014000 pos:   9306
> size:   291
> +ret: 0         st: 0 flags:0  ts:-0.058000
> +ret: 0         st: 0 flags:1 dts:-0.007000 pts:-0.007000 pos:    748
> size:   320
> +ret: 0         st: 0 flags:1  ts: 2.836000
> +ret: 0         st: 0 flags:1 dts: 1.014000 pts: 1.014000 pos:   9306
> size:   291
> +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.614000 pts: 0.614000 pos:   5903
> size:   159
> +ret: 0         st: 0 flags:0  ts:-0.482000
> +ret: 0         st: 0 flags:1 dts:-0.007000 pts:-0.007000 pos:    748
> size:   320
> +ret: 0         st: 0 flags:1  ts: 2.413000
> +ret: 0         st: 0 flags:1 dts: 1.014000 pts: 1.014000 pos:   9306
> size:   291
> +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.194000 pts: 0.194000 pos:   2512
> size:   159
> +ret: 0         st: 0 flags:0  ts:-0.905000
> +ret: 0         st: 0 flags:1 dts:-0.007000 pts:-0.007000 pos:    748
> size:   320
> +ret: 0         st: 0 flags:1  ts: 1.989000
> +ret: 0         st: 0 flags:1 dts: 1.014000 pts: 1.014000 pos:   9306
> size:   291
> +ret: 0         st:-1 flags:0  ts: 0.883340
> +ret: 0         st: 0 flags:1 dts: 0.894000 pts: 0.894000 pos:   8154
> size:   155
> +ret: 0         st:-1 flags:1  ts:-0.222493
> +ret: 0         st: 0 flags:1 dts:-0.007000 pts:-0.007000 pos:    748
> size:   320
> +ret:-1         st: 0 flags:0  ts: 2.672000
> +ret: 0         st: 0 flags:1  ts: 1.566000
> +ret: 0         st: 0 flags:1 dts: 1.014000 pts: 1.014000 pos:   9306
> size:   291
> +ret: 0         st:-1 flags:0  ts: 0.460008
> +ret: 0         st: 0 flags:1 dts: 0.474000 pts: 0.474000 pos:   4768
> size:   153
> +ret: 0         st:-1 flags:1  ts:-0.645825
> +ret: 0         st: 0 flags:1 dts:-0.007000 pts:-0.007000 pos:    748
> size:   320
> --
> 2.8.0.rc3.226.g39d4020
>
>
Michael Niedermayer July 28, 2016, 4:48 p.m. UTC | #2 
On Wed, Jul 27, 2016 at 06:35:50PM -0700, Chris Cunningham wrote:
> The file to upload to fate-suite can be found here:
> https://storage.googleapis.com/chcunningham-chrome-shared/codec_delay_opus.mkv

uploaded

thx

[...]
Chris Cunningham July 28, 2016, 8:13 p.m. UTC | #3 
Thanks Michael. Do you mean to also apply the patch to add the test? Maybe
you're waiting for further review.

On Thu, Jul 28, 2016 at 9:48 AM, Michael Niedermayer <michael@niedermayer.cc
> wrote:

> On Wed, Jul 27, 2016 at 06:35:50PM -0700, Chris Cunningham wrote:
> > The file to upload to fate-suite can be found here:
> >
> https://storage.googleapis.com/chcunningham-chrome-shared/codec_delay_opus.mkv
>
> uploaded
>
> thx
>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Those who are best at talking, realize last or never when they are wrong.
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>
chcunningham@chromium.org July 28, 2016, 8:24 p.m. UTC | #4 
Thanks Michael. Do you mean to also apply the patch to add the test? Maybe
you're waiting for further review.

On Thu, Jul 28, 2016 at 9:48 AM, Michael Niedermayer <michael@niedermayer.cc
> wrote:

> On Wed, Jul 27, 2016 at 06:35:50PM -0700, Chris Cunningham wrote:
> > The file to upload to fate-suite can be found here:
> >
> https://storage.googleapis.com/chcunningham-chrome-shared/codec_delay_opus.mkv
>
> uploaded
>
> thx
>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Those who are best at talking, realize last or never when they are wrong.
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>
James Almer July 28, 2016, 8:28 p.m. UTC | #5 
On 7/28/2016 5:24 PM, Chris Cunningham wrote:
> Thanks Michael. Do you mean to also apply the patch to add the test? Maybe
> you're waiting for further review.

Usually samples are uploaded some hours or a day before pushing the commit
that uses them, to give the FATE clients time to sync.

Also, please don't top post.

> 
> On Thu, Jul 28, 2016 at 9:48 AM, Michael Niedermayer <michael@niedermayer.cc
>> wrote:
> 
>> On Wed, Jul 27, 2016 at 06:35:50PM -0700, Chris Cunningham wrote:
>>> The file to upload to fate-suite can be found here:
>>>
>> https://storage.googleapis.com/chcunningham-chrome-shared/codec_delay_opus.mkv
>>
>> uploaded
>>
>> thx
>>
>> [...]
>> --
>> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>>
>> Those who are best at talking, realize last or never when they are wrong.
>>
>> _______________________________________________
>> 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
>
Michael Niedermayer July 28, 2016, 10:54 p.m. UTC | #6 
On Wed, Jul 27, 2016 at 06:33:30PM -0700, chcunningham@chromium.org wrote:
> From: Chris Cunningham <chcunningham@chromium.org>
> 
> Also cleanup parens for the skip_to_timecode check.
> ---
>  libavformat/matroskadec.c      |  2 +-
>  tests/fate/seek.mak            |  3 +++
>  tests/ref/seek/mkv-codec-delay | 48 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 52 insertions(+), 1 deletion(-)
>  create mode 100644 tests/ref/seek/mkv-codec-delay

LGTM
ill apply tomorrow unless i forget in which case ill apply when someone
reminds me

[...]
Michael Niedermayer July 30, 2016, 12:11 a.m. UTC | #7 
On Fri, Jul 29, 2016 at 12:54:13AM +0200, Michael Niedermayer wrote:
> On Wed, Jul 27, 2016 at 06:33:30PM -0700, chcunningham@chromium.org wrote:
> > From: Chris Cunningham <chcunningham@chromium.org>
> > 
> > Also cleanup parens for the skip_to_timecode check.
> > ---
> >  libavformat/matroskadec.c      |  2 +-
> >  tests/fate/seek.mak            |  3 +++
> >  tests/ref/seek/mkv-codec-delay | 48 ++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 52 insertions(+), 1 deletion(-)
> >  create mode 100644 tests/ref/seek/mkv-codec-delay
> 
> LGTM
> ill apply tomorrow unless i forget in which case ill apply when someone
> reminds me

applied

thx

[...]
diff mbox

Patch

diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index 60b1b34..d07a092 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -3153,7 +3153,7 @@  static int matroska_parse_block(MatroskaDemuxContext *matroska, uint8_t *data,
         // Compare signed timecodes. Timecode may be negative due to codec delay
         // offset. We don't support timestamps greater than int64_t anyway - see
         // AVPacket's pts.
-        if ((int64_t)timecode < (int64_t)(matroska->skip_to_timecode))
+        if ((int64_t)timecode < (int64_t)matroska->skip_to_timecode)
             return res;
         if (is_keyframe)
             matroska->skip_to_keyframe = 0;
diff --git a/tests/fate/seek.mak b/tests/fate/seek.mak
index b831cf8..f835da5 100644
--- a/tests/fate/seek.mak
+++ b/tests/fate/seek.mak
@@ -247,8 +247,11 @@  FATE_SEEK += $(FATE_SEEK_LAVF-yes:%=fate-seek-lavf-%)
 
 FATE_SEEK_EXTRA-$(CONFIG_MP3_DEMUXER)   += fate-seek-extra-mp3
 FATE_SEEK_EXTRA-$(call ALLYES, CACHE_PROTOCOL PIPE_PROTOCOL MP3_DEMUXER) += fate-seek-cache-pipe
+FATE_SEEK_EXTRA-$(CONFIG_MATROSKA_DEMUXER) += fate-seek-mkv-codec-delay
 fate-seek-extra-mp3:  CMD = run libavformat/tests/seek$(EXESUF) $(TARGET_SAMPLES)/gapless/gapless.mp3 -fastseek 1
 fate-seek-cache-pipe: CMD = cat $(TARGET_SAMPLES)/gapless/gapless.mp3 | run libavformat/tests/seek$(EXESUF) cache:pipe:0 -read_ahead_limit -1
+fate-seek-mkv-codec-delay:   CMD = run libavformat/tests/seek$(EXESUF) $(TARGET_SAMPLES)/mkv/codec_delay_opus.mkv
+
 FATE_SEEK_EXTRA += $(FATE_SEEK_EXTRA-yes)
 
 
diff --git a/tests/ref/seek/mkv-codec-delay b/tests/ref/seek/mkv-codec-delay
new file mode 100644
index 0000000..9d4582c
--- /dev/null
+++ b/tests/ref/seek/mkv-codec-delay
@@ -0,0 +1,48 @@ 
+ret: 0         st: 0 flags:1 dts:-0.007000 pts:-0.007000 pos:    748 size:   320
+ret: 0         st:-1 flags:0  ts:-1.000000
+ret: 0         st: 0 flags:1 dts:-0.007000 pts:-0.007000 pos:    748 size:   320
+ret: 0         st:-1 flags:1  ts: 1.894167
+ret: 0         st: 0 flags:1 dts: 1.014000 pts: 1.014000 pos:   9306 size:   291
+ret: 0         st: 0 flags:0  ts: 0.788000
+ret: 0         st: 0 flags:1 dts: 0.794000 pts: 0.794000 pos:   7358 size:   154
+ret: 0         st: 0 flags:1  ts:-0.317000
+ret: 0         st: 0 flags:1 dts:-0.007000 pts:-0.007000 pos:    748 size:   320
+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: 1.014000 pts: 1.014000 pos:   9306 size:   291
+ret: 0         st: 0 flags:0  ts: 0.365000
+ret: 0         st: 0 flags:1 dts: 0.374000 pts: 0.374000 pos:   3963 size:   150
+ret: 0         st: 0 flags:1  ts:-0.741000
+ret: 0         st: 0 flags:1 dts:-0.007000 pts:-0.007000 pos:    748 size:   320
+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: 1.014000 pts: 1.014000 pos:   9306 size:   291
+ret: 0         st: 0 flags:0  ts:-0.058000
+ret: 0         st: 0 flags:1 dts:-0.007000 pts:-0.007000 pos:    748 size:   320
+ret: 0         st: 0 flags:1  ts: 2.836000
+ret: 0         st: 0 flags:1 dts: 1.014000 pts: 1.014000 pos:   9306 size:   291
+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.614000 pts: 0.614000 pos:   5903 size:   159
+ret: 0         st: 0 flags:0  ts:-0.482000
+ret: 0         st: 0 flags:1 dts:-0.007000 pts:-0.007000 pos:    748 size:   320
+ret: 0         st: 0 flags:1  ts: 2.413000
+ret: 0         st: 0 flags:1 dts: 1.014000 pts: 1.014000 pos:   9306 size:   291
+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.194000 pts: 0.194000 pos:   2512 size:   159
+ret: 0         st: 0 flags:0  ts:-0.905000
+ret: 0         st: 0 flags:1 dts:-0.007000 pts:-0.007000 pos:    748 size:   320
+ret: 0         st: 0 flags:1  ts: 1.989000
+ret: 0         st: 0 flags:1 dts: 1.014000 pts: 1.014000 pos:   9306 size:   291
+ret: 0         st:-1 flags:0  ts: 0.883340
+ret: 0         st: 0 flags:1 dts: 0.894000 pts: 0.894000 pos:   8154 size:   155
+ret: 0         st:-1 flags:1  ts:-0.222493
+ret: 0         st: 0 flags:1 dts:-0.007000 pts:-0.007000 pos:    748 size:   320
+ret:-1         st: 0 flags:0  ts: 2.672000
+ret: 0         st: 0 flags:1  ts: 1.566000
+ret: 0         st: 0 flags:1 dts: 1.014000 pts: 1.014000 pos:   9306 size:   291
+ret: 0         st:-1 flags:0  ts: 0.460008
+ret: 0         st: 0 flags:1 dts: 0.474000 pts: 0.474000 pos:   4768 size:   153
+ret: 0         st:-1 flags:1  ts:-0.645825
+ret: 0         st: 0 flags:1 dts:-0.007000 pts:-0.007000 pos:    748 size:   320