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

Fix downloads of streams with missing MediaFormat #10165

Merged
merged 5 commits into from
Sep 19, 2023
Merged

Conversation

TobiGr
Copy link
Member

@TobiGr TobiGr commented Jun 15, 2023

What is it?

  • Bugfix (user facing)
  • Feature (user facing)
  • Codebase improvement (dev facing)
  • Meta improvement to the project (dev facing)

Description of the changes in your PR

  • Rertrieve MediaFormats for streams in case the extractor was not able to get them. Sometimes the MediaFormat can be retrieved from the HTTP response header that is fetched when getting the stream size.

  • The dialog now also displays "Unknown Quality" if the quality is unknown. Previously, the media format name was displayed as title and as quality info.

  • Some code improvements simplifying castings by directly casting in if-conditions using instanceof

Before/After Screenshots/Screen Record

Before After

Fixes the following issue(s)

Relies on the following changes

TeamNewPipe/NewPipeExtractor#1074

APK testing

The APK can be found by going to the "Checks" tab below the title. On the left pane, click on "CI", scroll down to "artifacts" and click "app" to download the zip file which contains the debug APK of this PR.

Due diligence

@TobiGr TobiGr added bug Issue is related to a bug soundcloud Service, https://soundcloud.com/ downloader Issue is related to the downloader labels Jun 15, 2023
@TobiGr TobiGr changed the title Fix/media format Fix downloads of streams with missing MediaFormat Jun 15, 2023
@TobiGr TobiGr force-pushed the fix/media-format branch 4 times, most recently from 8e2daa4 to d0c1919 Compare July 19, 2023 00:06
@TobiGr TobiGr marked this pull request as draft July 19, 2023 15:22
@TobiGr TobiGr marked this pull request as ready for review July 19, 2023 21:51
@TobiGr
Copy link
Member Author

TobiGr commented Aug 14, 2023

  • Rebased
  • Removed encoding stuff as discussed in the review above
  • Added a few more comments & docs

@sonarcloud
Copy link

sonarcloud bot commented Aug 14, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 13 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@AudricV AudricV requested a review from Stypox August 30, 2023 18:00
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.

Thank you! I will push a small commit to improve upon the last comment, and then merge

}

// extract the file extension / suffix
final String[] p = fileName.split("\\.");
Copy link
Member

Choose a reason for hiding this comment

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

I was going to say "split this without using a regex" but then stumbled upon this: https://stackoverflow.com/a/60048140

// try to get the format by content type
// some mime types are not unique for every format, those are omitted
final List<MediaFormat> formats = MediaFormat.getAllFromMimeType(
response.getHeader("Content-Type"));
Copy link
Member

@Stypox Stypox Sep 19, 2023

Choose a reason for hiding this comment

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

response.getHeader("Content-Type") will return null if there are no such headers. Fortunately that's not a problem since getAllFromMimeType works normally with null strings, but it should be changed anyway. The loop below can also be optimized to exit-early as soon as a conflicting media format is found, and also to prevent creating a new list.

@sonarcloud
Copy link

sonarcloud bot commented Sep 19, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@Stypox Stypox merged commit 725c18e into dev Sep 19, 2023
7 of 8 checks passed
@TobiGr TobiGr deleted the fix/media-format branch September 19, 2023 14:06
This was referenced Oct 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue is related to a bug downloader Issue is related to the downloader soundcloud Service, https://soundcloud.com/
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SoundCloud Download "Unknown Quality" audio cannot be played
2 participants