Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add descriptive and locale properties to audio streams #1026

Merged

Conversation

AudricV
Copy link
Member

@AudricV AudricV commented Feb 12, 2023

  • I carefully read the contribution guidelines and agree to them.
  • I have tested the API against NewPipe.
  • I agree to create a pull request for NewPipe as soon as possible to make it compatible with the changed API. No breaking changes.

This PR adds support to extract descriptive audio support property in AudioStream (only implemented on YouTube, with a test added), which are currently not distinguished with main audio tracks.

It also adds the ability to get a Locale for audio languages, which should make easier for clients to get a language object, and so avoid them to parse the audio track id (which may be not reliable in some services, such as MediaCCC where there is no audio track ID but only a language tag) to get a Locale. Support and tests have been added for YouTube (streams and DASH manifests creators) and MediaCCC (other services doesn't seem to support multiple audio tracks, as far I know).

A few documentation improvements and fixes (in the code and in the documentation) around the changes done have been also made.

This PR is required to add an optimal multiple audio tracks support in NewPipe.

@AudricV AudricV added enhancement New feature or request youtube service, https://www.youtube.com/ media.ccc.de service, https://media.ccc.de labels Feb 12, 2023
Copy link
Member

@Stypox Stypox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! Code looks good to me.

Could you add some javadoc explanation about what descriptive audios are?

@AudricV AudricV force-pushed the audio-streams-descriptive-and-locale-properties branch from 3d05ee2 to 7509058 Compare February 20, 2023 18:33
@AudricV
Copy link
Member Author

AudricV commented Feb 20, 2023

Could you add some javadoc explanation about what descriptive audios are?

Done

@AudricV AudricV requested a review from Stypox February 20, 2023 18:34
@AudricV AudricV force-pushed the audio-streams-descriptive-and-locale-properties branch from 7509058 to 4392b18 Compare February 22, 2023 18:53
@AudricV AudricV requested a review from Stypox February 25, 2023 12:57
Also improve AudioStream's audio language documentation
Getting audio tracks locales by parsing their ID or their label, should not be
done by clients, but by the extractor.

This commit adds the ability to store the Locale of an AudioStream, which is
used to compare similar AudioStreams (in the equalStats method).
…cale when available

Also use audio track setters only for audio itags.
This test uses video TjxC-evzxdk.

Also improve a bit YoutubeStreamExtractorDefaultTest.AudioTrackLanguage test.
…o test audio locale property

The Hindi audio track language presence test has been changed from audio track
label to audio locale.
@AudricV AudricV force-pushed the audio-streams-descriptive-and-locale-properties branch from 4392b18 to 95b3f5e Compare February 26, 2023 18:06
@AudricV AudricV marked this pull request as draft February 28, 2023 11:11
@Stypox Stypox marked this pull request as ready for review March 1, 2023 10:15
@Stypox Stypox merged commit 6bdd698 into TeamNewPipe:dev Mar 1, 2023
@AudricV AudricV deleted the audio-streams-descriptive-and-locale-properties branch March 1, 2023 10:16
@Theta-Dev Theta-Dev mentioned this pull request Mar 19, 2023
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request media.ccc.de service, https://media.ccc.de youtube service, https://www.youtube.com/
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants