Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: "Yann E. MORIN" <yann.morin.1998@free.fr>
To: Julien Olivain <ju.o@free.fr>
Cc: buildroot@buildroot.org
Subject: Re: [Buildroot] [PATCH 1/2] support/testing: test_audio_codec_base.py: new helper class
Date: Sat, 15 Jun 2024 20:09:48 +0200	[thread overview]
Message-ID: <Zm3Y7Fd-Zpga_LAv@landeda> (raw)
In-Reply-To: <20240615091612.9579-1-ju.o@free.fr>

Julien, All,

On 2024-06-15 11:16 +0200, Julien Olivain spake thusly:
> This is a helper class providing a template for testing audio codec
> programs such as lame mp3 encoder, flac tools, ogg vorbis-tools, ...
> 
> Signed-off-by: Julien Olivain <ju.o@free.fr>
> ---
[--SNIP--]
> diff --git a/support/testing/tests/package/test_audio_codec_base.py b/support/testing/tests/package/test_audio_codec_base.py
> new file mode 100644
> index 00000000000..59c7efcad3c
> --- /dev/null
> +++ b/support/testing/tests/package/test_audio_codec_base.py
> @@ -0,0 +1,86 @@
> +import math
> +import os
> +
> +import infra.basetest
> +
> +
> +class TestAudioCodecBase(infra.basetest.BRTest):
> +    """Common class to test an audio codec package.
> +
> +    This base class builds a Buildroot system image containing the
> +    package enabled in its config, start the emulator, login to it. It
> +    prepares an input test WAV file containing a tone. This WAV file
> +    is encoded into a new output file in the native encoder format. It
> +    is then decoded to a new output WAV file. This final output WAV
> +    file is checked to contain the expected tone generated in the
> +    initial input WAV file. There is no specific check about file
> +    size, nor audio quality. The acceptance criteria is the
> +    recognition of the tone. Note: the tone generation is made with
> +    the Sox package, and the tone recognition is made with the Aubio
> +    package.
> +
> +    Each test case that inherits from this class must have:
> +    __test__ = True  - to let nose2 know that it is a test case

If the class name does not stat with "Test", then nose2 will not
consider it a test case, it would not be necxessary to set __test__ =
False here, and rely on inheriters to set it.

I see that there are many such cases already in the tree, but I don;t
think we should continue in that direction.

Maybe just name this base calss as such:

    class BaseAudioCodec(infra.basetest.BRTest):

But see below: I think we should consider it a test by itself: it
validates that the sox-aubio combo does work as expected.

[--SNIP--]
> +    def encode_test(self, input_filename):
> +        msg = "method must be implemented in derived class"
> +        raise NotImplementedError(msg)

I am not python expert, but I think that would be better handled by an
"ABC", and Abstract Base Class (https://docs.python.org/3/library/abc.html),
which I think would look like:

    import abc
    class BaseAudioCodec(abc.ABC):
        @abc.abstractmethod
        def encode_test(self, input_filename):
            ...

Note that the ellipsis is really a real ellipsis, not a placeholder (see
https://docs.python.org/3/reference/datamodel.html#ellipsis).

But as I suggested above, we should just make this class a proper test,
and have empty encode() and decode() functions.

It would validate that the tone generated by sox can be decoded by
aubio.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

  parent reply	other threads:[~2024-06-15 18:10 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-15  9:16 [Buildroot] [PATCH 1/2] support/testing: test_audio_codec_base.py: new helper class Julien Olivain
2024-06-15  9:16 ` [Buildroot] [PATCH 2/2] support/testing: add lame runtime test Julien Olivain
2024-06-15 18:09 ` Yann E. MORIN [this message]
2024-06-21 14:40   ` [Buildroot] [PATCH 1/2] support/testing: test_audio_codec_base.py: new helper class Julien Olivain
2024-08-07 19:35     ` Thomas Petazzoni via buildroot
2024-08-07 21:09       ` Julien Olivain
2024-08-07 21:20         ` Thomas Petazzoni via buildroot
2024-08-06 21:24 ` Thomas Petazzoni via buildroot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Zm3Y7Fd-Zpga_LAv@landeda \
    --to=yann.morin.1998@free.fr \
    --cc=buildroot@buildroot.org \
    --cc=ju.o@free.fr \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox