diff mbox

[FFmpeg-devel] fate: add fate-adts-id3v1-demux

Message ID 20170604160839.5748-1-jamrial@gmail.com
State Accepted
Headers show

Commit Message

James Almer June 4, 2017, 4:08 p.m. UTC
This test the demuxer discarding non ADTS frames at the beginning and
end of the input.

As a side effect, this commit also enables fate-adts-demux, which was
accidentally disabled in 324f0fbff1245f9e9e1dda29ecb03138a2de287d.

Signed-off-by: James Almer <jamrial@gmail.com>
---
Sample is in http://0x0.st/6gI.aac

Should be renamed to id3v1.aac and placed in the aac folder.

 tests/fate/demux.mak            | 3 ++-
 tests/ref/fate/adts-id3v1-demux | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)
 create mode 100644 tests/ref/fate/adts-id3v1-demux

Comments

James Almer June 4, 2017, 8:29 p.m. UTC | #1
On 6/4/2017 1:08 PM, James Almer wrote:
> This test the demuxer discarding non ADTS frames at the beginning and
> end of the input.
> 
> As a side effect, this commit also enables fate-adts-demux, which was
> accidentally disabled in 324f0fbff1245f9e9e1dda29ecb03138a2de287d.
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
> Sample is in http://0x0.st/6gI.aac

I could instead use a smaller sample (with say 10 or so frames) to use
framecrc instead of crc and not end up with a huge ref file, but such a
small sample with garbage data before the first adts frame ends up
having a probing score of 1.

According to lcov there's already a test with a file that also probes
with a score of 1, so should i assume it's safe or would it be too
fragile? Worst case I could just force the aac demuxer in the test i guess.

> 
> Should be renamed to id3v1.aac and placed in the aac folder.
> 
>  tests/fate/demux.mak            | 3 ++-
>  tests/ref/fate/adts-id3v1-demux | 1 +
>  2 files changed, 3 insertions(+), 1 deletion(-)
>  create mode 100644 tests/ref/fate/adts-id3v1-demux
> 
> diff --git a/tests/fate/demux.mak b/tests/fate/demux.mak
> index cf69e2fe2c..f3e054f90b 100644
> --- a/tests/fate/demux.mak
> +++ b/tests/fate/demux.mak
> @@ -1,8 +1,9 @@
>  FATE_SAMPLES_DEMUX-$(call DEMDEC, AVI, FRAPS) += fate-avio-direct
>  fate-avio-direct: CMD = framecrc -avioflags direct -i $(TARGET_SAMPLES)/fraps/fraps-v5-bouncing-balls-partial.avi -avioflags direct
>  
> -FATE_SAMPLES_DEMUX-$(DEMDEC, AAC, AAC) += fate-adts-demux
> +FATE_SAMPLES_DEMUX-$(call DEMDEC, AAC, AAC) += fate-adts-demux fate-adts-id3v1-demux
>  fate-adts-demux: CMD = crc -i $(TARGET_SAMPLES)/aac/ct_faac-adts.aac -acodec copy
> +fate-adts-id3v1-demux: CMD = crc -i $(TARGET_SAMPLES)/aac/id3v1.aac -acodec copy
>  
>  FATE_SAMPLES_DEMUX-$(CONFIG_AEA_DEMUXER) += fate-aea-demux
>  fate-aea-demux: CMD = crc -i $(TARGET_SAMPLES)/aea/chirp.aea -acodec copy
> diff --git a/tests/ref/fate/adts-id3v1-demux b/tests/ref/fate/adts-id3v1-demux
> new file mode 100644
> index 0000000000..f201d80bcd
> --- /dev/null
> +++ b/tests/ref/fate/adts-id3v1-demux
> @@ -0,0 +1 @@
> +CRC=0x302a4a48
>
Michael Niedermayer June 4, 2017, 11:49 p.m. UTC | #2
On Sun, Jun 04, 2017 at 01:08:39PM -0300, James Almer wrote:
> This test the demuxer discarding non ADTS frames at the beginning and
> end of the input.
> 
> As a side effect, this commit also enables fate-adts-demux, which was
> accidentally disabled in 324f0fbff1245f9e9e1dda29ecb03138a2de287d.
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
> Sample is in http://0x0.st/6gI.aac

uploaded, we can change it though if a smaller file is wanted before
the test is pushed

test passes on linux x86 32/64 arm, mips and mingw32/64

thx

[...]
James Almer June 5, 2017, 2:29 p.m. UTC | #3
On 6/4/2017 8:49 PM, Michael Niedermayer wrote:
> On Sun, Jun 04, 2017 at 01:08:39PM -0300, James Almer wrote:
>> This test the demuxer discarding non ADTS frames at the beginning and
>> end of the input.
>>
>> As a side effect, this commit also enables fate-adts-demux, which was
>> accidentally disabled in 324f0fbff1245f9e9e1dda29ecb03138a2de287d.
>>
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>> Sample is in http://0x0.st/6gI.aac
> 
> uploaded, we can change it though if a smaller file is wanted before
> the test is pushed

