AudioStream: remove unused youtube-specific fields #1150
+26
−116
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
These fields were added in TeamNewPipe#810 in preparation for a DASH support that never materialized … well it did materialize, but did not use any of these variables. Since they are youtube-specific, it does not make much sense to export them in the generic API, lest people start depending on them as if they belonged to all backends. Well, 4 variables are actually already depended on in places, they will have to be checked separately. But this is the low-hanging fruit.
Profpatsch
force-pushed
the
AudioStream-remove-unused-fields
branch
from
a719b7d
to
8448f0a
Compare
TobiGr
added
youtube
TobiGr
reviewed
Mar 18, 2024
FireMasterK
reviewed
Apr 2, 2024
Comment on lines
-36
to
-44
| // Fields for DASH | ||
| private int itag = ITAG_NOT_AVAILABLE_OR_NOT_APPLICABLE; | ||
| private int bitrate; | ||
| private int initStart; | ||
| private int initEnd; | ||
| private int indexStart; | ||
| private int indexEnd; | ||
| private String quality; | ||
| private String codec; |
There was a problem hiding this comment.
Hi, I was the one who added these fields originally, they are used in Piped's backend for providing sidX boxes for streaming with DASH.
Here's how an audio representation is created by a client: https://github.com/TeamPiped/Piped/blob/8ee2a1c20737ce9db84186e00849b405d6c097af/src/utils/DashUtils.js#L99-L117
There was a problem hiding this comment.
Then maybe a comment should be added that these were added for Piped? Similar confusion can take place whenever someone compares NP and NPE code in the future.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
These fields were added in
#810 in preparation for a DASH support that never materialized … well it did materialize, but did not use any of these variables.
Since they are youtube-specific, it does not make much sense to export them in the generic API, lest people start depending on them as if they belonged to all backends.
Well, 4 variables are actually already depended on in places, they will have to be checked separately. But this is the low-hanging fruit.