| Message ID | 20240218104534.17743-1-cus@passwd.hu |
|---|---|
| State | Accepted |
| Commit | 86410e55adaf3cd9016863d83aaa4b5c43fb0d30 |
| Headers | show |
| Series | [FFmpeg-devel] fate: use an even more exotic channel layout mov-mp4-pcm-float test | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
> 在 2024年2月18日，下午6:45，Marton Balint <cus@passwd.hu> 写道： > > The old layout happened to be a native layout and therefore missed some > recently fixed layout parsing bugs. > > Signed-off-by: Marton Balint <cus@passwd.hu> > --- > tests/fate/mov.mak | 2 +- > tests/ref/fate/mov-mp4-pcm-float | 4 ++-- > 2 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/tests/fate/mov.mak b/tests/fate/mov.mak > index 4850c8aa94..8d154c8b5b 100644 > --- a/tests/fate/mov.mak > +++ b/tests/fate/mov.mak > @@ -187,7 +187,7 @@ fate-mov-mp4-pcm: CMD = transcode wav $(TARGET_PATH)/tests/data/asynth-44100-1.w > FATE_MOV_FFMPEG-$(call TRANSCODE, PCM_S16LE, MOV, WAV_DEMUXER PAN_FILTER) \ > += fate-mov-mp4-pcm-float > fate-mov-mp4-pcm-float: tests/data/asynth-44100-1.wav > -fate-mov-mp4-pcm-float: CMD = transcode wav $(TARGET_PATH)/tests/data/asynth-44100-1.wav mp4 "-af aresample,pan=FL+LFE+BR|c0=c0|c1=c0|c2=c0 -c:a pcm_f32le" "-map 0 -c copy -frames:a 0" > +fate-mov-mp4-pcm-float: CMD = transcode wav $(TARGET_PATH)/tests/data/asynth-44100-1.wav mp4 "-af aresample,pan=FR+FL+FR|c0=c0|c1=c0|c2=c0 -c:a pcm_f32le" "-map 0 -c copy -frames:a 0" > > fate-mov-pcm-remux: tests/data/asynth-44100-1.wav > fate-mov-pcm-remux: CMD = md5 -i $(TARGET_PATH)/tests/data/asynth-44100-1.wav -map 0 -c copy -fflags +bitexact -f mp4 > diff --git a/tests/ref/fate/mov-mp4-pcm-float b/tests/ref/fate/mov-mp4-pcm-float > index 851b79090c..7da8fd2aba 100644 > --- a/tests/ref/fate/mov-mp4-pcm-float > +++ b/tests/ref/fate/mov-mp4-pcm-float > @@ -1,7 +1,7 @@ > -691a76a847e0f3720c09cca341971f19 *tests/data/fate/mov-mp4-pcm-float.mp4 > +7b998e652d5b7154e646a98bd2bf28a1 *tests/data/fate/mov-mp4-pcm-float.mp4 > 3175929 tests/data/fate/mov-mp4-pcm-float.mp4 > #tb 0: 1/44100 > #media_type 0: audio > #codec_id 0: pcm_f32le > #sample_rate 0: 44100 > -#channel_layout_name 0: 3 channels (FL+LFE+BR) > +#channel_layout_name 0: 3 channels (FR+FL+FR) It’s possible to create such file manually, however, I’m wondering how it works physically with two speakers at the same position (FR)? > -- > 2.35.3 > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
On 2/18/2024 7:45 AM, Marton Balint wrote: > The old layout happened to be a native layout and therefore missed some > recently fixed layout parsing bugs. > > Signed-off-by: Marton Balint <cus@passwd.hu> > --- > tests/fate/mov.mak | 2 +- > tests/ref/fate/mov-mp4-pcm-float | 4 ++-- > 2 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/tests/fate/mov.mak b/tests/fate/mov.mak > index 4850c8aa94..8d154c8b5b 100644 > --- a/tests/fate/mov.mak > +++ b/tests/fate/mov.mak > @@ -187,7 +187,7 @@ fate-mov-mp4-pcm: CMD = transcode wav $(TARGET_PATH)/tests/data/asynth-44100-1.w > FATE_MOV_FFMPEG-$(call TRANSCODE, PCM_S16LE, MOV, WAV_DEMUXER PAN_FILTER) \ > += fate-mov-mp4-pcm-float > fate-mov-mp4-pcm-float: tests/data/asynth-44100-1.wav > -fate-mov-mp4-pcm-float: CMD = transcode wav $(TARGET_PATH)/tests/data/asynth-44100-1.wav mp4 "-af aresample,pan=FL+LFE+BR|c0=c0|c1=c0|c2=c0 -c:a pcm_f32le" "-map 0 -c copy -frames:a 0" > +fate-mov-mp4-pcm-float: CMD = transcode wav $(TARGET_PATH)/tests/data/asynth-44100-1.wav mp4 "-af aresample,pan=FR+FL+FR|c0=c0|c1=c0|c2=c0 -c:a pcm_f32le" "-map 0 -c copy -frames:a 0" Wouldn't FR+FL+LFE be enough to test this? While also generating a file that's realistic. > > fate-mov-pcm-remux: tests/data/asynth-44100-1.wav > fate-mov-pcm-remux: CMD = md5 -i $(TARGET_PATH)/tests/data/asynth-44100-1.wav -map 0 -c copy -fflags +bitexact -f mp4 > diff --git a/tests/ref/fate/mov-mp4-pcm-float b/tests/ref/fate/mov-mp4-pcm-float > index 851b79090c..7da8fd2aba 100644 > --- a/tests/ref/fate/mov-mp4-pcm-float > +++ b/tests/ref/fate/mov-mp4-pcm-float > @@ -1,7 +1,7 @@ > -691a76a847e0f3720c09cca341971f19 *tests/data/fate/mov-mp4-pcm-float.mp4 > +7b998e652d5b7154e646a98bd2bf28a1 *tests/data/fate/mov-mp4-pcm-float.mp4 > 3175929 tests/data/fate/mov-mp4-pcm-float.mp4 > #tb 0: 1/44100 > #media_type 0: audio > #codec_id 0: pcm_f32le > #sample_rate 0: 44100 > -#channel_layout_name 0: 3 channels (FL+LFE+BR) > +#channel_layout_name 0: 3 channels (FR+FL+FR)
On Sun, 18 Feb 2024, James Almer wrote: > On 2/18/2024 7:45 AM, Marton Balint wrote: >> The old layout happened to be a native layout and therefore missed some >> recently fixed layout parsing bugs. >> >> Signed-off-by: Marton Balint <cus@passwd.hu> >> --- >> tests/fate/mov.mak | 2 +- >> tests/ref/fate/mov-mp4-pcm-float | 4 ++-- >> 2 files changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/tests/fate/mov.mak b/tests/fate/mov.mak >> index 4850c8aa94..8d154c8b5b 100644 >> --- a/tests/fate/mov.mak >> +++ b/tests/fate/mov.mak >> @@ -187,7 +187,7 @@ fate-mov-mp4-pcm: CMD = transcode wav >> $(TARGET_PATH)/tests/data/asynth-44100-1.w >> FATE_MOV_FFMPEG-$(call TRANSCODE, PCM_S16LE, MOV, WAV_DEMUXER >> PAN_FILTER) \ >> += fate-mov-mp4-pcm-float >> fate-mov-mp4-pcm-float: tests/data/asynth-44100-1.wav >> -fate-mov-mp4-pcm-float: CMD = transcode wav >> $(TARGET_PATH)/tests/data/asynth-44100-1.wav mp4 "-af >> aresample,pan=FL+LFE+BR|c0=c0|c1=c0|c2=c0 -c:a pcm_f32le" "-map 0 -c copy >> -frames:a 0" >> +fate-mov-mp4-pcm-float: CMD = transcode wav >> $(TARGET_PATH)/tests/data/asynth-44100-1.wav mp4 "-af >> aresample,pan=FR+FL+FR|c0=c0|c1=c0|c2=c0 -c:a pcm_f32le" "-map 0 -c copy >> -frames:a 0" > > Wouldn't FR+FL+LFE be enough to test this? While also generating a file > that's realistic. It depends on what you want to test. With the old code, for FR+FL+LFE, only the channel order would have been wrong, with FR+FL+FR also the channel count. Having the same channel position in the same track is not that theoretical, at least for MOV I have samples where an additional FR/FL track is used for Music/Effects. I admit, for MP4 that might be less common though, and I also admit that using a separate track for it would be better. But as we know, nothing is ideal in practice... Nevertheless, I can change the test to FR+FL+LFE if that is preferred. Regards, Marton
On 2/18/2024 3:38 PM, Marton Balint wrote: > > > On Sun, 18 Feb 2024, James Almer wrote: > >> On 2/18/2024 7:45 AM, Marton Balint wrote: >>> The old layout happened to be a native layout and therefore missed some >>> recently fixed layout parsing bugs. >>> >>> Signed-off-by: Marton Balint <cus@passwd.hu> >>> --- >>> tests/fate/mov.mak | 2 +- >>> tests/ref/fate/mov-mp4-pcm-float | 4 ++-- >>> 2 files changed, 3 insertions(+), 3 deletions(-) >>> >>> diff --git a/tests/fate/mov.mak b/tests/fate/mov.mak >>> index 4850c8aa94..8d154c8b5b 100644 >>> --- a/tests/fate/mov.mak >>> +++ b/tests/fate/mov.mak >>> @@ -187,7 +187,7 @@ fate-mov-mp4-pcm: CMD = transcode wav >>> $(TARGET_PATH)/tests/data/asynth-44100-1.w >>> FATE_MOV_FFMPEG-$(call TRANSCODE, PCM_S16LE, MOV, WAV_DEMUXER >>> PAN_FILTER) \ >>> += fate-mov-mp4-pcm-float >>> fate-mov-mp4-pcm-float: tests/data/asynth-44100-1.wav >>> -fate-mov-mp4-pcm-float: CMD = transcode wav >>> $(TARGET_PATH)/tests/data/asynth-44100-1.wav mp4 "-af >>> aresample,pan=FL+LFE+BR|c0=c0|c1=c0|c2=c0 -c:a pcm_f32le" "-map 0 -c >>> copy >>> -frames:a 0" >>> +fate-mov-mp4-pcm-float: CMD = transcode wav >>> $(TARGET_PATH)/tests/data/asynth-44100-1.wav mp4 "-af >>> aresample,pan=FR+FL+FR|c0=c0|c1=c0|c2=c0 -c:a pcm_f32le" "-map 0 -c >>> copy >>> -frames:a 0" >> >> Wouldn't FR+FL+LFE be enough to test this? While also generating a >> file that's realistic. > > It depends on what you want to test. With the old code, for FR+FL+LFE, > only the channel order would have been wrong, with FR+FL+FR also the > channel count. > > Having the same channel position in the same track is not that > theoretical, at least for MOV I have samples where an additional FR/FL > track is used for Music/Effects. I admit, for MP4 that might be less > common though, and I also admit that using a separate track for it would > be better. But as we know, nothing is ideal in practice... > > Nevertheless, I can change the test to FR+FL+LFE if that is preferred. No, it's ok as is if it tests more things that can potentially regress. > > Regards, > Marton > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
On Sun, 18 Feb 2024, James Almer wrote: > On 2/18/2024 3:38 PM, Marton Balint wrote: >> >> >> On Sun, 18 Feb 2024, James Almer wrote: >> >>> On 2/18/2024 7:45 AM, Marton Balint wrote: >>>> The old layout happened to be a native layout and therefore missed some >>>> recently fixed layout parsing bugs. >>>> >>>> Signed-off-by: Marton Balint <cus@passwd.hu> >>>> --- >>>> tests/fate/mov.mak | 2 +- >>>> tests/ref/fate/mov-mp4-pcm-float | 4 ++-- >>>> 2 files changed, 3 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/tests/fate/mov.mak b/tests/fate/mov.mak >>>> index 4850c8aa94..8d154c8b5b 100644 >>>> --- a/tests/fate/mov.mak >>>> +++ b/tests/fate/mov.mak >>>> @@ -187,7 +187,7 @@ fate-mov-mp4-pcm: CMD = transcode wav >>>> $(TARGET_PATH)/tests/data/asynth-44100-1.w >>>> FATE_MOV_FFMPEG-$(call TRANSCODE, PCM_S16LE, MOV, WAV_DEMUXER >>>> PAN_FILTER) \ >>>> += fate-mov-mp4-pcm-float >>>> fate-mov-mp4-pcm-float: tests/data/asynth-44100-1.wav >>>> -fate-mov-mp4-pcm-float: CMD = transcode wav >>>> $(TARGET_PATH)/tests/data/asynth-44100-1.wav mp4 "-af >>>> aresample,pan=FL+LFE+BR|c0=c0|c1=c0|c2=c0 -c:a pcm_f32le" "-map 0 -c >>>> copy >>>> -frames:a 0" >>>> +fate-mov-mp4-pcm-float: CMD = transcode wav >>>> $(TARGET_PATH)/tests/data/asynth-44100-1.wav mp4 "-af >>>> aresample,pan=FR+FL+FR|c0=c0|c1=c0|c2=c0 -c:a pcm_f32le" "-map 0 -c >>>> copy >>>> -frames:a 0" >>> >>> Wouldn't FR+FL+LFE be enough to test this? While also generating a file >>> that's realistic. >> >> It depends on what you want to test. With the old code, for FR+FL+LFE, >> only the channel order would have been wrong, with FR+FL+FR also the >> channel count. >> >> Having the same channel position in the same track is not that >> theoretical, at least for MOV I have samples where an additional FR/FL >> track is used for Music/Effects. I admit, for MP4 that might be less >> common though, and I also admit that using a separate track for it would >> be better. But as we know, nothing is ideal in practice... >> >> Nevertheless, I can change the test to FR+FL+LFE if that is preferred. > > No, it's ok as is if it tests more things that can potentially regress. Ok, will apply as is then. Thanks, Marton
diff --git a/tests/fate/mov.mak b/tests/fate/mov.mak index 4850c8aa94..8d154c8b5b 100644 --- a/tests/fate/mov.mak +++ b/tests/fate/mov.mak @@ -187,7 +187,7 @@ fate-mov-mp4-pcm: CMD = transcode wav $(TARGET_PATH)/tests/data/asynth-44100-1.w FATE_MOV_FFMPEG-$(call TRANSCODE, PCM_S16LE, MOV, WAV_DEMUXER PAN_FILTER) \ += fate-mov-mp4-pcm-float fate-mov-mp4-pcm-float: tests/data/asynth-44100-1.wav -fate-mov-mp4-pcm-float: CMD = transcode wav $(TARGET_PATH)/tests/data/asynth-44100-1.wav mp4 "-af aresample,pan=FL+LFE+BR|c0=c0|c1=c0|c2=c0 -c:a pcm_f32le" "-map 0 -c copy -frames:a 0" +fate-mov-mp4-pcm-float: CMD = transcode wav $(TARGET_PATH)/tests/data/asynth-44100-1.wav mp4 "-af aresample,pan=FR+FL+FR|c0=c0|c1=c0|c2=c0 -c:a pcm_f32le" "-map 0 -c copy -frames:a 0" fate-mov-pcm-remux: tests/data/asynth-44100-1.wav fate-mov-pcm-remux: CMD = md5 -i $(TARGET_PATH)/tests/data/asynth-44100-1.wav -map 0 -c copy -fflags +bitexact -f mp4 diff --git a/tests/ref/fate/mov-mp4-pcm-float b/tests/ref/fate/mov-mp4-pcm-float index 851b79090c..7da8fd2aba 100644 --- a/tests/ref/fate/mov-mp4-pcm-float +++ b/tests/ref/fate/mov-mp4-pcm-float @@ -1,7 +1,7 @@ -691a76a847e0f3720c09cca341971f19 *tests/data/fate/mov-mp4-pcm-float.mp4 +7b998e652d5b7154e646a98bd2bf28a1 *tests/data/fate/mov-mp4-pcm-float.mp4 3175929 tests/data/fate/mov-mp4-pcm-float.mp4 #tb 0: 1/44100 #media_type 0: audio #codec_id 0: pcm_f32le #sample_rate 0: 44100 -#channel_layout_name 0: 3 channels (FL+LFE+BR) +#channel_layout_name 0: 3 channels (FR+FL+FR)
The old layout happened to be a native layout and therefore missed some recently fixed layout parsing bugs. Signed-off-by: Marton Balint <cus@passwd.hu> --- tests/fate/mov.mak | 2 +- tests/ref/fate/mov-mp4-pcm-float | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)