Well, i want it mainly because framecrc is much more informative and
clearly reflects what this patch fixed, unlike crc.
Since the sample i posted above is 130 frames long it would make a huge
framecrc ref file, so my question was if a small sample that probes with
a score of 1 is a good idea for the test or too fragile.

> 
> test passes on linux x86 32/64 arm, mips and mingw32/64
> 
> thx
> 
> [...]
> 
> 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
Hendrik Leppkes June 5, 2017, 2:33 p.m. UTC | #4
On Mon, Jun 5, 2017 at 4:29 PM, James Almer <jamrial@gmail.com> wrote:
> On 6/4/2017 8:49 PM, Michael Niedermayer wrote:
>> On Sun, Jun 04, 2017 at 01:08:39PM -0300, James Almer wrote:
>>> This test the demuxer discarding non ADTS frames at the beginning and
>>> end of the input.
>>>
>>> As a side effect, this commit also enables fate-adts-demux, which was
>>> accidentally disabled in 324f0fbff1245f9e9e1dda29ecb03138a2de287d.
>>>
>>> Signed-off-by: James Almer <jamrial@gmail.com>
>>> ---
>>> Sample is in http://0x0.st/6gI.aac
>>
>> uploaded, we can change it though if a smaller file is wanted before
>> the test is pushed
>
> Well, i want it mainly because framecrc is much more informative and
> clearly reflects what this patch fixed, unlike crc.
> Since the sample i posted above is 130 frames long it would make a huge
> framecrc ref file, so my question was if a small sample that probes with
> a score of 1 is a good idea for the test or too fragile.
>

If its not really a probe test, you could just set a fixed format and
avoid any issues from that entirely.

- Hendrik
James Almer June 5, 2017, 3:07 p.m. UTC | #5
On 6/5/2017 11:33 AM, Hendrik Leppkes wrote:
> On Mon, Jun 5, 2017 at 4:29 PM, James Almer <jamrial@gmail.com> wrote:
>> On 6/4/2017 8:49 PM, Michael Niedermayer wrote:
>>> On Sun, Jun 04, 2017 at 01:08:39PM -0300, James Almer wrote:
>>>> This test the demuxer discarding non ADTS frames at the beginning and
>>>> end of the input.
>>>>
>>>> As a side effect, this commit also enables fate-adts-demux, which was
>>>> accidentally disabled in 324f0fbff1245f9e9e1dda29ecb03138a2de287d.
>>>>
>>>> Signed-off-by: James Almer <jamrial@gmail.com>
>>>> ---
>>>> Sample is in http://0x0.st/6gI.aac
>>>
>>> uploaded, we can change it though if a smaller file is wanted before
>>> the test is pushed
>>
>> Well, i want it mainly because framecrc is much more informative and
>> clearly reflects what this patch fixed, unlike crc.
>> Since the sample i posted above is 130 frames long it would make a huge
>> framecrc ref file, so my question was if a small sample that probes with
>> a score of 1 is a good idea for the test or too fragile.
>>
> 
> If its not really a probe test, you could just set a fixed format and
> avoid any issues from that entirely.
> 
> - Hendrik

Alright, i'll do that.
diff mbox

Patch

diff --git a/tests/fate/demux.mak b/tests/fate/demux.mak
index cf69e2fe2c..f3e054f90b 100644
--- a/tests/fate/demux.mak
+++ b/tests/fate/demux.mak
@@ -1,8 +1,9 @@ 
 FATE_SAMPLES_DEMUX-$(call DEMDEC, AVI, FRAPS) += fate-avio-direct
 fate-avio-direct: CMD = framecrc -avioflags direct -i $(TARGET_SAMPLES)/fraps/fraps-v5-bouncing-balls-partial.avi -avioflags direct
 
-FATE_SAMPLES_DEMUX-$(DEMDEC, AAC, AAC) += fate-adts-demux
+FATE_SAMPLES_DEMUX-$(call DEMDEC, AAC, AAC) += fate-adts-demux fate-adts-id3v1-demux
 fate-adts-demux: CMD = crc -i $(TARGET_SAMPLES)/aac/ct_faac-adts.aac -acodec copy
+fate-adts-id3v1-demux: CMD = crc -i $(TARGET_SAMPLES)/aac/id3v1.aac -acodec copy
 
 FATE_SAMPLES_DEMUX-$(CONFIG_AEA_DEMUXER) += fate-aea-demux
 fate-aea-demux: CMD = crc -i $(TARGET_SAMPLES)/aea/chirp.aea -acodec copy
diff --git a/tests/ref/fate/adts-id3v1-demux b/tests/ref/fate/adts-id3v1-demux
new file mode 100644
index 0000000000..f201d80bcd
--- /dev/null
+++ b/tests/ref/fate/adts-id3v1-demux
@@ -0,0 +1 @@ 
+CRC=0x302a4a48