Add Audio Folder
Opened by polinaeterna (closed)

Would resolve  #3964

AudioFolder loads a .txt file with transcriptions and creates a dataset with all audiofiles in provided directory that has a transcription (independently of the directory structure) as a single split (train).

Can be loaded via:
```python
# for local dirs
dataset = load_dataset("audiofolder", data_dir="/path/to/folder", transcripts_filename="transcripts.txt")
```
```python
# for local and remote zip archives
dataset = load_dataset("audiofolder", data_files="path/to/archive/archive.zip", transcripts_filename="transcripts.txt")
```
default transcriptions filename is `transcripts.txt`. it should have the following structure:
```
audio_id_1   transcription text 1
audio_id_1   transcription text 1
```
separator is `\t`!

---
sorry for first old commits from other branch, don't know how that happened... 

Comments:
The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/datasets/pr_3963). All of your documentation changes will be reflected on that endpoint.

Feel free to merge `master` into this branch to fix the CI errors related to Google Drive :)

I think we can just remove the test that is based on dummy data, or make it have the `sampling_rate` parameter hardcoded in the test

IMO it's important to keep this loader aligned with `imagefolder`. I'm aware that the current `imagefolder` API is limiting because only labels can be inferred from the directory structure, which means it can only be used for classification and self-supervised pretraining. However, to make the loader more generic, we plan to support [metadata files](https://huggingface.slack.com/archives/C02JB9L6JKF/p1645450017434029?thread_ts=1645157416.389499&cid=C02JB9L6JKF) (will work on that this week),  and in the audio case, these files can store transcripts.

Streaming TAR archives (`iter_archive`) is not supported by any of the loaders currently, so we can add that in a separate PR for all of them (to keep this PR simple).

WDYT?

> Streaming TAR archives (iter_archive) is not supported by any of the loaders currently, so we can add that in a separate PR for all of them (to keep this PR simple).

Yes definitely, we can see that later

> to make the loader more generic, we plan to support [metadata files](https://huggingface.slack.com/archives/C02JB9L6JKF/p1645450017434029?thread_ts=1645157416.389499&cid=C02JB9L6JKF) (will work on that this week), and in the audio case, these files can store transcripts.

Could you share an example of what the structure would look like in this case ?

Note that for audio we ultimately should be able to load several splits at once (common voice, librispeech, etc. all have splits), unlike the current imagefolder implementation that puts everything in `train` (EDIT: I mean, when we pass `data_dir`). If we want consistency then we would need the same for imagefolder.

> I think we can just remove the test that is based on dummy data, or make it have the sampling_rate parameter hardcoded in the test

Not sure what to do with `test_builder_class` and `test_load_dataset_offline`, I don't really want to drop these tests completely but do you think it's a good idea to hardcode builder loading like this: 🤔
```
if dataset_name == "audiofolder":
    builder = builder_cls(name=name, cache_dir=tmp_cache_dir, sampling_rate=16_000)
else:
    builder = builder_cls(name=name, cache_dir=tmp_cache_dir)
```
@mariosasko totally agree on that APIs should be aligned, do you think we should implement metadata support first? Or maybe we can merge this PR with explicit single transcript file and add full metadata support further.

Splits support is definitely a required feature too, I think we can implement it in the future PR too.   


btw i've found a workaround for splits generation :D

```
from datasets.data_files import DataFilesDict

ds = load_dataset(
    "audiofolder",
    data_files=DataFilesDict(
        {
            "train":"../audiofolder/AudioTestSplits/train.zip",
            "test": "../audiofolder/AudioTestSplits/test.zip"
        }
    ),
    sampling_rate=16_000
)
```

> Not sure what to do with test_builder_class and test_load_dataset_offline, I don't really want to drop these tests completely but do you think it's a good idea to hardcode builder loading like this: 🤔

Yes it's fine. If you you're not a fan of having such parameters directly at the core of the code you can declare a global variable `PACKAGED_MODULES_TEST_KWARGS = {"audiofolder": {"sampling_rate": 16_000}}` and do
```python
builder_kwargs = PACKAGED_MODULES_TEST_KWARGS.get(name, {})
builder = builder_cls(name=name, cache_dir=tmp_cache_dir, **builder_kwargs)
```

> btw i've found a workaround for splits generation :D

Yes that works :) Note that you don't have to use `DataFilesDict` and you can pass a python dict directly (`DataFilesDict` is for internal usage only)

@lhoestq @mariosasko please take a look at the code and feel free to add your comments and discuss the potential issues
 
after we are satisfied with the code, I'll write the documentation 

@lhoestq it appeared that this PR already exists... https://github.com/huggingface/datasets/pull/3364

> The current problem with this loader is that it supports the ASR task by default, which could be surprising for the users thinking that this is the Image Folder counterpart for audio. To avoid this, we should support the audio classification task by default instead (we can add a template for it in this PR), where the label column is inferred from the directory structure.

Right indeed, good catch. It's better to keep polishing the API rather than pushing fast something that can be confusing for users. Let's go for maximum alignment between the two then @polinaeterna ?

@mariosasko sorry, I didn't understand from your previous message that by aligning with the ImageFolder you mean inferring labels from directories names. Sure, that's not a problem, I can add the corresponding code. Do you also mean that in this version we should get rid of transcription file and feature and add it in the future when the metadata support https://github.com/huggingface/datasets/pull/4069 will be merged?  
My understanding was that support for ASR task is more crucial than audio classification as it's more "common", but I would ask @anton-l and @patrickvonplaten about this. Anyway, it's not a problem to implement the classification task first, and the ASR one later. 

> Do you also mean that in this version we should get rid of transcription file and feature and add it in the future when the metadata support https://github.com/huggingface/datasets/pull/4069 will be merged?

We can wait for the linked PR to be merged first and then add the changes to this PR to have support for ASR from the get-go.

Don't follow 100% here, but as @polinaeterna said I think ASR is much more common than audio classification. Also, do you guys think a lot of users will use both the audio and image folder functionality ? Is it very important to have audio and image aligned here? Note that in Transformers while all models follow a common API, audio and vision models can be very different with respect to pre- and post-processing

> I think ASR is much more common than audio classification

I agree, the main focus is ASR

> do you guys think a lot of users will use both the audio and image folder functionality ?

Yup I think so, people don't just use public academic datasets right ? `imagefolder` is almost used 1k times a week, and it's just the beginning.

> Is it very important to have audio and image aligned here?

If we can get some consistency for free, let's take it ^^ This way it will be easy for users to go from one modality to another, and documentation will be simpler.

> Note that in Transformers while all models follow a common API, audio and vision models can be very different with respect to pre- and post-processing

That make total sense. Here this is mainly about raw data loading (before preprocessing) so we just need to make something generic, no matter what task the data is used for. Even though actually we know that ASR will be the main usage for now :p

Let me know if it's clearer now or if you have other questions !