Skip to content

Technical song info fixes - #463

Open
XCroatoanX wants to merge 3 commits into
ssalggnikool:masterfrom
XCroatoanX:technical-song-info-fixes
Open

Technical song info fixes#463
XCroatoanX wants to merge 3 commits into
ssalggnikool:masterfrom
XCroatoanX:technical-song-info-fixes

Conversation

@XCroatoanX

Copy link
Copy Markdown
Contributor

Fixing track info being unreliable while switching between transcoding and original file. Fixed downloaded song showing wrong info.

@ssalggnikool ssalggnikool left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

i'm not sure what this pr is solving, your description is kind of vague and you didn't link any open issues to this, and some of these changes feel random

"database": {
"version": 3,
"identityHash": "36a1f8f31a8ab108261200449c17a0c1",
"identityHash": "b3fc255f03fed85317c2787e1971fe1e",

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

the old download database schema file changed, this might cause a crash if going to v3 to v4, just undo all changes in this file

Comment on lines +1053 to +1068
try {
val file = snapshot.data.toFile()
val options = BitmapFactory.Options().apply {
inJustDecodeBounds = true
}
BitmapFactory.decodeFile(file.absolutePath, options)
options.inSampleSize = calculateInSampleSize(options, 256, 256)
options.inJustDecodeBounds = false
val bitmap = BitmapFactory.decodeFile(file.absolutePath, options)
if (bitmap != null) {
val stream = ByteArrayOutputStream()
bitmap.compress(Bitmap.CompressFormat.JPEG, 75, stream)
val result = stream.toByteArray()
bitmap.recycle()
result
} else null

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

what is the point of changing this, it's unrelated to what you were fixing in the first place, also calculateInSampleSize is duplicated

val requestedMimeType: String?
)

private fun MediaItem.toMediaItem(): MediaItem {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

it's very unclear what this is doing because it's named toMediaItem but it takes a MediaItem, and returns a MediaItem, so it sounds like it's converting a MediaItem to a MediaItem, which makes no sense, looking at its implementation doesn't make it any clearer as to what it's doing either, not sure what the point of this is

_uiState.update { it.copy(playbackSpeed = value) }
}

private data class MediaItemTag(

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

this isn't used

}

private fun getSongUrl(song: DomainSong): NSURL? {
private fun getSongUrlInfo(song: DomainSong): Triple<NSURL, Int?, String?>? {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

instead of a Triple you should just make a new data class to contain this info

Comment on lines -1010 to -1013
} else {
metadataBuilder.setArtworkUri(
coverArtId?.let { sessionManager.getCoverArtUrl(it).toUri() }
)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

unclear why this was moved out of the else statement

}
}

fun getDownloadedFilePath(songId: String): String? {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

i don't think this function is used anymore

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants