diff mbox series

[FFmpeg-devel,3/3] test: hlsenc: Use unique init/segment file names for the fmp4_ac3 test

Message ID 20200802064128.15055-3-martin@martin.st
State Accepted
Commit 4ad868497f2b5b10bff934f46f1b590c28e999de
Headers show
Series [FFmpeg-devel,1/3] test: hlsenc: Make the hls_fmp4 sample file name match the target
Related show

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate fail Make fate failed

Commit Message

Martin Storsjö Aug. 2, 2020, 6:41 a.m. UTC
Previously, the hls-fmp4 and hls-fmp4_ac3 tests used the same file
names for init and segment files, which occasionally could cause
corruption and failed tests, if the input files for both tests are
generated in parallel, as they could overwrite each other.

This happened to work some of the time, as the fmp4_ac3 test actually
only checked the init segment file (which the fmp4 test case never
wrote, due to using the incorrect hls_segment_type option) and the
fmp4 test case always regenerated the input files due to mismatched
target and file names.
---
 tests/fate/hlsenc.mak | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Andreas Rheinhardt Aug. 2, 2020, 10:49 a.m. UTC | #1
Martin Storsjö:
> Previously, the hls-fmp4 and hls-fmp4_ac3 tests used the same file
> names for init and segment files, which occasionally could cause
> corruption and failed tests, if the input files for both tests are
> generated in parallel, as they could overwrite each other.
> 
> This happened to work some of the time, as the fmp4_ac3 test actually
> only checked the init segment file (which the fmp4 test case never
> wrote, due to using the incorrect hls_segment_type option) and the
> fmp4 test case always regenerated the input files due to mismatched
> target and file names.
> ---
>  tests/fate/hlsenc.mak | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/fate/hlsenc.mak b/tests/fate/hlsenc.mak
> index a57450cd7c..b3e87c0542 100644
> --- a/tests/fate/hlsenc.mak
> +++ b/tests/fate/hlsenc.mak
> @@ -101,13 +101,13 @@ tests/data/hls_fmp4_ac3.m3u8: TAG = GEN
>  tests/data/hls_fmp4_ac3.m3u8: ffmpeg$(PROGSSUF)$(EXESUF) | tests/data
>  	$(M)$(TARGET_EXEC) $(TARGET_PATH)/$< \
>  	-stream_loop 4 -i $(SAMPLES)/ac3/monsters_inc_5.1_448_small.ac3 -c copy -map 0 \
> -	-hls_segment_type fmp4 -hls_fmp4_init_filename now.mp4 -hls_list_size 0 \
> -	-hls_time 2 -hls_segment_filename "$(TARGET_PATH)/tests/data/hls_fmp4_%d.m4s" \
> +	-hls_segment_type fmp4 -hls_fmp4_init_filename now_ac3.mp4 -hls_list_size 0 \
> +	-hls_time 2 -hls_segment_filename "$(TARGET_PATH)/tests/data/hls_fmp4_ac3_%d.m4s" \
>  	$(TARGET_PATH)/tests/data/hls_fmp4_ac3.m3u8 2>/dev/null
>  
>  FATE_HLSENC-$(call ALLYES, HLS_DEMUXER EAC3_DEMUXER) += fate-hls-fmp4_ac3
>  fate-hls-fmp4_ac3: tests/data/hls_fmp4_ac3.m3u8
> -fate-hls-fmp4_ac3: CMD = probeaudiostream $(TARGET_PATH)/tests/data/now.mp4
> +fate-hls-fmp4_ac3: CMD = probeaudiostream $(TARGET_PATH)/tests/data/now_ac3.mp4
>  
>  FATE_SAMPLES_FFMPEG += $(FATE_HLSENC-yes)
>  fate-hlsenc: $(FATE_HLSENC-yes)
> 
This test uses ffprobe, yet has no ffprobe dependency. It should
probably be included in FATE_SAMPLES_FFMPEG_FFPROBE; the other tests in
this file meanwhile all generate their samples themselves, so they could
be in FATE_FFMPEG if I am not mistaken.

- Andreas
Martin Storsjö Aug. 2, 2020, 3:57 p.m. UTC | #2
On Sunday, August 2, 2020, Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
wrote:

> Martin Storsjö:
> > Previously, the hls-fmp4 and hls-fmp4_ac3 tests used the same file
> > names for init and segment files, which occasionally could cause
> > corruption and failed tests, if the input files for both tests are
> > generated in parallel, as they could overwrite each other.
> >
> > This happened to work some of the time, as the fmp4_ac3 test actually
> > only checked the init segment file (which the fmp4 test case never
> > wrote, due to using the incorrect hls_segment_type option) and the
> > fmp4 test case always regenerated the input files due to mismatched
> > target and file names.
> > ---
> >  tests/fate/hlsenc.mak | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/tests/fate/hlsenc.mak b/tests/fate/hlsenc.mak
> > index a57450cd7c..b3e87c0542 100644
> > --- a/tests/fate/hlsenc.mak
> > +++ b/tests/fate/hlsenc.mak
> > @@ -101,13 +101,13 @@ tests/data/hls_fmp4_ac3.m3u8: TAG = GEN
> >  tests/data/hls_fmp4_ac3.m3u8: ffmpeg$(PROGSSUF)$(EXESUF) | tests/data
> >       $(M)$(TARGET_EXEC) $(TARGET_PATH)/$< \
> >       -stream_loop 4 -i $(SAMPLES)/ac3/monsters_inc_5.1_448_small.ac3
> -c copy -map 0 \
> > -     -hls_segment_type fmp4 -hls_fmp4_init_filename now.mp4
> -hls_list_size 0 \
> > -     -hls_time 2 -hls_segment_filename "$(TARGET_PATH)/tests/data/hls_fmp4_%d.m4s"
> \
> > +     -hls_segment_type fmp4 -hls_fmp4_init_filename now_ac3.mp4
> -hls_list_size 0 \
> > +     -hls_time 2 -hls_segment_filename "$(TARGET_PATH)/tests/data/hls_fmp4_ac3_%d.m4s"
> \
> >       $(TARGET_PATH)/tests/data/hls_fmp4_ac3.m3u8 2>/dev/null
> >
> >  FATE_HLSENC-$(call ALLYES, HLS_DEMUXER EAC3_DEMUXER) +=
> fate-hls-fmp4_ac3
> >  fate-hls-fmp4_ac3: tests/data/hls_fmp4_ac3.m3u8
> > -fate-hls-fmp4_ac3: CMD = probeaudiostream $(TARGET_PATH)/tests/data/now.
> mp4
> > +fate-hls-fmp4_ac3: CMD = probeaudiostream $(TARGET_PATH)/tests/data/now_
> ac3.mp4
> >
> >  FATE_SAMPLES_FFMPEG += $(FATE_HLSENC-yes)
> >  fate-hlsenc: $(FATE_HLSENC-yes)
> >
> This test uses ffprobe, yet has no ffprobe dependency. It should
> probably be included in FATE_SAMPLES_FFMPEG_FFPROBE; the other tests in
> this file meanwhile all generate their samples themselves, so they could
> be in FATE_FFMPEG if I am not mistaken.
>

Fair enough, I guess I can try to make a patch to fix that aspect.

But that's orthogonal to this patch, which I'm hoping would fix the
spurious failures in fate-hls-fmp4 that I've been running into. Any
comments on this change in itself?

// Martin
Martin Storsjö Aug. 3, 2020, 8:51 p.m. UTC | #3
On Sun, 2 Aug 2020, Martin Storsjö wrote:

> On Sunday, August 2, 2020, Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> wrote:
>       Martin Storsjö:
>       > Previously, the hls-fmp4 and hls-fmp4_ac3 tests used the same
>       file
>       > names for init and segment files, which occasionally could
>       cause
>       > corruption and failed tests, if the input files for both tests
>       are
>       > generated in parallel, as they could overwrite each other.
>       >
>       > This happened to work some of the time, as the fmp4_ac3 test
>       actually
>       > only checked the init segment file (which the fmp4 test case
>       never
>       > wrote, due to using the incorrect hls_segment_type option) and
>       the
>       > fmp4 test case always regenerated the input files due to
>       mismatched
>       > target and file names.
>       > ---
>       >  tests/fate/hlsenc.mak | 6 +++---
>       >  1 file changed, 3 insertions(+), 3 deletions(-)
>       >
>       > diff --git a/tests/fate/hlsenc.mak b/tests/fate/hlsenc.mak
>       > index a57450cd7c..b3e87c0542 100644
>       > --- a/tests/fate/hlsenc.mak
>       > +++ b/tests/fate/hlsenc.mak
>       > @@ -101,13 +101,13 @@ tests/data/hls_fmp4_ac3.m3u8: TAG = GEN
>       >  tests/data/hls_fmp4_ac3.m3u8: ffmpeg$(PROGSSUF)$(EXESUF) |
>       tests/data
>       >       $(M)$(TARGET_EXEC) $(TARGET_PATH)/$< \
>       >       -stream_loop 4 -i
>       $(SAMPLES)/ac3/monsters_inc_5.1_448_small.ac3 -c copy -map 0 \
>       > -     -hls_segment_type fmp4 -hls_fmp4_init_filename now.mp4
>       -hls_list_size 0 \
>       > -     -hls_time 2 -hls_segment_filename
>       "$(TARGET_PATH)/tests/data/hls_fmp4_%d.m4s" \
>       > +     -hls_segment_type fmp4 -hls_fmp4_init_filename
>       now_ac3.mp4 -hls_list_size 0 \
>       > +     -hls_time 2 -hls_segment_filename
>       "$(TARGET_PATH)/tests/data/hls_fmp4_ac3_%d.m4s" \
>       >       $(TARGET_PATH)/tests/data/hls_fmp4_ac3.m3u8 2>/dev/null
>       > 
>       >  FATE_HLSENC-$(call ALLYES, HLS_DEMUXER EAC3_DEMUXER) +=
>       fate-hls-fmp4_ac3
>       >  fate-hls-fmp4_ac3: tests/data/hls_fmp4_ac3.m3u8
>       > -fate-hls-fmp4_ac3: CMD = probeaudiostream
>       $(TARGET_PATH)/tests/data/now.mp4
>       > +fate-hls-fmp4_ac3: CMD = probeaudiostream
>       $(TARGET_PATH)/tests/data/now_ac3.mp4
>       > 
>       >  FATE_SAMPLES_FFMPEG += $(FATE_HLSENC-yes)
>       >  fate-hlsenc: $(FATE_HLSENC-yes)
>       >
>       This test uses ffprobe, yet has no ffprobe dependency. It should
>       probably be included in FATE_SAMPLES_FFMPEG_FFPROBE; the other
>       tests in
>       this file meanwhile all generate their samples themselves, so
>       they could
>       be in FATE_FFMPEG if I am not mistaken.
> 
> 
> Fair enough, I guess I can try to make a patch to fix that aspect.
> 
> But that's orthogonal to this patch, which I'm hoping would fix the spurious
> failures in fate-hls-fmp4 that I've been running into. Any comments on this
> change in itself?

This patch was ok'd on irc by Anton and Jan, so I'll go ahead and push it 
- the requested unrelated change is up for review.

// Martin
diff mbox series

Patch

diff --git a/tests/fate/hlsenc.mak b/tests/fate/hlsenc.mak
index a57450cd7c..b3e87c0542 100644
--- a/tests/fate/hlsenc.mak
+++ b/tests/fate/hlsenc.mak
@@ -101,13 +101,13 @@  tests/data/hls_fmp4_ac3.m3u8: TAG = GEN
 tests/data/hls_fmp4_ac3.m3u8: ffmpeg$(PROGSSUF)$(EXESUF) | tests/data
 	$(M)$(TARGET_EXEC) $(TARGET_PATH)/$< \
 	-stream_loop 4 -i $(SAMPLES)/ac3/monsters_inc_5.1_448_small.ac3 -c copy -map 0 \
-	-hls_segment_type fmp4 -hls_fmp4_init_filename now.mp4 -hls_list_size 0 \
-	-hls_time 2 -hls_segment_filename "$(TARGET_PATH)/tests/data/hls_fmp4_%d.m4s" \
+	-hls_segment_type fmp4 -hls_fmp4_init_filename now_ac3.mp4 -hls_list_size 0 \
+	-hls_time 2 -hls_segment_filename "$(TARGET_PATH)/tests/data/hls_fmp4_ac3_%d.m4s" \
 	$(TARGET_PATH)/tests/data/hls_fmp4_ac3.m3u8 2>/dev/null
 
 FATE_HLSENC-$(call ALLYES, HLS_DEMUXER EAC3_DEMUXER) += fate-hls-fmp4_ac3
 fate-hls-fmp4_ac3: tests/data/hls_fmp4_ac3.m3u8
-fate-hls-fmp4_ac3: CMD = probeaudiostream $(TARGET_PATH)/tests/data/now.mp4
+fate-hls-fmp4_ac3: CMD = probeaudiostream $(TARGET_PATH)/tests/data/now_ac3.mp4
 
 FATE_SAMPLES_FFMPEG += $(FATE_HLSENC-yes)
 fate-hlsenc: $(FATE_HLSENC-yes)